From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> Switch virDomainObjList to inherit from virObjectLockable and make all the APIs acquire/release the mutex when running. This makes virDomainObjList completely self-locking and no longer reliant on the hypervisor driver locks --- src/conf/domain_conf.c | 75 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 79da5eb..a1e899f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -60,7 +60,7 @@ verify(VIR_DOMAIN_VIRT_LAST <= 32); struct _virDomainObjList { - virObject parent; + virObjectLockable parent; /* uuid string -> virDomainObj mapping * for O(1), lockless lookup-by-uuid */ @@ -715,7 +715,7 @@ static int virDomainObjOnceInit(void) virDomainObjDispose))) return -1; - if (!(virDomainObjListClass = virClassNew(virClassForObject(), + if (!(virDomainObjListClass = virClassNew(virClassForObjectLockable(), "virDomainObjList", sizeof(virDomainObjList), virDomainObjListDispose))) @@ -840,9 +840,11 @@ virDomainObjPtr virDomainObjListFindByID(const virDomainObjListPtr doms, int id) { virDomainObjPtr obj; + virObjectLock(doms); obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id); if (obj) virObjectLock(obj); + virObjectUnlock(doms); return obj; } @@ -853,11 +855,13 @@ virDomainObjPtr virDomainObjListFindByUUID(const virDomainObjListPtr doms, char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainObjPtr obj; + virObjectLock(doms); virUUIDFormat(uuid, uuidstr); obj = virHashLookup(doms->objs, uuidstr); if (obj) virObjectLock(obj); + virObjectUnlock(doms); return obj; } @@ -879,9 +883,11 @@ virDomainObjPtr virDomainObjListFindByName(const virDomainObjListPtr doms, const char *name) { virDomainObjPtr obj; + virObjectLock(doms); obj = virHashSearch(doms->objs, virDomainObjListSearchName, name); if (obj) virObjectLock(obj); + virObjectUnlock(doms); return obj; } @@ -1880,25 +1886,32 @@ void virDomainObjAssignDef(virDomainObjPtr domain, * current def is Live * */ -virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, - virCapsPtr caps, - const virDomainDefPtr def, - unsigned int flags, - virDomainDefPtr *oldDef) +static virDomainObjPtr +virDomainObjListAddLocked(virDomainObjListPtr doms, + virCapsPtr caps, + const virDomainDefPtr def, + unsigned int flags, + virDomainDefPtr *oldDef) { virDomainObjPtr vm; char uuidstr[VIR_UUID_STRING_BUFLEN]; + if (oldDef) *oldDef = false; + virUUIDFormat(def->uuid, uuidstr); + /* See if a VM with matching UUID already exists */ - if ((vm = virDomainObjListFindByUUID(doms, def->uuid))) { + if ((vm = virHashLookup(doms->objs, uuidstr))) { + virObjectLock(vm); /* UUID matches, but if names don't match, refuse it */ if (STRNEQ(vm->def->name, def->name)) { virUUIDFormat(vm->def->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, _("domain '%s' is already defined with uuid %s"), vm->def->name, uuidstr); + virObjectUnlock(vm); + vm = NULL; goto cleanup; } @@ -1908,6 +1921,8 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, virReportError(VIR_ERR_OPERATION_INVALID, _("domain is already active as '%s'"), vm->def->name); + virObjectUnlock(vm); + vm = NULL; goto cleanup; } } @@ -1934,12 +1949,11 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, } } else { /* UUID does not match, but if a name matches, refuse it */ - if ((vm = virDomainObjListFindByName(doms, def->name))) { + if ((vm = virHashSearch(doms->objs, virDomainObjListSearchName, def->name))) { virUUIDFormat(vm->def->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, _("domain '%s' already exists with uuid %s"), def->name, uuidstr); - virObjectUnlock(vm); vm = NULL; goto cleanup; } @@ -1958,6 +1972,21 @@ cleanup: return vm; } + +virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, + virCapsPtr caps, + const virDomainDefPtr def, + unsigned int flags, + virDomainDefPtr *oldDef) +{ + virDomainObjPtr ret; + + virObjectLock(doms); + ret = virDomainObjListAddLocked(doms, caps, def, flags, oldDef); + virObjectUnlock(doms); + return ret; +} + /* * Mark the running VM config as transient. Ensures transient hotplug * operations do not persist past shutdown. @@ -2076,11 +2105,14 @@ void virDomainObjListRemove(virDomainObjListPtr doms, virDomainObjPtr dom) { char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virObjectLock(doms); virUUIDFormat(dom->def->uuid, uuidstr); virObjectUnlock(dom); virHashRemoveEntry(doms->objs, uuidstr); + virObjectUnlock(doms); } @@ -14880,6 +14912,7 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms, virDomainObjPtr dom; int autostart; int newVM = 1; + char uuidstr[VIR_UUID_STRING_BUFLEN]; if ((configFile = virDomainConfigFile(configDir, name)) == NULL) goto error; @@ -14896,7 +14929,10 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms, /* if the domain is already in our hashtable, we only need to * update the autostart flag */ - if ((dom = virDomainObjListFindByUUID(doms, def->uuid))) { + virUUIDFormat(def->uuid, uuidstr); + + if ((dom = virHashLookup(doms->objs, uuidstr))) { + virObjectLock(dom); dom->autostart = autostart; if (virDomainObjIsActive(dom) && @@ -14911,7 +14947,7 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms, return dom; } - if (!(dom = virDomainObjListAdd(doms, caps, def, 0, NULL))) + if (!(dom = virDomainObjListAddLocked(doms, caps, def, 0, NULL))) goto error; dom->autostart = autostart; @@ -14999,6 +15035,8 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, return -1; } + virObjectLock(doms); + while ((entry = readdir(dir))) { virDomainObjPtr dom; @@ -15036,7 +15074,7 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, } closedir(dir); - + virObjectUnlock(doms); return 0; } @@ -15161,10 +15199,12 @@ static void virDomainObjListCountInactive(void *payload, const void *name ATTRIB int virDomainObjListNumOfDomains(virDomainObjListPtr doms, int active) { int count = 0; + virObjectLock(doms); if (active) virHashForEach(doms->objs, virDomainObjListCountActive, &count); else virHashForEach(doms->objs, virDomainObjListCountInactive, &count); + virObjectUnlock(doms); return count; } @@ -15189,7 +15229,9 @@ int virDomainObjListGetActiveIDs(virDomainObjListPtr doms, int maxids) { struct virDomainIDData data = { 0, maxids, ids }; + virObjectLock(doms); virHashForEach(doms->objs, virDomainObjListCopyActiveIDs, &data); + virObjectUnlock(doms); return data.numids; } @@ -15225,7 +15267,9 @@ int virDomainObjListGetInactiveNames(virDomainObjListPtr doms, { struct virDomainNameData data = { 0, 0, maxnames, names }; int i; + virObjectLock(doms); virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, &data); + virObjectUnlock(doms); if (data.oom) { virReportOOMError(); goto cleanup; @@ -15261,8 +15305,9 @@ int virDomainObjListForEach(virDomainObjListPtr doms, struct virDomainListIterData data = { callback, opaque, 0, }; + virObjectLock(doms); virHashForEach(doms->objs, virDomainObjListHelper, &data); - + virObjectUnlock(doms); return data.ret; } @@ -16033,6 +16078,7 @@ virDomainObjListExport(virDomainObjListPtr doms, struct virDomainListData data = { conn, NULL, flags, 0, false }; + virObjectLock(doms); if (domains) { if (VIR_ALLOC_N(data.domains, virHashSize(doms->objs) + 1) < 0) { virReportOOMError(); @@ -16062,6 +16108,7 @@ cleanup: } VIR_FREE(data.domains); + virObjectUnlock(doms); return ret; } -- 1.8.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list