[libvirt] [PATCH 6/9] vz: cleanup loading domain code
Nikolay Shirokovskiy
nshirokovskiy at virtuozzo.com
Tue Jun 14 08:45:57 UTC 2016
9c14a9ab introduced vzNewDomain function to enlist libvirt domain
object before actually creating vz sdk domain. Fix should fix
race on same vz sdk domain added event where libvirt domain object is
enlisted too. But later eb5e9c1e added locked checks for
adding livirtd domain object to list on vz sdk domain added event.
Thus now approach of 9c14a9ab is unnecessary complicated.
See we have otherwise unuseful prlsdkGetDomainIds function only
to create minimal domain definition to create libvirt domain object.
Also vzNewDomain is difficult to use as it creates partially
constructed domain object.
Let's move back to original approach where prlsdkLoadDomain do
all the necessary job. Another benefit is that we can now
take driver lock for bare minimum and in single place. Reducing
locking time have small disadvatage of double parsing on race
conditions which is typical if domain is added thru vz driver.
Well we have this double parse inevitably with current vz sdk api
on any domain updates so i would not take it here seriously.
Performance events subscribtion is done before locked check and
therefore could be done twice on races but this is not the problem.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
---
src/vz/vz_driver.c | 14 +---
src/vz/vz_sdk.c | 184 +++++++++++++++++++++++++----------------------------
src/vz/vz_sdk.h | 9 ++-
src/vz/vz_utils.c | 24 -------
src/vz/vz_utils.h | 4 --
5 files changed, 94 insertions(+), 141 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 8abf6f1..545dc79 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -733,7 +733,6 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
if (flags & VIR_DOMAIN_DEFINE_VALIDATE)
parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA;
- virObjectLock(driver);
if ((def = virDomainDefParseString(xml, driver->caps, driver->xmlopt,
parse_flags)) == NULL)
goto cleanup;
@@ -741,9 +740,6 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
olddom = virDomainObjListFindByUUID(driver->domains, def->uuid);
if (olddom == NULL) {
virResetLastError();
- newdom = vzNewDomain(driver, def->name, def->uuid);
- if (!newdom)
- goto cleanup;
if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
if (prlsdkCreateVm(driver, def))
goto cleanup;
@@ -757,7 +753,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
goto cleanup;
}
- if (prlsdkLoadDomain(driver, newdom))
+ if (!(newdom = prlsdkAddDomainByUUID(driver, def->uuid)))
goto cleanup;
} else {
int state, reason;
@@ -805,7 +801,6 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
virObjectUnlock(newdom);
}
virDomainDefFree(def);
- virObjectUnlock(driver);
return retdom;
}
@@ -2626,7 +2621,6 @@ vzDomainMigrateFinish3Params(virConnectPtr dconn,
vzConnPtr privconn = dconn->privateData;
vzDriverPtr driver = privconn->driver;
const char *name = NULL;
- PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
virCheckFlags(VZ_MIGRATION_FLAGS, NULL);
@@ -2656,11 +2650,8 @@ vzDomainMigrateFinish3Params(virConnectPtr dconn,
return NULL;
}
- sdkdom = prlsdkSdkDomainLookupByName(driver, name);
- if (sdkdom == PRL_INVALID_HANDLE)
- goto cleanup;
- if (!(dom = prlsdkNewDomainByHandle(driver, sdkdom)))
+ if (!(dom = prlsdkAddDomainByName(driver, name)))
goto cleanup;
domain = virGetDomain(dconn, dom->def->name, dom->def->uuid);
@@ -2674,7 +2665,6 @@ vzDomainMigrateFinish3Params(virConnectPtr dconn,
VIR_WARN("Can't provide domain '%s' after successfull migration.", name);
if (dom)
virObjectUnlock(dom);
- PrlHandle_Free(sdkdom);
return domain;
}
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index d1eb6f6..612a059 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -412,37 +412,6 @@ prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid)
}
static int
-prlsdkGetDomainIds(PRL_HANDLE sdkdom,
- char **name,
- unsigned char *uuid)
-{
- char uuidstr[VIR_UUID_STRING_BRACED_BUFLEN];
- PRL_RESULT pret;
-
- if (name && !(*name = prlsdkGetStringParamVar(PrlVmCfg_GetName, sdkdom)))
- goto error;
-
- if (uuid) {
- pret = prlsdkGetStringParamBuf(PrlVmCfg_GetUuid,
- sdkdom, uuidstr, sizeof(uuidstr));
- prlsdkCheckRetGoto(pret, error);
-
- if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Domain UUID is malformed or empty"));
- goto error;
- }
- }
-
- return 0;
-
- error:
- if (name)
- VIR_FREE(*name);
- return -1;
-}
-
-static int
prlsdkGetDomainState(PRL_HANDLE sdkdom, VIRTUAL_MACHINE_STATE_PTR vmState)
{
PRL_HANDLE job = PRL_INVALID_HANDLE;
@@ -1242,36 +1211,6 @@ prlsdkConvertCpuMode(PRL_HANDLE sdkdom, virDomainDefPtr def)
return -1;
}
-virDomainObjPtr
-prlsdkNewDomainByHandle(vzDriverPtr driver, PRL_HANDLE sdkdom)
-{
- virDomainObjPtr dom = NULL;
- unsigned char uuid[VIR_UUID_BUFLEN];
- char *name = NULL;
-
- virObjectLock(driver);
- if (prlsdkGetDomainIds(sdkdom, &name, uuid) < 0)
- goto cleanup;
-
- /* we should make sure that there is no such a VM exists */
- if ((dom = virDomainObjListFindByUUID(driver->domains, uuid)))
- goto cleanup;
-
- if (!(dom = vzNewDomain(driver, name, uuid)))
- goto cleanup;
-
- if (prlsdkLoadDomain(driver, dom) < 0) {
- virDomainObjListRemove(driver->domains, dom);
- dom = NULL;
- goto cleanup;
- }
-
- cleanup:
- virObjectUnlock(driver);
- VIR_FREE(name);
- return dom;
-}
-
static PRL_HANDLE
prlsdkGetDevByDevIndex(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE type, PRL_UINT32 devIndex)
{
@@ -1495,8 +1434,16 @@ prlsdkConvertBootOrder(PRL_HANDLE sdkdom, virDomainDefPtr def)
return ret;
}
-int
-prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom)
+/* if dom is NULL adds new domain into domain list
+ * if dom not NULL updates given locked dom object.
+ *
+ * Returned object is locked.
+ */
+
+static virDomainObjPtr
+prlsdkLoadDomain(vzDriverPtr driver,
+ PRL_HANDLE sdkdom,
+ virDomainObjPtr dom)
{
virDomainDefPtr def = NULL;
vzDomObjPtr pdom = NULL;
@@ -1506,24 +1453,26 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom)
PRL_UINT32 ram;
PRL_UINT32 envId;
PRL_VM_AUTOSTART_OPTION autostart;
- PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
PRL_HANDLE job;
-
- virCheckNonNullArgGoto(dom, error);
-
- pdom = dom->privateData;
- sdkdom = prlsdkSdkDomainLookupByUUID(driver, dom->def->uuid);
- if (sdkdom == PRL_INVALID_HANDLE)
- return -1;
+ char uuidstr[VIR_UUID_STRING_BRACED_BUFLEN];
if (!(def = virDomainDefNew()))
goto error;
- def->virtType = dom->def->virtType;
- def->id = dom->def->id;
+ if (!(def->name = prlsdkGetStringParamVar(PrlVmCfg_GetName, sdkdom)))
+ goto error;
+
+ pret = prlsdkGetStringParamBuf(PrlVmCfg_GetUuid,
+ sdkdom, uuidstr, sizeof(uuidstr));
+ prlsdkCheckRetGoto(pret, error);
- if (prlsdkGetDomainIds(sdkdom, &def->name, def->uuid) < 0)
+ if (prlsdkUUIDParse(uuidstr, def->uuid) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Domain UUID is malformed or empty"));
goto error;
+ }
+
+ def->virtType = VIR_DOMAIN_VIRT_VZ;
def->onReboot = VIR_DOMAIN_LIFECYCLE_RESTART;
def->onPoweroff = VIR_DOMAIN_LIFECYCLE_DESTROY;
@@ -1588,20 +1537,38 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom)
goto error;
}
- if (!pdom->sdkdom) {
+ if (!dom) {
+ virDomainObjPtr olddom = NULL;
+
job = PrlVm_SubscribeToPerfStats(sdkdom, NULL);
if (PRL_FAILED(waitJob(job)))
goto error;
- PrlHandle_AddRef(sdkdom);
+ virObjectLock(driver);
+ if (!(olddom = virDomainObjListFindByUUID(driver->domains, def->uuid)))
+ dom = virDomainObjListAdd(driver->domains, def, driver->xmlopt, 0, NULL);
+ virObjectUnlock(driver);
+
+ if (olddom) {
+ virDomainDefFree(def);
+ return olddom;
+ } else if (!dom) {
+ goto error;
+ }
+
+ pdom = dom->privateData;
pdom->sdkdom = sdkdom;
+ PrlHandle_AddRef(sdkdom);
+ dom->persistent = 1;
+ } else {
+ /* assign new virDomainDef without any checks
+ * we can't use virDomainObjAssignDef, because it checks
+ * for state and domain name */
+ virDomainDefFree(dom->def);
+ dom->def = def;
}
- /* assign new virDomainDef without any checks
- * we can't use virDomainObjAssignDef, because it checks
- * for state and domain name */
- virDomainDefFree(dom->def);
- dom->def = def;
+ pdom = dom->privateData;
pdom->id = envId;
prlsdkConvertDomainState(domainState, envId, dom);
@@ -1611,12 +1578,11 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom)
else
dom->autostart = 0;
- PrlHandle_Free(sdkdom);
- return 0;
+ return dom;
+
error:
- PrlHandle_Free(sdkdom);
virDomainDefFree(def);
- return -1;
+ return NULL;
}
int
@@ -1642,7 +1608,7 @@ prlsdkLoadDomains(vzDriverPtr driver)
pret = PrlResult_GetParamByIndex(result, i, &sdkdom);
prlsdkCheckRetGoto(pret, error);
- if ((dom = prlsdkNewDomainByHandle(driver, sdkdom)))
+ if ((dom = prlsdkLoadDomain(driver, sdkdom, NULL)))
virObjectUnlock(dom);
PrlHandle_Free(sdkdom);
@@ -1658,6 +1624,38 @@ prlsdkLoadDomains(vzDriverPtr driver)
return -1;
}
+virDomainObjPtr
+prlsdkAddDomainByUUID(vzDriverPtr driver, const unsigned char *uuid)
+{
+ PRL_HANDLE sdkdom;
+ virDomainObjPtr dom;
+
+ sdkdom = prlsdkSdkDomainLookupByUUID(driver, uuid);
+ if (sdkdom == PRL_INVALID_HANDLE)
+ return NULL;
+
+ dom = prlsdkLoadDomain(driver, sdkdom, NULL);
+
+ PrlHandle_Free(sdkdom);
+ return dom;
+}
+
+virDomainObjPtr
+prlsdkAddDomainByName(vzDriverPtr driver, const char *name)
+{
+ PRL_HANDLE sdkdom;
+ virDomainObjPtr dom;
+
+ sdkdom = prlsdkSdkDomainLookupByName(driver, name);
+ if (sdkdom == PRL_INVALID_HANDLE)
+ return NULL;
+
+ dom = prlsdkLoadDomain(driver, sdkdom, NULL);
+
+ PrlHandle_Free(sdkdom);
+ return dom;
+}
+
int
prlsdkUpdateDomain(vzDriverPtr driver, virDomainObjPtr dom)
{
@@ -1668,7 +1666,7 @@ prlsdkUpdateDomain(vzDriverPtr driver, virDomainObjPtr dom)
if (waitJob(job))
return -1;
- return prlsdkLoadDomain(driver, dom);
+ return prlsdkLoadDomain(driver, pdom->sdkdom, dom) ? 0 : -1;
}
static int prlsdkSendEvent(vzDriverPtr driver,
@@ -1787,15 +1785,9 @@ prlsdkHandleVmAddedEvent(vzDriverPtr driver,
virDomainObjPtr dom = NULL;
PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
- dom = virDomainObjListFindByUUID(driver->domains, uuid);
- if (!dom) {
- sdkdom = prlsdkSdkDomainLookupByUUID(driver, uuid);
- if (sdkdom == PRL_INVALID_HANDLE)
- goto cleanup;
-
- if (!(dom = prlsdkNewDomainByHandle(driver, sdkdom)))
- goto cleanup;
- }
+ if (!(dom = virDomainObjListFindByUUID(driver->domains, uuid)) &&
+ !(dom = prlsdkAddDomainByUUID(driver, uuid)))
+ goto cleanup;
prlsdkSendEvent(driver, 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 9d143ce..e32001a 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -30,10 +30,11 @@ int prlsdkConnect(vzDriverPtr driver);
void prlsdkDisconnect(vzDriverPtr driver);
int
prlsdkLoadDomains(vzDriverPtr driver);
+virDomainObjPtr
+prlsdkAddDomainByUUID(vzDriverPtr driver, const unsigned char *uuid);
+virDomainObjPtr
+prlsdkAddDomainByName(vzDriverPtr driver, const char *name);
int prlsdkUpdateDomain(vzDriverPtr driver, virDomainObjPtr dom);
-int
-prlsdkLoadDomain(vzDriverPtr driver,
- virDomainObjPtr dom);
int prlsdkSubscribeToPCSEvents(vzDriverPtr driver);
void prlsdkUnsubscribeFromPCSEvents(vzDriverPtr driver);
PRL_RESULT prlsdkStart(PRL_HANDLE sdkdom);
@@ -97,5 +98,3 @@ prlsdkMigrate(virDomainObjPtr dom,
PRL_HANDLE
prlsdkSdkDomainLookupByName(vzDriverPtr driver, const char *name);
-virDomainObjPtr
-prlsdkNewDomainByHandle(vzDriverPtr driver, PRL_HANDLE sdkdom);
diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c
index eef9db2..b629bc3 100644
--- a/src/vz/vz_utils.c
+++ b/src/vz/vz_utils.c
@@ -159,30 +159,6 @@ vzGetOutput(const char *binary, ...)
return outbuf;
}
-virDomainObjPtr
-vzNewDomain(vzDriverPtr driver, const char *name, const unsigned char *uuid)
-{
- virDomainDefPtr def = NULL;
- virDomainObjPtr dom = NULL;
-
- if (!(def = virDomainDefNewFull(name, uuid, -1)))
- goto error;
-
- def->virtType = VIR_DOMAIN_VIRT_VZ;
-
- if (!(dom = virDomainObjListAdd(driver->domains, def,
- driver->xmlopt,
- 0, NULL)))
- goto error;
-
- dom->persistent = 1;
- return dom;
-
- error:
- virDomainDefFree(def);
- return NULL;
-}
-
static void
vzInitCaps(unsigned long vzVersion, vzCapabilitiesPtr vzCaps)
{
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index 9069340..20ba1fd 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -117,10 +117,6 @@ vzGetDriverConnection(void);
void
vzDestroyDriverConnection(void);
-virDomainObjPtr
-vzNewDomain(vzDriverPtr driver,
- const char *name,
- const unsigned char *uuid);
int
vzInitVersion(vzDriverPtr driver);
int
--
1.8.3.1
More information about the libvir-list
mailing list