On 05/21/2015 04:49 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.

Signed-off-by: Maxim Nestratov <mnestratov@parallels.com>

Thanks, ACKED and pushed.

--- 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 786e0ad..4d4582f 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1312,6 +1312,12 @@ prlsdkLoadDomain(parallelsConnPtr privconn, *s = '\0'; }

+ pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart); + prlsdkCheckRetGoto(pret, error); + + if (prlsdkGetDomainState(sdkdom, &domainState) < 0) + goto error; + if (virDomainDefAddImplicitControllers(def) < 0) goto error;

@@ -1349,15 +1355,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 +1368,9 @@ prlsdkLoadDomain(parallelsConnPtr privconn, goto error; }

+ if (prlsdkConvertDomainState(domainState, envId, dom) < 0) + goto error; + if (!pdom->sdkdom) { pret = PrlHandle_AddRef(sdkdom); prlsdkCheckRetGoto(pret, error);

On 05/21/2015 04:49 PM, Maxim Nestratov wrote:

Cleanup code in prlsdkLoadDomain doesn't take into account the fact if private domain structure along with freeing function is assigned or not. In case it is, we shouldn't call it manually because virDomainObjListRemove calls it and frees pdom. 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.

OK, pushed, thanks.

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 4d4582f..c4ad4eb 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; }