Re: [libvirt] [PATCH v3 05/13] Make virDomainObjList self-locking via virObjectLockable

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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)))

@@ -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; }