[libvirt] [PATCH 0/4] vz: rework domain creation
Patch 1 and 2: Fix race condition when adding domain to domains list Race condition: User calls defineXML to create new instance. The main thread from vzDomainDefineXMLFlags() creates new instance by prlsdkCreateVm. Then this thread calls prlsdkAddDomain to add new domain to domains list. The second thread receives notification from hypervisor that new VM was created. It calls prlsdkHandleVmAddedEvent() and also tries to add new domain to domains list. These two threads call virDomainObjListFindByUUID() from prlsdkAddDomain() and don't find new domain. So they add two domains with the same uuid to domains list. This fix splits logic of prlsdkAddDomain() into two new functions. 1. prlsdkNewDomain() creates new empty domain in domains list with the specific uuid. 2. prlsdkSetDomainInstance() add data from VM to domain object. Patch 3 fixes notification subscription. To get domain info we should receive and handle notification from hypervisor. Mikhail Feoktistov (4): vz: Split logic of prlsdkAddDomain() into two functions vz: Remove prlsdkAddDomain() and use new functions vz: remove code duplication from LoadDomain() vz: fix notification subscription src/vz/vz_driver.c | 14 ++++- src/vz/vz_sdk.c | 156 ++++++++++++++++++++++++++++++----------------------- src/vz/vz_sdk.h | 8 ++- 3 files changed, 107 insertions(+), 71 deletions(-) -- 1.8.3.1
Add two functions 1. prlsdkNewDomain() creates new empty domain in domains list with the specific uuid. 2. prlsdkSetDomainInstance() add data from VM to domain object. --- src/vz/vz_sdk.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.h | 8 ++++++++ 2 files changed, 57 insertions(+) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index b78c413..c705517 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1509,6 +1509,55 @@ prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom) return retdom ? 0 : -1; } +int +prlsdkSetDomainInstance(vzConnPtr privconn, virDomainObjPtr dom, const unsigned char *uuid) +{ + PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; + virDomainObjPtr retdom; + + sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); + if (sdkdom == PRL_INVALID_HANDLE) + return -1; + + retdom = prlsdkLoadDomain(privconn, sdkdom, dom); + PrlHandle_Free(sdkdom); + return retdom ? 0 : -1; +} + +virDomainObjPtr +prlsdkNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid) +{ + virDomainDefPtr def = NULL; + virDomainObjPtr dom = NULL; + vzDomObjPtr pdom = NULL; + + if (!(def = virDomainDefNewFull(name, uuid, -1))) + goto error; + + if (VIR_ALLOC(pdom) < 0) + goto error; + + if (STREQ(privconn->drivername, "vz")) + def->virtType = VIR_DOMAIN_VIRT_VZ; + else + def->virtType = VIR_DOMAIN_VIRT_PARALLELS; + + if (!(dom = virDomainObjListAdd(privconn->domains, def, + privconn->xmlopt, + 0, NULL))) + goto error; + + dom->privateData = pdom; + dom->privateDataFreeFunc = prlsdkDomObjFreePrivate; + dom->persistent = 1; + return dom; + + error: + virDomainDefFree(def); + VIR_FREE(pdom); + return NULL; +} + static int prlsdkSendEvent(vzConnPtr privconn, virDomainObjPtr dom, virDomainEventType lvEventType, diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index ff6be07..060635e 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -32,6 +32,14 @@ int prlsdkLoadDomains(vzConnPtr privconn); virDomainObjPtr prlsdkAddDomain(vzConnPtr privconn, const unsigned char *uuid); +virDomainObjPtr +prlsdkNewDomain(vzConnPtr privconn, + char *name, + const unsigned char *uuid); +int +prlsdkSetDomainInstance(vzConnPtr privconn, + virDomainObjPtr dom, + const unsigned char *uuid); int prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom); int prlsdkSubscribeToPCSEvents(vzConnPtr privconn); void prlsdkUnsubscribeFromPCSEvents(vzConnPtr privconn); -- 1.8.3.1
On 23.01.2016 11:42, Mikhail Feoktistov wrote:
Add two functions 1. prlsdkNewDomain() creates new empty domain in domains list with the specific uuid. 2. prlsdkSetDomainInstance() add data from VM to domain object. --- src/vz/vz_sdk.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.h | 8 ++++++++ 2 files changed, 57 insertions(+)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index b78c413..c705517 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1509,6 +1509,55 @@ prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom) return retdom ? 0 : -1; }
+int +prlsdkSetDomainInstance(vzConnPtr privconn, virDomainObjPtr dom, const unsigned char *uuid) +{ + PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; + virDomainObjPtr retdom; + + sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); + if (sdkdom == PRL_INVALID_HANDLE) + return -1; + + retdom = prlsdkLoadDomain(privconn, sdkdom, dom); + PrlHandle_Free(sdkdom); + return retdom ? 0 : -1; +} I suggest name it prlsdkLoadDomainUUID where use pass uuid instead of handle. Another option is to just inline it. We use it only once. + +virDomainObjPtr +prlsdkNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid) +{ + virDomainDefPtr def = NULL; + virDomainObjPtr dom = NULL; + vzDomObjPtr pdom = NULL; + + if (!(def = virDomainDefNewFull(name, uuid, -1))) + goto error; + + if (VIR_ALLOC(pdom) < 0) + goto error; + + if (STREQ(privconn->drivername, "vz")) + def->virtType = VIR_DOMAIN_VIRT_VZ; + else + def->virtType = VIR_DOMAIN_VIRT_PARALLELS; + + if (!(dom = virDomainObjListAdd(privconn->domains, def, + privconn->xmlopt, + 0, NULL))) + goto error; + + dom->privateData = pdom; + dom->privateDataFreeFunc = prlsdkDomObjFreePrivate; + dom->persistent = 1; + return dom; + + error: + virDomainDefFree(def); + VIR_FREE(pdom); + return NULL; +} + static int prlsdkSendEvent(vzConnPtr privconn, virDomainObjPtr dom, virDomainEventType lvEventType, diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index ff6be07..060635e 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -32,6 +32,14 @@ int prlsdkLoadDomains(vzConnPtr privconn); virDomainObjPtr prlsdkAddDomain(vzConnPtr privconn, const unsigned char *uuid); +virDomainObjPtr +prlsdkNewDomain(vzConnPtr privconn, + char *name, + const unsigned char *uuid); +int +prlsdkSetDomainInstance(vzConnPtr privconn, + virDomainObjPtr dom, + const unsigned char *uuid); int prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom); int prlsdkSubscribeToPCSEvents(vzConnPtr privconn); void prlsdkUnsubscribeFromPCSEvents(vzConnPtr privconn);
Use prlsdkNewDomain() and prlsdkSetDomainInstance() instead of prlsdkAddDomain() New algorithm for creating an instance: In vzDomainDefineXMLFlags() we add new domain to domain list by calling prlsdkNewDomain() and only after that we call CreateVm() to create VM. It means that we "reserve" domain object with the specific uuid. After creation of new VM we add info from this VM to reserved domain object by calling sdkSetDomainInstance(). In notification handler prlsdkHandleVmAddedEvent() we check the existence of a domain and if it doesn't exist we add new domain by calling prlsdkLoadDomain(). --- src/vz/vz_driver.c | 14 ++++++++++++-- src/vz/vz_sdk.c | 38 +++++++++++++++----------------------- src/vz/vz_sdk.h | 2 -- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index f73f8ef..d1cf8d0 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -687,6 +687,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virDomainPtr retdom = NULL; virDomainDefPtr def; virDomainObjPtr olddom = NULL; + virDomainObjPtr newdom = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); @@ -702,6 +703,10 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) olddom = virDomainObjListFindByUUID(privconn->domains, def->uuid); if (olddom == NULL) { virResetLastError(); + newdom = prlsdkNewDomain(privconn, def->name, def->uuid); + if (!newdom) + goto cleanup; + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { if (prlsdkCreateVm(conn, def)) goto cleanup; @@ -715,8 +720,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; } - olddom = prlsdkAddDomain(privconn, def->uuid); - if (!olddom) + if (prlsdkSetDomainInstance(privconn, newdom, def->uuid)) goto cleanup; } else { int state, reason; @@ -757,6 +761,12 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) cleanup: if (olddom) virObjectUnlock(olddom); + if (newdom) { + if (!retdom) + virDomainObjListRemove(privconn->domains, newdom); + else + virObjectUnlock(newdom); + } virDomainDefFree(def); vzDriverUnlock(privconn); return retdom; diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index c705517..765f5f1 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1473,27 +1473,6 @@ prlsdkLoadDomains(vzConnPtr privconn) return -1; } -virDomainObjPtr -prlsdkAddDomain(vzConnPtr privconn, const unsigned char *uuid) -{ - PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; - virDomainObjPtr dom; - - 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; - - dom = prlsdkLoadDomain(privconn, sdkdom, NULL); - PrlHandle_Free(sdkdom); - return dom; -} - int prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom) { @@ -1672,11 +1651,24 @@ prlsdkHandleVmAddedEvent(vzConnPtr privconn, unsigned char *uuid) { virDomainObjPtr dom = NULL; + PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; - dom = prlsdkAddDomain(privconn, uuid); - if (dom == NULL) + dom = virDomainObjListFindByUUID(privconn->domains, uuid); + if (dom) { + /* domain is already in the list */ + goto send_event; + } + + sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); + if (sdkdom == PRL_INVALID_HANDLE) + return; + + dom = prlsdkLoadDomain(privconn, sdkdom, NULL); + PrlHandle_Free(sdkdom); + if (!dom) return; + send_event: prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_DEFINED, VIR_DOMAIN_EVENT_DEFINED_ADDED); diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index 060635e..19bce88 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -31,8 +31,6 @@ void prlsdkDisconnect(vzConnPtr privconn); int prlsdkLoadDomains(vzConnPtr privconn); virDomainObjPtr -prlsdkAddDomain(vzConnPtr privconn, const unsigned char *uuid); -virDomainObjPtr prlsdkNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid); -- 1.8.3.1
You use 'and' in description like this patch have to distinct purposes. Looks like a candidate for splitting. Say in first patch you use inroduced functions and thus fix a race. By the way this could be done in one step with introducing the functions. In second you refactor prlsdkHandleVmAddedEvent. But I don't like the way you do it much as you get non-cleanup goto in the new version. On 23.01.2016 11:42, Mikhail Feoktistov wrote:
Use prlsdkNewDomain() and prlsdkSetDomainInstance() instead of prlsdkAddDomain() New algorithm for creating an instance: In vzDomainDefineXMLFlags() we add new domain to domain list by calling prlsdkNewDomain() and only after that we call CreateVm() to create VM. It means that we "reserve" domain object with the specific uuid. After creation of new VM we add info from this VM to reserved domain object by calling sdkSetDomainInstance().
In notification handler prlsdkHandleVmAddedEvent() we check the existence of a domain and if it doesn't exist we add new domain by calling prlsdkLoadDomain(). --- src/vz/vz_driver.c | 14 ++++++++++++-- src/vz/vz_sdk.c | 38 +++++++++++++++----------------------- src/vz/vz_sdk.h | 2 -- 3 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index f73f8ef..d1cf8d0 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -687,6 +687,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virDomainPtr retdom = NULL; virDomainDefPtr def; virDomainObjPtr olddom = NULL; + virDomainObjPtr newdom = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); @@ -702,6 +703,10 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) olddom = virDomainObjListFindByUUID(privconn->domains, def->uuid); if (olddom == NULL) { virResetLastError(); + newdom = prlsdkNewDomain(privconn, def->name, def->uuid); + if (!newdom) + goto cleanup; + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { if (prlsdkCreateVm(conn, def)) goto cleanup; @@ -715,8 +720,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; }
- olddom = prlsdkAddDomain(privconn, def->uuid); - if (!olddom) + if (prlsdkSetDomainInstance(privconn, newdom, def->uuid)) goto cleanup; } else { int state, reason; @@ -757,6 +761,12 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) cleanup: if (olddom) virObjectUnlock(olddom); + if (newdom) { + if (!retdom) + virDomainObjListRemove(privconn->domains, newdom); + else + virObjectUnlock(newdom); + } virDomainDefFree(def); vzDriverUnlock(privconn); return retdom; diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index c705517..765f5f1 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1473,27 +1473,6 @@ prlsdkLoadDomains(vzConnPtr privconn) return -1; }
-virDomainObjPtr -prlsdkAddDomain(vzConnPtr privconn, const unsigned char *uuid) -{ - PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; - virDomainObjPtr dom; - - 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; - - dom = prlsdkLoadDomain(privconn, sdkdom, NULL); - PrlHandle_Free(sdkdom); - return dom; -} - int prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom) { @@ -1672,11 +1651,24 @@ prlsdkHandleVmAddedEvent(vzConnPtr privconn, unsigned char *uuid) { virDomainObjPtr dom = NULL; + PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
- dom = prlsdkAddDomain(privconn, uuid); - if (dom == NULL) + dom = virDomainObjListFindByUUID(privconn->domains, uuid); + if (dom) { + /* domain is already in the list */ + goto send_event; + } + + sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); + if (sdkdom == PRL_INVALID_HANDLE) + return; + + dom = prlsdkLoadDomain(privconn, sdkdom, NULL); + PrlHandle_Free(sdkdom); + if (!dom) return;
+ send_event: prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_DEFINED, VIR_DOMAIN_EVENT_DEFINED_ADDED);
diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index 060635e..19bce88 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -31,8 +31,6 @@ void prlsdkDisconnect(vzConnPtr privconn); int prlsdkLoadDomains(vzConnPtr privconn); virDomainObjPtr -prlsdkAddDomain(vzConnPtr privconn, const unsigned char *uuid); -virDomainObjPtr prlsdkNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid);
Now we create new domain by calling prlsdkNewDomain(). In LoadDomain() we just load info from vm instance to domain object. So remove code to create domain from LoadDomain(). --- src/vz/vz_sdk.c | 55 +++++++++++++++++-------------------------------------- 1 file changed, 17 insertions(+), 38 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 765f5f1..d9f2127 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1260,26 +1260,29 @@ prlsdkLoadDomain(vzConnPtr privconn, PRL_UINT32 ram; PRL_UINT32 envId; PRL_VM_AUTOSTART_OPTION autostart; + unsigned char uuid[VIR_UUID_BUFLEN]; + char *name; virCheckNonNullArgGoto(privconn, error); virCheckNonNullArgGoto(sdkdom, error); - if (!(def = virDomainDefNew())) + if (prlsdkGetDomainIds(sdkdom, &name, uuid) < 0) goto error; if (!olddom) { - if (VIR_ALLOC(pdom) < 0) - goto error; + dom = prlsdkNewDomain(privconn, name, uuid); + def = dom->def; + pdom = dom->privateData; } else { + if (!(def = virDomainDefNewFull(name, uuid, -1))) + goto error; pdom = olddom->privateData; + if (STREQ(privconn->drivername, "vz")) + def->virtType = VIR_DOMAIN_VIRT_VZ; + else + def->virtType = VIR_DOMAIN_VIRT_PARALLELS; } - - if (STREQ(privconn->drivername, "vz")) - def->virtType = VIR_DOMAIN_VIRT_VZ; - else - def->virtType = VIR_DOMAIN_VIRT_PARALLELS; - - def->id = -1; + VIR_FREE(name); /* we will remove this field in the near future, so let's set it * to NULL temporarily */ @@ -1292,9 +1295,6 @@ prlsdkLoadDomain(vzConnPtr privconn, goto error; } - if (prlsdkGetDomainIds(sdkdom, &def->name, def->uuid) < 0) - goto error; - def->onReboot = VIR_DOMAIN_LIFECYCLE_RESTART; def->onPoweroff = VIR_DOMAIN_LIFECYCLE_DESTROY; def->onCrash = VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY; @@ -1369,23 +1369,10 @@ prlsdkLoadDomain(vzConnPtr privconn, } if (olddom) { - /* assign new virDomainDef without any checks */ - /* we can't use virDomainObjAssignDef, because it checks - * for state and domain name */ dom = olddom; virDomainDefFree(dom->def); dom->def = def; - } else { - if (!(dom = virDomainObjListAdd(privconn->domains, def, - privconn->xmlopt, - 0, NULL))) - goto error; } - /* dom is locked here */ - - dom->privateData = pdom; - dom->privateDataFreeFunc = prlsdkDomObjFreePrivate; - dom->persistent = 1; switch (autostart) { case PAO_VM_START_ON_LOAD: @@ -1411,19 +1398,11 @@ prlsdkLoadDomain(vzConnPtr privconn, return dom; error: - 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); - + VIR_FREE(name); + if (!olddom && dom) virDomainObjListRemove(privconn->domains, dom); - } - /* 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) + + if (olddom && !dom) virDomainDefFree(def); return NULL; -- 1.8.3.1
prlsdkLoadDomain works in 2 different cases: 1. Called first time for a domain. Then it creates and initializes it. Then updates it from sdk handle. 2. Called when domain is already in list. In this case it updates domain from sdk handle. I think we could end up in a better series if we split this function into 2: first is to create and initialize, second update domain from sdk handle (load). On 23.01.2016 11:42, Mikhail Feoktistov wrote:
Now we create new domain by calling prlsdkNewDomain(). In LoadDomain() we just load info from vm instance to domain object. So remove code to create domain from LoadDomain().
--- src/vz/vz_sdk.c | 55 +++++++++++++++++-------------------------------------- 1 file changed, 17 insertions(+), 38 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 765f5f1..d9f2127 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1260,26 +1260,29 @@ prlsdkLoadDomain(vzConnPtr privconn, PRL_UINT32 ram; PRL_UINT32 envId; PRL_VM_AUTOSTART_OPTION autostart; + unsigned char uuid[VIR_UUID_BUFLEN]; + char *name;
virCheckNonNullArgGoto(privconn, error); virCheckNonNullArgGoto(sdkdom, error);
- if (!(def = virDomainDefNew())) + if (prlsdkGetDomainIds(sdkdom, &name, uuid) < 0) goto error;
if (!olddom) { - if (VIR_ALLOC(pdom) < 0) - goto error; + dom = prlsdkNewDomain(privconn, name, uuid); + def = dom->def; + pdom = dom->privateData; } else { + if (!(def = virDomainDefNewFull(name, uuid, -1))) + goto error; pdom = olddom->privateData; + if (STREQ(privconn->drivername, "vz")) + def->virtType = VIR_DOMAIN_VIRT_VZ; + else + def->virtType = VIR_DOMAIN_VIRT_PARALLELS; } - - if (STREQ(privconn->drivername, "vz")) - def->virtType = VIR_DOMAIN_VIRT_VZ; - else - def->virtType = VIR_DOMAIN_VIRT_PARALLELS; - - def->id = -1; + VIR_FREE(name);
/* we will remove this field in the near future, so let's set it * to NULL temporarily */ @@ -1292,9 +1295,6 @@ prlsdkLoadDomain(vzConnPtr privconn, goto error; }
- if (prlsdkGetDomainIds(sdkdom, &def->name, def->uuid) < 0) - goto error; - def->onReboot = VIR_DOMAIN_LIFECYCLE_RESTART; def->onPoweroff = VIR_DOMAIN_LIFECYCLE_DESTROY; def->onCrash = VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY; @@ -1369,23 +1369,10 @@ prlsdkLoadDomain(vzConnPtr privconn, }
if (olddom) { - /* assign new virDomainDef without any checks */ - /* we can't use virDomainObjAssignDef, because it checks - * for state and domain name */ dom = olddom; virDomainDefFree(dom->def); dom->def = def; - } else { - if (!(dom = virDomainObjListAdd(privconn->domains, def, - privconn->xmlopt, - 0, NULL))) - goto error; } - /* dom is locked here */ - - dom->privateData = pdom; - dom->privateDataFreeFunc = prlsdkDomObjFreePrivate; - dom->persistent = 1;
switch (autostart) { case PAO_VM_START_ON_LOAD: @@ -1411,19 +1398,11 @@ prlsdkLoadDomain(vzConnPtr privconn,
return dom; error: - 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); - + VIR_FREE(name); + if (!olddom && dom) virDomainObjListRemove(privconn->domains, dom); - } - /* 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) + + if (olddom && !dom) virDomainDefFree(def);
return NULL;
Bug cause: Update the domain that is subscribed to hypervisor notification. LoadDomain() rewrites notifications fields in vzDomObj structure and makes domain as "unsubscribed". Fix: Initialize notification fields in vzDomObj only if we create a new domain. And do not reinitialize these fields if we update domain (by calling LoadDomain with olddom argument) --- src/vz/vz_sdk.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index d9f2127..ef319b7 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1288,13 +1288,6 @@ prlsdkLoadDomain(vzConnPtr privconn, * to NULL temporarily */ pdom->uuid = NULL; - pdom->cache.stats = PRL_INVALID_HANDLE; - pdom->cache.count = -1; - if (virCondInit(&pdom->cache.cond) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize condition")); - goto error; - } - def->onReboot = VIR_DOMAIN_LIFECYCLE_RESTART; def->onPoweroff = VIR_DOMAIN_LIFECYCLE_DESTROY; def->onCrash = VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY; @@ -1495,6 +1488,13 @@ prlsdkNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid) if (VIR_ALLOC(pdom) < 0) goto error; + pdom->cache.stats = PRL_INVALID_HANDLE; + pdom->cache.count = -1; + if (virCondInit(&pdom->cache.cond) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize condition")); + goto error; + } + if (STREQ(privconn->drivername, "vz")) def->virtType = VIR_DOMAIN_VIRT_VZ; else @@ -1512,7 +1512,7 @@ prlsdkNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid) error: virDomainDefFree(def); - VIR_FREE(pdom); + prlsdkDomObjFreePrivate(pdom); return NULL; } -- 1.8.3.1
participants (2)
-
Mikhail Feoktistov -
Nikolay Shirokovskiy