[libvirt] [PATCH 0/3] parallels: fix prlsdkLoadDomain and prlsdkAddDomain
Maxim Nestratov (3): parallels: fix prlsdkAddDomain parallels: move up updating autostart parameter in prlsdkLoadDomain parallels: fix possible crash in case of errors in prlsdkLoadDomain src/parallels/parallels_sdk.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) -- 2.1.0
Don't forget to unlock domain on error path Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 786e0ad..faae1ae 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1443,8 +1443,10 @@ prlsdkAddDomain(parallelsConnPtr privconn, const unsigned char *uuid) } sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); - if (sdkdom == PRL_INVALID_HANDLE) + if (sdkdom == PRL_INVALID_HANDLE) { + virObjectUnlock(dom); return NULL; + } dom = prlsdkLoadDomain(privconn, sdkdom, NULL); PrlHandle_Free(sdkdom); -- 2.1.0
On Mon, May 18, 2015 at 05:44:18PM +0300, Maxim Nestratov wrote:
Don't forget to unlock domain on error path
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 786e0ad..faae1ae 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1443,8 +1443,10 @@ prlsdkAddDomain(parallelsConnPtr privconn, const unsigned char *uuid) }
sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); - if (sdkdom == PRL_INVALID_HANDLE) + if (sdkdom == PRL_INVALID_HANDLE) { + virObjectUnlock(dom); return NULL; + }
dom = prlsdkLoadDomain(privconn, sdkdom, NULL); PrlHandle_Free(sdkdom);
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 05/18/2015 05:44 PM, Maxim Nestratov wrote:
Don't forget to unlock domain on error path
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 786e0ad..faae1ae 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1443,8 +1443,10 @@ prlsdkAddDomain(parallelsConnPtr privconn, const unsigned char *uuid) }
sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); - if (sdkdom == PRL_INVALID_HANDLE) + if (sdkdom == PRL_INVALID_HANDLE) { + virObjectUnlock(dom); return NULL; + }
dom = prlsdkLoadDomain(privconn, sdkdom, NULL); PrlHandle_Free(sdkdom);
Is it really needed? dom is always NULL on this error path. dom = virDomainObjListFindByUUID(privconn->domains, uuid); if (dom) { /* domain is already in the list */ return dom; } sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); if (sdkdom == PRL_INVALID_HANDLE) return NULL;
21.05.2015 9:50, Dmitry Guryanov пишет:
On 05/18/2015 05:44 PM, Maxim Nestratov wrote:
Don't forget to unlock domain on error path
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 786e0ad..faae1ae 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1443,8 +1443,10 @@ prlsdkAddDomain(parallelsConnPtr privconn, const unsigned char *uuid) } sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); - if (sdkdom == PRL_INVALID_HANDLE) + if (sdkdom == PRL_INVALID_HANDLE) { + virObjectUnlock(dom); return NULL; + } dom = prlsdkLoadDomain(privconn, sdkdom, NULL); PrlHandle_Free(sdkdom);
Is it really needed? dom is always NULL on this error path.
Ah, sure. You are right. This patch can be skipped
dom = virDomainObjListFindByUUID(privconn->domains, uuid); if (dom) { /* domain is already in the list */ return dom; }
sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); if (sdkdom == PRL_INVALID_HANDLE) return NULL;
It is better to get all necessary parameters and check them on newly created configuration before actually creating a domain with them or applying them to an existing domain. Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index faae1ae..5c15e94 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1312,6 +1312,9 @@ prlsdkLoadDomain(parallelsConnPtr privconn, *s = '\0'; } + pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart); + prlsdkCheckRetGoto(pret, error); + if (virDomainDefAddImplicitControllers(def) < 0) goto error; @@ -1349,15 +1352,6 @@ prlsdkLoadDomain(parallelsConnPtr privconn, dom->privateDataFreeFunc = prlsdkDomObjFreePrivate; dom->persistent = 1; - if (prlsdkGetDomainState(sdkdom, &domainState) < 0) - goto error; - - if (prlsdkConvertDomainState(domainState, envId, dom) < 0) - goto error; - - pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart); - prlsdkCheckRetGoto(pret, error); - switch (autostart) { case PAO_VM_START_ON_LOAD: dom->autostart = 1; @@ -1371,6 +1365,12 @@ prlsdkLoadDomain(parallelsConnPtr privconn, goto error; } + if (prlsdkGetDomainState(sdkdom, &domainState) < 0) + goto error; + + if (prlsdkConvertDomainState(domainState, envId, dom) < 0) + goto error; + if (!pdom->sdkdom) { pret = PrlHandle_AddRef(sdkdom); prlsdkCheckRetGoto(pret, error); -- 2.1.0
On Mon, May 18, 2015 at 05:44:19PM +0300, Maxim Nestratov wrote:
It is better to get all necessary parameters and check them on newly created configuration before actually creating a domain with them or applying them to an existing domain.
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 05/18/2015 05:44 PM, Maxim Nestratov wrote:
It is better to get all necessary parameters and check them on newly created configuration before actually creating a domain with them or applying them to an existing domain.
What problem could it cause?
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index faae1ae..5c15e94 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1312,6 +1312,9 @@ prlsdkLoadDomain(parallelsConnPtr privconn, *s = '\0'; }
+ pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart); + prlsdkCheckRetGoto(pret, error); + if (virDomainDefAddImplicitControllers(def) < 0) goto error;
@@ -1349,15 +1352,6 @@ prlsdkLoadDomain(parallelsConnPtr privconn, dom->privateDataFreeFunc = prlsdkDomObjFreePrivate; dom->persistent = 1;
- if (prlsdkGetDomainState(sdkdom, &domainState) < 0) - goto error; - - if (prlsdkConvertDomainState(domainState, envId, dom) < 0) - goto error; - - pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart); - prlsdkCheckRetGoto(pret, error); - switch (autostart) { case PAO_VM_START_ON_LOAD: dom->autostart = 1; @@ -1371,6 +1365,12 @@ prlsdkLoadDomain(parallelsConnPtr privconn, goto error; }
+ if (prlsdkGetDomainState(sdkdom, &domainState) < 0) + goto error; +
Why don't you move this ^^ call?
+ if (prlsdkConvertDomainState(domainState, envId, dom) < 0) + goto error; + if (!pdom->sdkdom) { pret = PrlHandle_AddRef(sdkdom); prlsdkCheckRetGoto(pret, error);
21.05.2015 9:55, Dmitry Guryanov пишет:
On 05/18/2015 05:44 PM, Maxim Nestratov wrote:
It is better to get all necessary parameters and check them on newly created configuration before actually creating a domain with them or applying them to an existing domain.
What problem could it cause?
First of all if we ask something from parallels dispatcher it takes time and doing such things with locked domain isn't a good practice. I am not saying that PrlVmCfg_GetAutoStart will go to dispatcher but other SDK functions will. What I wanted here is to group SDK calls together. Secondly, there can be a race when we call prlsdkLoadDomain function from domain creation event handler after the domain is deleted by concurrent extern call. So it's better to fail before we actually create a domain.
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index faae1ae..5c15e94 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1312,6 +1312,9 @@ prlsdkLoadDomain(parallelsConnPtr privconn, *s = '\0'; } + pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart); + prlsdkCheckRetGoto(pret, error); + if (virDomainDefAddImplicitControllers(def) < 0) goto error; @@ -1349,15 +1352,6 @@ prlsdkLoadDomain(parallelsConnPtr privconn, dom->privateDataFreeFunc = prlsdkDomObjFreePrivate; dom->persistent = 1; - if (prlsdkGetDomainState(sdkdom, &domainState) < 0) - goto error; - - if (prlsdkConvertDomainState(domainState, envId, dom) < 0) - goto error; - - pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart); - prlsdkCheckRetGoto(pret, error); - switch (autostart) { case PAO_VM_START_ON_LOAD: dom->autostart = 1; @@ -1371,6 +1365,12 @@ prlsdkLoadDomain(parallelsConnPtr privconn, goto error; } + if (prlsdkGetDomainState(sdkdom, &domainState) < 0) + goto error; +
Why don't you move this ^^ call?
Makes sence even more than PrlVmCfg_GetAutoStart. Just missed it. I'll add it in the second version of the patchset.
+ if (prlsdkConvertDomainState(domainState, envId, dom) < 0) + goto error; + if (!pdom->sdkdom) { pret = PrlHandle_AddRef(sdkdom); prlsdkCheckRetGoto(pret, error);
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. Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- 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; } -- 2.1.0
On Mon, May 18, 2015 at 05:44:20PM +0300, 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.
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
ACK, but will let Dmitry sanity check and push this series too. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
20.05.2015 15:32, Daniel P. Berrange пишет:
On Mon, May 18, 2015 at 05:44:20PM +0300, 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.
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) ACK, but will let Dmitry sanity check and push this series too.
Regards, Daniel Ok. Thank you Daniel.
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@parallels.com> --- 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; }
21.05.2015 10:04, Dmitry Guryanov пишет:
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.
I didn't get you. We already assign this function to dom->privateDataFreeFunc, and seems to the right place for this function.
You can give virDomainXMLPrivateDataCallbacks structure as second argument of virDomainXMLOptionNew, and it will create appropriate virDomainXMLOption structure.
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- 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; }
Sorry for top-posting, Check code of virDomainObjListAdd. It assigns xmlopt->privateData->free to dom->privateDataFreeFunc. ________________________________________ От: Maxim Nestratov Отправлено: 21 мая 2015 г. 10:17 Кому: Dmitry Guryanov; Maxim Nestratov; libvir-list@redhat.com; Dmitry Guryanov Тема: Re: [PATCH 3/3] parallels: fix possible crash in case of errors in prlsdkLoadDomain 21.05.2015 10:04, Dmitry Guryanov пишет:
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.
I didn't get you. We already assign this function to dom->privateDataFreeFunc, and seems to the right place for this function.
You can give virDomainXMLPrivateDataCallbacks structure as second argument of virDomainXMLOptionNew, and it will create appropriate virDomainXMLOption structure.
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- 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; }
21.05.2015 10:29, Dmitry Guryanov пишет:
Sorry for top-posting, Check code of virDomainObjListAdd. It assigns xmlopt->privateData->free to dom->privateDataFreeFunc. I don't see why we can't do it manually, while problem the patch fixes is a bit different. We create three objects within this function: 'def', 'privdom', and 'dom'. On error path depending on the place where we failed, we need to use different scenarios to cleanup: delete all we allocated and not to do it twice. The code without the patch frees 'def' and 'pdom 'structures twice. In other words, if we didn't manage to assign 'pdom' to 'dom' because we failed earlier we should call privateDataFree function manully, and if we didn't set 'def' to 'dom', either new or existed one, we should free it manually too. Otherwise cleanup will be made by virDomainObjListRemove. Using xmlopt can't fix described problems.
________________________________________ От: Maxim Nestratov Отправлено: 21 мая 2015 г. 10:17 Кому: Dmitry Guryanov; Maxim Nestratov; libvir-list@redhat.com; Dmitry Guryanov Тема: Re: [PATCH 3/3] parallels: fix possible crash in case of errors in prlsdkLoadDomain
21.05.2015 10:04, Dmitry Guryanov пишет:
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
On 05/18/2015 05:44 PM, Maxim Nestratov wrote: think you just need to set virDomainXMLOptionPtr->privateData->free to our prlsdkDomObjFreePrivate.
I didn't get you. We already assign this function to dom->privateDataFreeFunc, and seems to the right place for this function.
You can give virDomainXMLPrivateDataCallbacks structure as second argument of virDomainXMLOptionNew, and it will create appropriate virDomainXMLOption structure.
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- 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; }
participants (5)
-
Daniel P. Berrange -
Dmitry Guryanov -
Dmitry Guryanov -
Maxim Nestratov -
Maxim Nestratov