[libvirt] [PATCH 0/6] parallels: get rid of cached domains list in driver
The goal of this patch series is to get rid of cached domains list. Someone can create several connections to libvirt or change something (start VM or change parameters) using native tools. In such case first connection will not get any info about those changes. There is no way to check, if the cache valid, so remove it and always query needed data using prlctl command. Dmitry Guryanov (6): parallels: return a new list of domains from parallelsLoadDomains parallels: rename uuidstr to __uuidstr in macro parallels: return up-to-date info about domains parallels: don't track domain state in global domains list parallels: don't use stored domains list in parallelsDomainDefineXML parallels: remove domains list from _parallelsConn structure src/parallels/parallels_driver.c | 423 ++++++++++++++++++-------------------- src/parallels/parallels_utils.h | 1 - 2 files changed, 195 insertions(+), 229 deletions(-)
Make parallelsLoadDomains more useful, so that it will not touch list in global _parallelsConn structure, but allocate and return a new one instead. So it can be used later, when you need the up-to-date list of VMs instead of the one stored in _parallelsConn structure. Also rename parallelsLoadDomains to parallelsGetDomains and add new function parallelsGetDomain, which is used in parallelsDomainDefineXML. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_driver.c | 134 +++++++++++++++++++++++++------------- src/parallels/parallels_utils.h | 2 +- 2 files changed, 90 insertions(+), 46 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index c14f616..794d61d 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -401,11 +401,8 @@ parallelsAddVNCInfo(virDomainDefPtr def, virJSONValuePtr jobj_root) return ret; } -/* - * Must be called with privconn->lock held - */ static virDomainObjPtr -parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) +parallelsParseDomainInfo(parallelsConnPtr privconn, virJSONValuePtr jobj) { virDomainObjPtr dom = NULL; virDomainDefPtr def = NULL; @@ -528,11 +525,12 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) if (parallelsAddVNCInfo(def, jobj) < 0) goto cleanup; - if (!(dom = virDomainAssignDef(privconn->caps, - &privconn->domains, def, false))) + if (!(dom = virDomainObjNew(privconn->caps))) goto cleanup; /* dom is locked here */ + dom->def = def; + dom->privateDataFreeFunc = parallelsDomObjFreePrivate; dom->privateData = pdom; dom->persistent = 1; @@ -562,22 +560,21 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) } /* - * Must be called with privconn->lock held - * * if domain_name is NULL - load information about all * registered domains. */ -static int -parallelsLoadDomains(parallelsConnPtr privconn, const char *domain_name) +static virDomainObjListPtr +parallelsGetDomains(parallelsConnPtr privconn) { int count, i; virJSONValuePtr jobj; virJSONValuePtr jobj2; virDomainObjPtr dom = NULL; - int ret = -1; + virDomainObjListPtr domains = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; jobj = parallelsParseOutput(PRLCTL, "list", "-j", "-a", "-i", "-H", - "--vmtype", "vm", domain_name, NULL); + "--vmtype", "vm", NULL); if (!jobj) { parallelsParseError(); goto cleanup; @@ -589,6 +586,11 @@ parallelsLoadDomains(parallelsConnPtr privconn, const char *domain_name) goto cleanup; } + if (VIR_ALLOC(domains) < 0) + goto error; + if (virDomainObjListInit(domains) < 0) + goto error; + for (i = 0; i < count; i++) { jobj2 = virJSONValueArrayGet(jobj, i); if (!jobj2) { @@ -596,16 +598,59 @@ parallelsLoadDomains(parallelsConnPtr privconn, const char *domain_name) goto cleanup; } - dom = parallelsLoadDomain(privconn, jobj2); + dom = parallelsParseDomainInfo(privconn, jobj2); if (!dom) goto cleanup; + + virUUIDFormat(dom->def->uuid, uuidstr); + if (virHashAddEntry(domains->objs, uuidstr, dom) < 0) { + VIR_FREE(dom); + goto error; + } } - ret = 0; + cleanup: + virJSONValueFree(jobj); + return domains; + error: + virJSONValueFree(jobj); + virDomainObjListDeinit(domains); + VIR_FREE(domains); + return NULL; +} + +static virDomainObjPtr +parallelsGetDomain(parallelsConnPtr privconn, const char *domain_name) +{ + int count; + virJSONValuePtr jobj; + virJSONValuePtr jobj2; + virDomainObjPtr dom = NULL; + + jobj = parallelsParseOutput(PRLCTL, "list", "-j", "-a", "-i", "-H", + "--vmtype", "vm", domain_name, NULL); + if (!jobj) { + parallelsParseError(); + goto cleanup; + } + + count = virJSONValueArraySize(jobj); + if (count != 1) { + parallelsParseError(); + goto cleanup; + } + + jobj2 = virJSONValueArrayGet(jobj, 0); + if (!jobj2) { + parallelsParseError(); + goto cleanup; + } + + dom = parallelsParseDomainInfo(privconn, jobj2); cleanup: virJSONValueFree(jobj); - return ret; + return dom; } static int @@ -626,18 +671,14 @@ parallelsOpenDefault(virConnectPtr conn) if (!(privconn->caps = parallelsBuildCapabilities())) goto error; - if (virDomainObjListInit(&privconn->domains) < 0) + if (!(privconn->domains = parallelsGetDomains(privconn))) goto error; conn->privateData = privconn; - if (parallelsLoadDomains(privconn, NULL)) - goto error; - return VIR_DRV_OPEN_SUCCESS; error: - virDomainObjListDeinit(&privconn->domains); virCapabilitiesFree(privconn->caps); virStoragePoolObjListFree(&privconn->pools); VIR_FREE(privconn); @@ -684,7 +725,8 @@ parallelsClose(virConnectPtr conn) parallelsDriverLock(privconn); virCapabilitiesFree(privconn->caps); - virDomainObjListDeinit(&privconn->domains); + virDomainObjListDeinit(privconn->domains); + VIR_FREE(privconn->domains); conn->privateData = NULL; parallelsDriverUnlock(privconn); @@ -747,7 +789,7 @@ parallelsListDomains(virConnectPtr conn, int *ids, int maxids) int n; parallelsDriverLock(privconn); - n = virDomainObjListGetActiveIDs(&privconn->domains, ids, maxids); + n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids); parallelsDriverUnlock(privconn); return n; @@ -760,7 +802,7 @@ parallelsNumOfDomains(virConnectPtr conn) int count; parallelsDriverLock(privconn); - count = virDomainObjListNumOfDomains(&privconn->domains, 1); + count = virDomainObjListNumOfDomains(privconn->domains, 1); parallelsDriverUnlock(privconn); return count; @@ -774,7 +816,7 @@ parallelsListDefinedDomains(virConnectPtr conn, char **const names, int maxnames parallelsDriverLock(privconn); memset(names, 0, sizeof(*names) * maxnames); - n = virDomainObjListGetInactiveNames(&privconn->domains, names, + n = virDomainObjListGetInactiveNames(privconn->domains, names, maxnames); parallelsDriverUnlock(privconn); @@ -788,7 +830,7 @@ parallelsNumOfDefinedDomains(virConnectPtr conn) int count; parallelsDriverLock(privconn); - count = virDomainObjListNumOfDomains(&privconn->domains, 0); + count = virDomainObjListNumOfDomains(privconn->domains, 0); parallelsDriverUnlock(privconn); return count; @@ -804,7 +846,7 @@ parallelsListAllDomains(virConnectPtr conn, virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1); parallelsDriverLock(privconn); - ret = virDomainList(conn, privconn->domains.objs, domains, flags); + ret = virDomainList(conn, privconn->domains->objs, domains, flags); parallelsDriverUnlock(privconn); return ret; @@ -818,7 +860,7 @@ parallelsLookupDomainByID(virConnectPtr conn, int id) virDomainObjPtr dom; parallelsDriverLock(privconn); - dom = virDomainFindByID(&privconn->domains, id); + dom = virDomainFindByID(privconn->domains, id); parallelsDriverUnlock(privconn); if (dom == NULL) { @@ -844,7 +886,7 @@ parallelsLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) virDomainObjPtr dom; parallelsDriverLock(privconn); - dom = virDomainFindByUUID(&privconn->domains, uuid); + dom = virDomainFindByUUID(privconn->domains, uuid); parallelsDriverUnlock(privconn); if (dom == NULL) { @@ -873,7 +915,7 @@ parallelsLookupDomainByName(virConnectPtr conn, const char *name) virDomainObjPtr dom; parallelsDriverLock(privconn); - dom = virDomainFindByName(&privconn->domains, name); + dom = virDomainFindByName(privconn->domains, name); parallelsDriverUnlock(privconn); if (dom == NULL) { @@ -900,7 +942,7 @@ parallelsGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) int ret = -1; parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(&privconn->domains, domain->uuid); + privdom = virDomainFindByUUID(privconn->domains, domain->uuid); parallelsDriverUnlock(privconn); if (privdom == NULL) { @@ -930,7 +972,7 @@ parallelsGetOSType(virDomainPtr domain) char *ret = NULL; parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(&privconn->domains, domain->uuid); + privdom = virDomainFindByUUID(privconn->domains, domain->uuid); if (privdom == NULL) { parallelsDomNotFoundError(domain); goto cleanup; @@ -954,7 +996,7 @@ parallelsDomainIsPersistent(virDomainPtr domain) int ret = -1; parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(&privconn->domains, domain->uuid); + privdom = virDomainFindByUUID(privconn->domains, domain->uuid); if (privdom == NULL) { parallelsDomNotFoundError(domain); goto cleanup; @@ -979,7 +1021,7 @@ parallelsDomainGetState(virDomainPtr domain, virCheckFlags(0, -1); parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(&privconn->domains, domain->uuid); + privdom = virDomainFindByUUID(privconn->domains, domain->uuid); parallelsDriverUnlock(privconn); if (privdom == NULL) { @@ -1007,7 +1049,7 @@ parallelsDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) /* Flags checked by virDomainDefFormat */ parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(&privconn->domains, domain->uuid); + privdom = virDomainFindByUUID(privconn->domains, domain->uuid); parallelsDriverUnlock(privconn); if (privdom == NULL) { @@ -1034,7 +1076,7 @@ parallelsDomainGetAutostart(virDomainPtr domain, int *autostart) int ret = -1; parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(&privconn->domains, domain->uuid); + privdom = virDomainFindByUUID(privconn->domains, domain->uuid); parallelsDriverUnlock(privconn); if (privdom == NULL) { @@ -1066,7 +1108,7 @@ parallelsDomainChangeState(virDomainPtr domain, int ret = -1; parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(&privconn->domains, domain->uuid); + privdom = virDomainFindByUUID(privconn->domains, domain->uuid); parallelsDriverUnlock(privconn); if (privdom == NULL) { @@ -1621,13 +1663,13 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) goto cleanup; } - if ((dupVM = virDomainObjIsDuplicate(&privconn->domains, def, 0)) < 0) { + if ((dupVM = virDomainObjIsDuplicate(privconn->domains, def, 0)) < 0) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Already exists")); goto cleanup; } if (dupVM == 1) { - olddom = virDomainFindByUUID(&privconn->domains, def->uuid); + olddom = virDomainFindByUUID(privconn->domains, def->uuid); if (parallelsApplyChanges(olddom, def) < 0) { virDomainObjUnlock(olddom); goto cleanup; @@ -1635,7 +1677,7 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) virDomainObjUnlock(olddom); if (!(dom = virDomainAssignDef(privconn->caps, - &privconn->domains, def, false))) { + privconn->domains, def, false))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Can't allocate domobj")); goto cleanup; @@ -1643,15 +1685,17 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) def = NULL; } else { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + if (parallelsCreateVm(conn, def)) goto cleanup; - if (parallelsLoadDomains(privconn, def->name)) + + if (!(dom = parallelsGetDomain(privconn, def->name))) goto cleanup; - dom = virDomainFindByName(&privconn->domains, def->name); - if (!dom) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Domain for '%s' is not defined after creation"), - def->name ? def->name : _("(unnamed)")); + + virUUIDFormat(dom->def->uuid, uuidstr); + if (virHashAddEntry(privconn->domains->objs, uuidstr, dom) < 0) { + VIR_FREE(dom); goto cleanup; } } diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h index 6a27003..3468c1d 100644 --- a/src/parallels/parallels_utils.h +++ b/src/parallels/parallels_utils.h @@ -32,7 +32,7 @@ struct _parallelsConn { virMutex lock; - virDomainObjList domains; + virDomainObjListPtr domains; virStoragePoolObjList pools; virCapsPtr caps; virDomainEventStatePtr domainEventState; -- 1.7.1
Rename the variable in macro to avoid warnings "declaration of 'uuidstr' shadows a previous local" in the future. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_driver.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 794d61d..8b83a9d 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -60,12 +60,12 @@ #define PRLSRVCTL "prlsrvctl" #define PARALLELS_DEFAULT_ARCH "x86_64" -#define parallelsDomNotFoundError(domain) \ - do { \ - char uuidstr[VIR_UUID_STRING_BUFLEN]; \ - virUUIDFormat(domain->uuid, uuidstr); \ - virReportError(VIR_ERR_NO_DOMAIN, \ - _("no domain with matching uuid '%s'"), uuidstr); \ +#define parallelsDomNotFoundError(domain) \ + do { \ + char __uuidstr[VIR_UUID_STRING_BUFLEN]; \ + virUUIDFormat(domain->uuid, __uuidstr); \ + virReportError(VIR_ERR_NO_DOMAIN, \ + _("no domain with matching uuid '%s'"), __uuidstr); \ } while (0) #define parallelsParseError() \ -- 1.7.1
Use parallelsGetDomain/'s in functions for domain lookup and information obtaining. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_driver.c | 185 +++++++++++++++++++------------------ 1 files changed, 95 insertions(+), 90 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 8b83a9d..de3d53f 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -786,12 +786,16 @@ static int parallelsListDomains(virConnectPtr conn, int *ids, int maxids) { parallelsConnPtr privconn = conn->privateData; - int n; + virDomainObjListPtr domains = NULL; + int n = -1; - parallelsDriverLock(privconn); - n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids); - parallelsDriverUnlock(privconn); + if (!(domains = parallelsGetDomains(privconn))) + goto cleanup; + n = virDomainObjListGetActiveIDs(domains, ids, maxids); +cleanup: + virDomainObjListDeinit(domains); + VIR_FREE(domains); return n; } @@ -799,12 +803,17 @@ static int parallelsNumOfDomains(virConnectPtr conn) { parallelsConnPtr privconn = conn->privateData; - int count; + virDomainObjListPtr domains = NULL; + int count = -1; - parallelsDriverLock(privconn); - count = virDomainObjListNumOfDomains(privconn->domains, 1); - parallelsDriverUnlock(privconn); + if (!(domains = parallelsGetDomains(privconn))) + goto cleanup; + + count = virDomainObjListNumOfDomains(domains, 1); +cleanup: + virDomainObjListDeinit(domains); + VIR_FREE(domains); return count; } @@ -812,14 +821,18 @@ static int parallelsListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { parallelsConnPtr privconn = conn->privateData; - int n; + virDomainObjListPtr domains = NULL; + int n = -1; + + if (!(domains = parallelsGetDomains(privconn))) + goto cleanup; - parallelsDriverLock(privconn); memset(names, 0, sizeof(*names) * maxnames); - n = virDomainObjListGetInactiveNames(privconn->domains, names, - maxnames); - parallelsDriverUnlock(privconn); + n = virDomainObjListGetInactiveNames(domains, names, maxnames); +cleanup: + virDomainObjListDeinit(domains); + VIR_FREE(domains); return n; } @@ -827,28 +840,39 @@ static int parallelsNumOfDefinedDomains(virConnectPtr conn) { parallelsConnPtr privconn = conn->privateData; - int count; + virDomainObjListPtr domains = NULL; + int count = -1; - parallelsDriverLock(privconn); - count = virDomainObjListNumOfDomains(privconn->domains, 0); - parallelsDriverUnlock(privconn); + if (!(domains = parallelsGetDomains(privconn))) + goto cleanup; + count = virDomainObjListNumOfDomains(domains, 0); + +cleanup: + virDomainObjListDeinit(domains); + VIR_FREE(domains); return count; } static int parallelsListAllDomains(virConnectPtr conn, - virDomainPtr **domains, + virDomainPtr **doms, unsigned int flags) { parallelsConnPtr privconn = conn->privateData; + virDomainObjListPtr domains = NULL; int ret = -1; virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1); - parallelsDriverLock(privconn); - ret = virDomainList(conn, privconn->domains->objs, domains, flags); - parallelsDriverUnlock(privconn); + if (!(domains = parallelsGetDomains(privconn))) + goto cleanup; + + ret = virDomainList(conn, domains->objs, doms, flags); + +cleanup: + virDomainObjListDeinit(domains); + VIR_FREE(domains); return ret; } @@ -857,11 +881,13 @@ parallelsLookupDomainByID(virConnectPtr conn, int id) { parallelsConnPtr privconn = conn->privateData; virDomainPtr ret = NULL; - virDomainObjPtr dom; + virDomainObjPtr dom = NULL; + virDomainObjListPtr domains = NULL; - parallelsDriverLock(privconn); - dom = virDomainFindByID(privconn->domains, id); - parallelsDriverUnlock(privconn); + if (!(domains = parallelsGetDomains(privconn))) + goto cleanup; + + dom = virDomainFindByID(domains, id); if (dom == NULL) { virReportError(VIR_ERR_NO_DOMAIN, NULL); @@ -875,6 +901,8 @@ parallelsLookupDomainByID(virConnectPtr conn, int id) cleanup: if (dom) virDomainObjUnlock(dom); + virDomainObjListDeinit(domains); + VIR_FREE(domains); return ret; } @@ -883,15 +911,12 @@ parallelsLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) { parallelsConnPtr privconn = conn->privateData; virDomainPtr ret = NULL; - virDomainObjPtr dom; + virDomainObjPtr dom = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; - parallelsDriverLock(privconn); - dom = virDomainFindByUUID(privconn->domains, uuid); - parallelsDriverUnlock(privconn); + virUUIDFormat(uuid, uuidstr); - if (dom == NULL) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(uuid, uuidstr); + if (!(dom = parallelsGetDomain(privconn, uuidstr))) { virReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; @@ -902,8 +927,7 @@ parallelsLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) ret->id = dom->def->id; cleanup: - if (dom) - virDomainObjUnlock(dom); + virObjectUnref(dom); return ret; } @@ -912,13 +936,9 @@ parallelsLookupDomainByName(virConnectPtr conn, const char *name) { parallelsConnPtr privconn = conn->privateData; virDomainPtr ret = NULL; - virDomainObjPtr dom; - - parallelsDriverLock(privconn); - dom = virDomainFindByName(privconn->domains, name); - parallelsDriverUnlock(privconn); + virDomainObjPtr dom = NULL; - if (dom == NULL) { + if (!(dom = parallelsGetDomain(privconn, name))) { virReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching name '%s'"), name); goto cleanup; @@ -929,8 +949,7 @@ parallelsLookupDomainByName(virConnectPtr conn, const char *name) ret->id = dom->def->id; cleanup: - if (dom) - virDomainObjUnlock(dom); + virObjectUnref(dom); return ret; } @@ -938,14 +957,13 @@ static int parallelsGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) { parallelsConnPtr privconn = domain->conn->privateData; - virDomainObjPtr privdom; + virDomainObjPtr privdom = NULL; int ret = -1; + char uuidstr[VIR_UUID_STRING_BUFLEN]; - parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(privconn->domains, domain->uuid); - parallelsDriverUnlock(privconn); + virUUIDFormat(domain->uuid, uuidstr); - if (privdom == NULL) { + if (!(privdom == parallelsGetDomain(privconn, uuidstr))) { parallelsDomNotFoundError(domain); goto cleanup; } @@ -958,8 +976,7 @@ parallelsGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) ret = 0; cleanup: - if (privdom) - virDomainObjUnlock(privdom); + virObjectUnref(privdom); return ret; } @@ -967,13 +984,13 @@ static char * parallelsGetOSType(virDomainPtr domain) { parallelsConnPtr privconn = domain->conn->privateData; - virDomainObjPtr privdom; - + virDomainObjPtr privdom = NULL; char *ret = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; - parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(privconn->domains, domain->uuid); - if (privdom == NULL) { + virUUIDFormat(domain->uuid, uuidstr); + + if (!(privdom = parallelsGetDomain(privconn, uuidstr))) { parallelsDomNotFoundError(domain); goto cleanup; } @@ -982,9 +999,7 @@ parallelsGetOSType(virDomainPtr domain) virReportOOMError(); cleanup: - if (privdom) - virDomainObjUnlock(privdom); - parallelsDriverUnlock(privconn); + virObjectUnref(privdom); return ret; } @@ -992,12 +1007,13 @@ static int parallelsDomainIsPersistent(virDomainPtr domain) { parallelsConnPtr privconn = domain->conn->privateData; - virDomainObjPtr privdom; + virDomainObjPtr privdom = NULL; int ret = -1; + char uuidstr[VIR_UUID_STRING_BUFLEN]; - parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(privconn->domains, domain->uuid); - if (privdom == NULL) { + virUUIDFormat(domain->uuid, uuidstr); + + if (!(privdom = parallelsGetDomain(privconn, uuidstr))) { parallelsDomNotFoundError(domain); goto cleanup; } @@ -1005,9 +1021,7 @@ parallelsDomainIsPersistent(virDomainPtr domain) ret = 1; cleanup: - if (privdom) - virDomainObjUnlock(privdom); - parallelsDriverUnlock(privconn); + virObjectUnref(privdom); return ret; } @@ -1016,15 +1030,15 @@ parallelsDomainGetState(virDomainPtr domain, int *state, int *reason, unsigned int flags) { parallelsConnPtr privconn = domain->conn->privateData; - virDomainObjPtr privdom; + virDomainObjPtr privdom = NULL; int ret = -1; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virCheckFlags(0, -1); - parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(privconn->domains, domain->uuid); - parallelsDriverUnlock(privconn); + virUUIDFormat(domain->uuid, uuidstr); - if (privdom == NULL) { + if (!(privdom = parallelsGetDomain(privconn, uuidstr))) { parallelsDomNotFoundError(domain); goto cleanup; } @@ -1033,8 +1047,7 @@ parallelsDomainGetState(virDomainPtr domain, ret = 0; cleanup: - if (privdom) - virDomainObjUnlock(privdom); + virObjectUnref(privdom); return ret; } @@ -1042,29 +1055,23 @@ static char * parallelsDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) { parallelsConnPtr privconn = domain->conn->privateData; - virDomainDefPtr def; - virDomainObjPtr privdom; + virDomainObjPtr privdom = NULL; char *ret = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; /* Flags checked by virDomainDefFormat */ - parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(privconn->domains, domain->uuid); - parallelsDriverUnlock(privconn); + virUUIDFormat(domain->uuid, uuidstr); - if (privdom == NULL) { + if (!(privdom = parallelsGetDomain(privconn, uuidstr))) { parallelsDomNotFoundError(domain); goto cleanup; } - def = (flags & VIR_DOMAIN_XML_INACTIVE) && - privdom->newDef ? privdom->newDef : privdom->def; - - ret = virDomainDefFormat(def, flags); + ret = virDomainDefFormat(privdom->def, flags); cleanup: - if (privdom) - virDomainObjUnlock(privdom); + virObjectUnref(privdom); return ret; } @@ -1072,14 +1079,13 @@ static int parallelsDomainGetAutostart(virDomainPtr domain, int *autostart) { parallelsConnPtr privconn = domain->conn->privateData; - virDomainObjPtr privdom; + virDomainObjPtr privdom = NULL; int ret = -1; + char uuidstr[VIR_UUID_STRING_BUFLEN]; - parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(privconn->domains, domain->uuid); - parallelsDriverUnlock(privconn); + virUUIDFormat(domain->uuid, uuidstr); - if (privdom == NULL) { + if (!(privdom = parallelsGetDomain(privconn, uuidstr))) { parallelsDomNotFoundError(domain); goto cleanup; } @@ -1088,8 +1094,7 @@ parallelsDomainGetAutostart(virDomainPtr domain, int *autostart) ret = 0; cleanup: - if (privdom) - virDomainObjUnlock(privdom); + virObjectUnref(privdom); return ret; } -- 1.7.1
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_driver.c | 101 ++++++-------------------------------- 1 files changed, 16 insertions(+), 85 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index de3d53f..1f6dcef 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1098,118 +1098,49 @@ parallelsDomainGetAutostart(virDomainPtr domain, int *autostart) return ret; } -typedef int (*parallelsChangeStateFunc) (virDomainObjPtr privdom); -#define PARALLELS_UUID(x) (((parallelsDomObjPtr)(x->privateData))->uuid) - -static int -parallelsDomainChangeState(virDomainPtr domain, - virDomainState req_state, const char *req_state_name, - parallelsChangeStateFunc chstate, - virDomainState new_state, int reason) -{ - parallelsConnPtr privconn = domain->conn->privateData; - virDomainObjPtr privdom; - int state; - int ret = -1; - - parallelsDriverLock(privconn); - privdom = virDomainFindByUUID(privconn->domains, domain->uuid); - parallelsDriverUnlock(privconn); - - if (privdom == NULL) { - parallelsDomNotFoundError(domain); - goto cleanup; - } - - state = virDomainObjGetState(privdom, NULL); - if (state != req_state) { - virReportError(VIR_ERR_INTERNAL_ERROR, _("domain '%s' not %s"), - privdom->def->name, req_state_name); - goto cleanup; - } - - if (chstate(privdom)) - goto cleanup; - - virDomainObjSetState(privdom, new_state, reason); - - ret = 0; - - cleanup: - if (privdom) - virDomainObjUnlock(privdom); - - return ret; -} - -static int parallelsPause(virDomainObjPtr privdom) -{ - return parallelsCmdRun(PRLCTL, "pause", PARALLELS_UUID(privdom), NULL); -} - static int parallelsPauseDomain(virDomainPtr domain) { - return parallelsDomainChangeState(domain, - VIR_DOMAIN_RUNNING, "running", - parallelsPause, - VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER); -} + char uuidstr[VIR_UUID_STRING_BUFLEN]; -static int parallelsResume(virDomainObjPtr privdom) -{ - return parallelsCmdRun(PRLCTL, "resume", PARALLELS_UUID(privdom), NULL); + virUUIDFormat(domain->uuid, uuidstr); + return parallelsCmdRun(PRLCTL, "pause", uuidstr, NULL) ? -1 : 0; } static int parallelsResumeDomain(virDomainPtr domain) { - return parallelsDomainChangeState(domain, - VIR_DOMAIN_PAUSED, "paused", - parallelsResume, - VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNPAUSED); -} + char uuidstr[VIR_UUID_STRING_BUFLEN]; -static int parallelsStart(virDomainObjPtr privdom) -{ - return parallelsCmdRun(PRLCTL, "start", PARALLELS_UUID(privdom), NULL); + virUUIDFormat(domain->uuid, uuidstr); + return parallelsCmdRun(PRLCTL, "resume", uuidstr, NULL) ? -1 : 0; } static int parallelsDomainCreate(virDomainPtr domain) { - return parallelsDomainChangeState(domain, - VIR_DOMAIN_SHUTOFF, "stopped", - parallelsStart, - VIR_DOMAIN_RUNNING, VIR_DOMAIN_EVENT_STARTED_BOOTED); -} + char uuidstr[VIR_UUID_STRING_BUFLEN]; -static int parallelsKill(virDomainObjPtr privdom) -{ - return parallelsCmdRun(PRLCTL, "stop", PARALLELS_UUID(privdom), "--kill", NULL); + virUUIDFormat(domain->uuid, uuidstr); + return parallelsCmdRun(PRLCTL, "start", uuidstr, NULL) ? -1 : 0; } static int parallelsDestroyDomain(virDomainPtr domain) { - return parallelsDomainChangeState(domain, - VIR_DOMAIN_RUNNING, "running", - parallelsKill, - VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_DESTROYED); -} + char uuidstr[VIR_UUID_STRING_BUFLEN]; -static int parallelsStop(virDomainObjPtr privdom) -{ - return parallelsCmdRun(PRLCTL, "stop", PARALLELS_UUID(privdom), NULL); + virUUIDFormat(domain->uuid, uuidstr); + return parallelsCmdRun(PRLCTL, "stop", uuidstr, "--kill", NULL) ? -1 : 0; } static int parallelsShutdownDomain(virDomainPtr domain) { - return parallelsDomainChangeState(domain, - VIR_DOMAIN_RUNNING, "running", - parallelsStop, - VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_SHUTDOWN); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(domain->uuid, uuidstr); + return parallelsCmdRun(PRLCTL, "stop", uuidstr, NULL) ? -1 : 0; } static int -- 1.7.1
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_driver.c | 40 +++++++++++++++---------------------- 1 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 1f6dcef..72e32ff 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1589,8 +1589,9 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) virDomainDefPtr def; virDomainObjPtr dom = NULL, olddom = NULL; int dupVM; + virDomainObjListPtr domains = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; - parallelsDriverLock(privconn); if ((def = virDomainDefParseString(privconn->caps, xml, 1 << VIR_DOMAIN_VIRT_PARALLELS, VIR_DOMAIN_XML_INACTIVE)) == NULL) { @@ -1599,41 +1600,32 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) goto cleanup; } - if ((dupVM = virDomainObjIsDuplicate(privconn->domains, def, 0)) < 0) { + if (!(domains = parallelsGetDomains(privconn))) + goto cleanup; + + if ((dupVM = virDomainObjIsDuplicate(domains, def, 0)) < 0) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Already exists")); goto cleanup; } if (dupVM == 1) { - olddom = virDomainFindByUUID(privconn->domains, def->uuid); + olddom = virDomainFindByUUID(domains, def->uuid); if (parallelsApplyChanges(olddom, def) < 0) { virDomainObjUnlock(olddom); goto cleanup; } virDomainObjUnlock(olddom); - - if (!(dom = virDomainAssignDef(privconn->caps, - privconn->domains, def, false))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Can't allocate domobj")); - goto cleanup; - } - - def = NULL; } else { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (parallelsCreateVm(conn, def)) goto cleanup; + } - if (!(dom = parallelsGetDomain(privconn, def->name))) - goto cleanup; + virUUIDFormat(def->uuid, uuidstr); - virUUIDFormat(dom->def->uuid, uuidstr); - if (virHashAddEntry(privconn->domains->objs, uuidstr, dom) < 0) { - VIR_FREE(dom); - goto cleanup; - } + if (!(dom = parallelsGetDomain(privconn, uuidstr))) { + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; } ret = virGetDomain(conn, dom->def->name, dom->def->uuid); @@ -1642,9 +1634,9 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) cleanup: virDomainDefFree(def); - if (dom) - virDomainObjUnlock(dom); - parallelsDriverUnlock(privconn); + virObjectUnref(dom); + virDomainObjListDeinit(domains); + VIR_FREE(domains); return ret; } -- 1.7.1
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_driver.c | 5 ----- src/parallels/parallels_utils.h | 1 - 2 files changed, 0 insertions(+), 6 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 72e32ff..96a8ab1 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -671,9 +671,6 @@ parallelsOpenDefault(virConnectPtr conn) if (!(privconn->caps = parallelsBuildCapabilities())) goto error; - if (!(privconn->domains = parallelsGetDomains(privconn))) - goto error; - conn->privateData = privconn; return VIR_DRV_OPEN_SUCCESS; @@ -725,8 +722,6 @@ parallelsClose(virConnectPtr conn) parallelsDriverLock(privconn); virCapabilitiesFree(privconn->caps); - virDomainObjListDeinit(privconn->domains); - VIR_FREE(privconn->domains); conn->privateData = NULL; parallelsDriverUnlock(privconn); diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h index 3468c1d..16a880f 100644 --- a/src/parallels/parallels_utils.h +++ b/src/parallels/parallels_utils.h @@ -32,7 +32,6 @@ struct _parallelsConn { virMutex lock; - virDomainObjListPtr domains; virStoragePoolObjList pools; virCapsPtr caps; virDomainEventStatePtr domainEventState; -- 1.7.1
On 120905 14:13:52, Dmitry Guryanov wrote:
The goal of this patch series is to get rid of cached domains list. Someone can create several connections to libvirt or change something (start VM or change parameters) using native tools. In such case first connection will not get any info about those changes. There is no way to check, if the cache valid, so remove it and always query needed data using prlctl command.
Hello, Please, disregard this patch series. Driver works too slow with it, especially with virt-manager. We decided to add special mode to prlctl, when it will listen for events and print them to stdout in json format. Then we can keep one prlctl process in such mode per libvirt connection and update cached list of domains, when some events occur.
Dmitry Guryanov (6): parallels: return a new list of domains from parallelsLoadDomains parallels: rename uuidstr to __uuidstr in macro parallels: return up-to-date info about domains parallels: don't track domain state in global domains list parallels: don't use stored domains list in parallelsDomainDefineXML parallels: remove domains list from _parallelsConn structure
src/parallels/parallels_driver.c | 423 ++++++++++++++++++-------------------- src/parallels/parallels_utils.h | 1 - 2 files changed, 195 insertions(+), 229 deletions(-)
participants (1)
-
Dmitry Guryanov