8:48 a.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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 02fcd84..31467d5 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 */
@@ -717,7 +717,7 @@ static int virDomainObjOnceInit(void)
virDomainObjDispose)))
return -1;
- if (!(virDomainObjListClass = virClassNew(virClassForObject(),
+ if (!(virDomainObjListClass = virClassNew(virClassForObjectLockable(),
"virDomainObjList",
sizeof(virDomainObjList),
virDomainObjListDispose)))
@@ -842,9 +842,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;
}
@@ -855,11 +857,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;
}
@@ -881,9 +885,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;
}
@@ -1882,25 +1888,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;
}
@@ -1910,6 +1923,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;
}
}
@@ -1936,12 +1951,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;
}
@@ -1960,6 +1974,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.
@@ -2078,11 +2107,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);
}
@@ -14863,6 +14895,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;
@@ -14879,7 +14912,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) &&
@@ -14894,7 +14930,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;
@@ -14982,6 +15018,8 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
return -1;
}
+ virObjectLock(doms);
+
while ((entry = readdir(dir))) {
virDomainObjPtr dom;
@@ -15019,7 +15057,7 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
}
closedir(dir);
-
+ virObjectUnlock(doms);
return 0;
}
@@ -15144,10 +15182,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;
}
@@ -15172,7 +15212,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;
}
@@ -15208,7 +15250,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;
@@ -15244,8 +15288,9 @@ int virDomainObjListForEach(virDomainObjListPtr doms,
struct virDomainListIterData data = {
callback, opaque, 0,
};
+ virObjectLock(doms);
virHashForEach(doms->objs, virDomainObjListHelper, &data);
-
+ virObjectUnlock(doms);
return data.ret;
}
@@ -16008,6 +16053,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();
@@ -16037,6 +16083,7 @@ cleanup:
}
VIR_FREE(data.domains);
+ virObjectUnlock(doms);
return ret;
}
--
1.8.0.2