mirror of https://gitee.com/openkylin/libvirt.git
vz: 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 functions. 1. vzNewDomain() creates new empty domain in domains list with the specific uuid. 2. prlsdkLoadDomain() add data from VM to domain object. New algorithm for creating an instance: In vzDomainDefineXMLFlags() we add new domain to domain list by calling vzNewDomain() 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 prlsdkLoadDomain(). Before this patch prlsdkLoadDomain() worked in 2 different cases: 1. It creates and initializes new domain. Then updates it from sdk handle. 2. It updates existed domain from sdk handle. In this patch we remove code which creates new domain from LoadDomain() and move it to vzNewDomain(). Now prlsdkLoadDomain() only updates domain from skd handle. In notification handler prlsdkHandleVmAddedEvent() we check the existence of a domain and if it doesn't exist we add new domain by calling vzNewDomain() and load info from sdk handle via prlsdkLoadDomain().
This commit is contained in:
Mikhail Feoktistov
Maxim Nestratov
parent
d5f0cf990b
commit
9c14a9ab42
|
|
@ -688,6 +688,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);
|
||||
|
|
@ -703,6 +704,9 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
|
|||
olddom = virDomainObjListFindByUUID(privconn->domains, def->uuid);
|
||||
if (olddom == NULL) {
|
||||
virResetLastError();
|
||||
newdom = vzNewDomain(privconn, def->name, def->uuid);
|
||||
if (!newdom)
|
||||
goto cleanup;
|
||||
if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
|
||||
if (prlsdkCreateVm(conn, def))
|
||||
goto cleanup;
|
||||
|
|
@ -716,8 +720,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
|
|||
goto cleanup;
|
||||
}
|
||||
|
||||
olddom = prlsdkAddDomain(privconn, def->uuid);
|
||||
if (!olddom)
|
||||
if (prlsdkLoadDomain(privconn, newdom))
|
||||
goto cleanup;
|
||||
} else {
|
||||
int state, reason;
|
||||
|
|
@ -758,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;
|
||||
|
|
|
|||
219
src/vz/vz_sdk.c
219
src/vz/vz_sdk.c
|
|
@ -408,7 +408,7 @@ prlsdkGetDomainState(PRL_HANDLE sdkdom, VIRTUAL_MACHINE_STATE_PTR vmState)
|
|||
return ret;
|
||||
}
|
||||
|
||||
static void
|
||||
void
|
||||
prlsdkDomObjFreePrivate(void *p)
|
||||
{
|
||||
vzDomObjPtr pdom = p;
|
||||
|
|
@ -1241,56 +1241,58 @@ prlsdkConvertCpuMode(PRL_HANDLE sdkdom, virDomainDefPtr def)
|
|||
return -1;
|
||||
}
|
||||
|
||||
/*
|
||||
* This function retrieves information about domain.
|
||||
* If the domains is already in the domains list
|
||||
* privconn->domains, then locked 'olddom' must be
|
||||
* provided. If the domains must be added to the list,
|
||||
* olddom must be NULL.
|
||||
*
|
||||
* The function return a pointer to a locked virDomainObj.
|
||||
*/
|
||||
static virDomainObjPtr
|
||||
prlsdkLoadDomain(vzConnPtr privconn,
|
||||
PRL_HANDLE sdkdom,
|
||||
virDomainObjPtr olddom)
|
||||
prlsdkNewDomainByHandle(vzConnPtr privconn, PRL_HANDLE sdkdom)
|
||||
{
|
||||
virDomainObjPtr dom = NULL;
|
||||
unsigned char uuid[VIR_UUID_BUFLEN];
|
||||
char *name = NULL;
|
||||
|
||||
if (prlsdkGetDomainIds(sdkdom, &name, uuid) < 0)
|
||||
goto cleanup;
|
||||
|
||||
if (!(dom = vzNewDomain(privconn, name, uuid)))
|
||||
goto cleanup;
|
||||
|
||||
if (prlsdkLoadDomain(privconn, dom) < 0) {
|
||||
virDomainObjListRemove(privconn->domains, dom);
|
||||
dom = NULL;
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
cleanup:
|
||||
VIR_FREE(name);
|
||||
return dom;
|
||||
}
|
||||
|
||||
int
|
||||
prlsdkLoadDomain(vzConnPtr privconn, virDomainObjPtr dom)
|
||||
{
|
||||
virDomainDefPtr def = NULL;
|
||||
vzDomObjPtr pdom = NULL;
|
||||
VIRTUAL_MACHINE_STATE domainState;
|
||||
char *home = NULL;
|
||||
|
||||
PRL_UINT32 buflen = 0;
|
||||
PRL_RESULT pret;
|
||||
PRL_UINT32 ram;
|
||||
PRL_UINT32 envId;
|
||||
PRL_VM_AUTOSTART_OPTION autostart;
|
||||
PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
|
||||
|
||||
virCheckNonNullArgGoto(privconn, error);
|
||||
virCheckNonNullArgGoto(sdkdom, error);
|
||||
virCheckNonNullArgGoto(dom, error);
|
||||
|
||||
pdom = dom->privateData;
|
||||
sdkdom = prlsdkSdkDomainLookupByUUID(privconn, dom->def->uuid);
|
||||
if (sdkdom == PRL_INVALID_HANDLE)
|
||||
return -1;
|
||||
|
||||
if (!(def = virDomainDefNew()))
|
||||
goto error;
|
||||
|
||||
if (!olddom) {
|
||||
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;
|
||||
}
|
||||
} else {
|
||||
pdom = olddom->privateData;
|
||||
}
|
||||
|
||||
if (STREQ(privconn->drivername, "vz"))
|
||||
def->virtType = VIR_DOMAIN_VIRT_VZ;
|
||||
else
|
||||
def->virtType = VIR_DOMAIN_VIRT_PARALLELS;
|
||||
|
||||
def->id = -1;
|
||||
def->virtType = dom->def->virtType;
|
||||
def->id = dom->def->id;
|
||||
|
||||
if (prlsdkGetDomainIds(sdkdom, &def->name, def->uuid) < 0)
|
||||
goto error;
|
||||
|
|
@ -1323,29 +1325,33 @@ prlsdkLoadDomain(vzConnPtr privconn,
|
|||
|
||||
pret = PrlVmCfg_GetEnvId(sdkdom, &envId);
|
||||
prlsdkCheckRetGoto(pret, error);
|
||||
pdom->id = envId;
|
||||
|
||||
buflen = 0;
|
||||
pret = PrlVmCfg_GetHomePath(sdkdom, NULL, &buflen);
|
||||
prlsdkCheckRetGoto(pret, error);
|
||||
|
||||
VIR_FREE(pdom->home);
|
||||
if (VIR_ALLOC_N(pdom->home, buflen) < 0)
|
||||
if (VIR_ALLOC_N(home, buflen) < 0)
|
||||
goto error;
|
||||
|
||||
pret = PrlVmCfg_GetHomePath(sdkdom, pdom->home, &buflen);
|
||||
pret = PrlVmCfg_GetHomePath(sdkdom, home, &buflen);
|
||||
prlsdkCheckRetGoto(pret, error);
|
||||
|
||||
/* For VMs pdom->home is actually /directory/config.pvs */
|
||||
/* For VMs home is actually /directory/config.pvs */
|
||||
if (!IS_CT(def)) {
|
||||
/* Get rid of /config.pvs in path string */
|
||||
char *s = strrchr(pdom->home, '/');
|
||||
char *s = strrchr(home, '/');
|
||||
if (s)
|
||||
*s = '\0';
|
||||
}
|
||||
|
||||
pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart);
|
||||
prlsdkCheckRetGoto(pret, error);
|
||||
if (autostart != PAO_VM_START_ON_LOAD &&
|
||||
autostart != PAO_VM_START_MANUAL) {
|
||||
virReportError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Unknown autostart mode: %X"), autostart);
|
||||
goto error;
|
||||
}
|
||||
|
||||
if (prlsdkGetDomainState(sdkdom, &domainState) < 0)
|
||||
goto error;
|
||||
|
|
@ -1368,65 +1374,35 @@ prlsdkLoadDomain(vzConnPtr privconn,
|
|||
goto error;
|
||||
}
|
||||
|
||||
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:
|
||||
dom->autostart = 1;
|
||||
break;
|
||||
case PAO_VM_START_MANUAL:
|
||||
dom->autostart = 0;
|
||||
break;
|
||||
default:
|
||||
virReportError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Unknown autostart mode: %X"), autostart);
|
||||
goto error;
|
||||
}
|
||||
|
||||
if (prlsdkConvertDomainState(domainState, envId, dom) < 0)
|
||||
goto error;
|
||||
|
||||
/* 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->id = envId;
|
||||
VIR_FREE(pdom->home);
|
||||
pdom->home = home;
|
||||
|
||||
if (!pdom->sdkdom) {
|
||||
pret = PrlHandle_AddRef(sdkdom);
|
||||
prlsdkCheckRetGoto(pret, error);
|
||||
PrlHandle_AddRef(sdkdom);
|
||||
pdom->sdkdom = sdkdom;
|
||||
}
|
||||
|
||||
return dom;
|
||||
if (autostart == PAO_VM_START_ON_LOAD)
|
||||
dom->autostart = 1;
|
||||
else
|
||||
dom->autostart = 0;
|
||||
|
||||
PrlHandle_Free(sdkdom);
|
||||
return 0;
|
||||
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);
|
||||
|
||||
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)
|
||||
virDomainDefFree(def);
|
||||
|
||||
return NULL;
|
||||
PrlHandle_Free(sdkdom);
|
||||
VIR_FREE(home);
|
||||
virDomainDefFree(def);
|
||||
return -1;
|
||||
}
|
||||
|
||||
int
|
||||
|
|
@ -1434,7 +1410,7 @@ prlsdkLoadDomains(vzConnPtr privconn)
|
|||
{
|
||||
PRL_HANDLE job = PRL_INVALID_HANDLE;
|
||||
PRL_HANDLE result;
|
||||
PRL_HANDLE sdkdom;
|
||||
PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
|
||||
PRL_UINT32 paramsCount;
|
||||
PRL_RESULT pret;
|
||||
size_t i = 0;
|
||||
|
|
@ -1450,63 +1426,36 @@ prlsdkLoadDomains(vzConnPtr privconn)
|
|||
|
||||
for (i = 0; i < paramsCount; i++) {
|
||||
pret = PrlResult_GetParamByIndex(result, i, &sdkdom);
|
||||
if (PRL_FAILED(pret)) {
|
||||
logPrlError(pret);
|
||||
PrlHandle_Free(sdkdom);
|
||||
goto error;
|
||||
}
|
||||
prlsdkCheckRetGoto(pret, error);
|
||||
|
||||
dom = prlsdkLoadDomain(privconn, sdkdom, NULL);
|
||||
if (!(dom = prlsdkNewDomainByHandle(privconn, sdkdom)))
|
||||
goto error;
|
||||
|
||||
virObjectUnlock(dom);
|
||||
PrlHandle_Free(sdkdom);
|
||||
|
||||
if (!dom)
|
||||
goto error;
|
||||
else
|
||||
virObjectUnlock(dom);
|
||||
sdkdom = PRL_INVALID_HANDLE;
|
||||
}
|
||||
|
||||
PrlHandle_Free(result);
|
||||
return 0;
|
||||
|
||||
error:
|
||||
PrlHandle_Free(sdkdom);
|
||||
PrlHandle_Free(result);
|
||||
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)
|
||||
{
|
||||
PRL_HANDLE job;
|
||||
virDomainObjPtr retdom = NULL;
|
||||
vzDomObjPtr pdom = dom->privateData;
|
||||
|
||||
job = PrlVm_RefreshConfig(pdom->sdkdom);
|
||||
if (waitJob(job))
|
||||
return -1;
|
||||
|
||||
retdom = prlsdkLoadDomain(privconn, pdom->sdkdom, dom);
|
||||
return retdom ? 0 : -1;
|
||||
return prlsdkLoadDomain(privconn, dom);
|
||||
}
|
||||
|
||||
static int prlsdkSendEvent(vzConnPtr privconn,
|
||||
|
|
@ -1623,15 +1572,25 @@ prlsdkHandleVmAddedEvent(vzConnPtr privconn,
|
|||
unsigned char *uuid)
|
||||
{
|
||||
virDomainObjPtr dom = NULL;
|
||||
PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
|
||||
|
||||
dom = prlsdkAddDomain(privconn, uuid);
|
||||
if (dom == NULL)
|
||||
return;
|
||||
dom = virDomainObjListFindByUUID(privconn->domains, uuid);
|
||||
if (!dom) {
|
||||
sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid);
|
||||
if (sdkdom == PRL_INVALID_HANDLE)
|
||||
goto cleanup;
|
||||
|
||||
if (!(dom = prlsdkNewDomainByHandle(privconn, sdkdom)))
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_DEFINED,
|
||||
VIR_DOMAIN_EVENT_DEFINED_ADDED);
|
||||
|
||||
virObjectUnlock(dom);
|
||||
cleanup:
|
||||
if (dom)
|
||||
virObjectUnlock(dom);
|
||||
PrlHandle_Free(sdkdom);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -30,9 +30,10 @@ int prlsdkConnect(vzConnPtr privconn);
|
|||
void prlsdkDisconnect(vzConnPtr privconn);
|
||||
int
|
||||
prlsdkLoadDomains(vzConnPtr privconn);
|
||||
virDomainObjPtr
|
||||
prlsdkAddDomain(vzConnPtr privconn, const unsigned char *uuid);
|
||||
int prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom);
|
||||
int
|
||||
prlsdkLoadDomain(vzConnPtr privconn,
|
||||
virDomainObjPtr dom);
|
||||
int prlsdkSubscribeToPCSEvents(vzConnPtr privconn);
|
||||
void prlsdkUnsubscribeFromPCSEvents(vzConnPtr privconn);
|
||||
PRL_RESULT prlsdkStart(PRL_HANDLE sdkdom);
|
||||
|
|
@ -77,3 +78,5 @@ int
|
|||
prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *time);
|
||||
int
|
||||
prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats, unsigned int nr_stats);
|
||||
void
|
||||
prlsdkDomObjFreePrivate(void *p);
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@
|
|||
#include "viralloc.h"
|
||||
#include "virjson.h"
|
||||
#include "vz_utils.h"
|
||||
#include "vz_sdk.h"
|
||||
#include "virstring.h"
|
||||
#include "datatypes.h"
|
||||
|
||||
|
|
@ -134,3 +135,46 @@ vzGetOutput(const char *binary, ...)
|
|||
|
||||
return outbuf;
|
||||
}
|
||||
|
||||
virDomainObjPtr
|
||||
vzNewDomain(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 (virCondInit(&pdom->cache.cond) < 0) {
|
||||
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize condition"));
|
||||
goto error;
|
||||
}
|
||||
pdom->cache.stats = PRL_INVALID_HANDLE;
|
||||
pdom->cache.count = -1;
|
||||
|
||||
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:
|
||||
if (pdom && pdom->cache.count == -1)
|
||||
virCondDestroy(&pdom->cache.cond);
|
||||
virDomainDefFree(def);
|
||||
VIR_FREE(pdom);
|
||||
return NULL;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -90,6 +90,10 @@ char * vzGetOutput(const char *binary, ...)
|
|||
ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL;
|
||||
void vzDriverLock(vzConnPtr driver);
|
||||
void vzDriverUnlock(vzConnPtr driver);
|
||||
virDomainObjPtr
|
||||
vzNewDomain(vzConnPtr privconn,
|
||||
char *name,
|
||||
const unsigned char *uuid);
|
||||
|
||||
# define PARALLELS_BLOCK_STATS_FOREACH(OP) \
|
||||
OP(rd_req, VIR_DOMAIN_BLOCK_STATS_READ_REQ, "read_requests") \
|
||||
|
|
|
|||
Loading…
Reference in New Issue