Re: [libvirt] [PATCH 3/3] parallels: fix possible crash in case of errors in prlsdkLoadDomain

On 05/18/2015 05:44 PM, Maxim Nestratov wrote: > Cleanup code in prlsdkLoadDomain doesn't take into account the fact > if private domain freeing function is assigned or not. In case it is, > we shouldn't call it manually because virDomainObjListRemove calls it. > Also, allocated def structure should be freed only if it's not > assigned to domain. Otherwise it will be called twice one time by > virDomainObjListRemove and the second by prlsdkLoadDomain itself. Libvirt now can assign this pointer inside virDomainObjListAdd. I think you just need to set virDomainXMLOptionPtr->privateData->free to our prlsdkDomObjFreePrivate.

You can give virDomainXMLPrivateDataCallbacks structure as second argument of virDomainXMLOptionNew, and it will create appropriate virDomainXMLOption structure. > > Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com&gt; > --- > src/parallels/parallels_sdk.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/src/parallels/parallels_sdk.c > b/src/parallels/parallels_sdk.c > index 5c15e94..2f2574e 100644 > --- a/src/parallels/parallels_sdk.c > +++ b/src/parallels/parallels_sdk.c > @@ -1379,10 +1379,21 @@ prlsdkLoadDomain(parallelsConnPtr privconn, > return dom; > error: > - if (dom && !olddom) > + if (dom && !olddom) { > + /* Domain isn't persistent means that we haven't yet set > + * prlsdkDomObjFreePrivate and should call it manually > + */ > + if (!dom->persistent) > + prlsdkDomObjFreePrivate(pdom); > + > virDomainObjListRemove(privconn->domains, dom); > - virDomainDefFree(def); > - prlsdkDomObjFreePrivate(pdom); > + } > + /* Delete newly allocated def only if we haven't assigned it to > domain > + * Otherwise we will end up with domain having invalid def > within it > + */ > + if (!dom) > + virDomainDefFree(def); > + > return NULL; > }