[libvirt] [PATCH 01/21] Convert virDomainObjListPtr to use a hash of domain objects

Daniel P. Berrange berrange at redhat.com
Fri Oct 23 13:05:30 UTC 2009


The current virDomainObjListPtr object stores domain objects in
an array. This means that to find a particular objects requires
O(n) time, and more critically acquiring O(n) mutex locks.

The new impl replaces the array with a virHashTable, keyed off
UUID. Finding a object based on UUID is now O(1) time, and only
requires a single mutex lock. Finding by name/id is unchanged
in complexity.

In changing this, all code which iterates over the array had
to be updated to use a hash table iterator function callback.
Several of the functions which were identically duplicating
across all drivers were pulled into domain_conf.c

* src/conf/domain_conf.h, src/conf/domain_conf.c: Change
  virDomainObjListPtr to use virHashTable. Add a initializer
  method virDomainObjListInit, and rename virDomainObjListFree
  to virDomainObjListDeinit, since its not actually freeing
  the container, only its contents. Also add some convenient
  methods virDomainObjListGetInactiveNames,
  virDomainObjListGetActiveIDs and virDomainObjListNumOfDomains
  which can be used to implement the correspondingly named
  public API entry points in drivers
* src/libvirt_private.syms: Export new methods from domain_conf.h
* src/lxc/lxc_driver.c, src/opennebula/one_driver.c,
  src/openvz/openvz_conf.c, src/openvz/openvz_driver.c,
  src/qemu/qemu_driver.c, src/test/test_driver.c,
  src/uml/uml_driver.c, src/vbox/vbox_tmpl.c: Update all code
  to deal with hash tables instead of arrays for domains
---
 src/conf/domain_conf.c      |  274 ++++++++++++++++++++++++++++++-------------
 src/conf/domain_conf.h      |   19 +++-
 src/libvirt_private.syms    |    6 +-
 src/lxc/lxc_driver.c        |  247 +++++++++++++++++----------------------
 src/opennebula/one_driver.c |   68 +++--------
 src/openvz/openvz_conf.c    |    7 +-
 src/openvz/openvz_driver.c  |   25 ++---
 src/qemu/qemu_driver.c      |  221 ++++++++++++++--------------------
 src/test/test_driver.c      |   60 +++-------
 src/uml/uml_driver.c        |  125 +++++++++-----------
 src/vbox/vbox_tmpl.c        |    2 -
 11 files changed, 519 insertions(+), 535 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7d558e8..de07e13 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -205,51 +205,93 @@ VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST,
 
 #ifndef PROXY
 
-virDomainObjPtr virDomainFindByID(const virDomainObjListPtr doms,
-                                  int id)
+int virDomainObjListInit(virDomainObjListPtr doms)
 {
-    unsigned int i;
-
-    for (i = 0 ; i < doms->count ; i++) {
-        virDomainObjLock(doms->objs[i]);
-        if (virDomainIsActive(doms->objs[i]) &&
-            doms->objs[i]->def->id == id)
-            return doms->objs[i];
-        virDomainObjUnlock(doms->objs[i]);
+    doms->objs = virHashCreate(50);
+    if (!doms->objs) {
+        virReportOOMError(NULL);
+        return -1;
     }
+    return 0;
+}
 
-    return NULL;
+
+static void virDomainObjListDeallocator(void *payload, const char *name ATTRIBUTE_UNUSED)
+{
+    virDomainObjPtr obj = payload;
+    virDomainObjFree(obj);
+}
+
+void virDomainObjListDeinit(virDomainObjListPtr doms)
+{
+    if (doms->objs)
+        virHashFree(doms->objs, virDomainObjListDeallocator);
+}
+
+
+static int virDomainObjListSearchID(const void *payload,
+                                    const char *name ATTRIBUTE_UNUSED,
+                                    const void *data)
+{
+    virDomainObjPtr obj = (virDomainObjPtr)payload;
+    const int *id = data;
+    int want = 0;
+
+    virDomainObjLock(obj);
+    if (virDomainIsActive(obj) &&
+        obj->def->id == *id)
+        want = 1;
+    virDomainObjUnlock(obj);
+    return want;
+}
+
+virDomainObjPtr virDomainFindByID(const virDomainObjListPtr doms,
+                                  int id)
+{
+    virDomainObjPtr obj;
+    obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id);
+    if (obj)
+        virDomainObjLock(obj);
+    return obj;
 }
 
 
 virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms,
                                     const unsigned char *uuid)
 {
-    unsigned int i;
+    char uuidstr[VIR_UUID_STRING_BUFLEN];
+    virDomainObjPtr obj;
 
-    for (i = 0 ; i < doms->count ; i++) {
-        virDomainObjLock(doms->objs[i]);
-        if (!memcmp(doms->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
-            return doms->objs[i];
-        virDomainObjUnlock(doms->objs[i]);
-    }
+    virUUIDFormat(uuid, uuidstr);
 
-    return NULL;
+    obj = virHashLookup(doms->objs, uuidstr);
+    if (obj)
+        virDomainObjLock(obj);
+    return obj;
+}
+
+static int virDomainObjListSearchName(const void *payload,
+                                      const char *name ATTRIBUTE_UNUSED,
+                                      const void *data)
+{
+    virDomainObjPtr obj = (virDomainObjPtr)payload;
+    int want = 0;
+
+    virDomainObjLock(obj);
+    if (STREQ(obj->def->name, (const char *)data))
+        want = 1;
+    virDomainObjUnlock(obj);
+    return want;
 }
 
 virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
                                     const char *name)
 {
-    unsigned int i;
-
-    for (i = 0 ; i < doms->count ; i++) {
-        virDomainObjLock(doms->objs[i]);
-        if (STREQ(doms->objs[i]->def->name, name))
-            return doms->objs[i];
-        virDomainObjUnlock(doms->objs[i]);
-    }
-
-    return NULL;
+    virDomainObjPtr obj;
+    obj = virHashSearch(doms->objs, virDomainObjListSearchName, name);
+    if (obj)
+        virDomainObjLock(obj);
+    return obj;
 }
 
 #endif /* !PROXY */
@@ -557,21 +599,6 @@ void virDomainObjFree(virDomainObjPtr dom)
     VIR_FREE(dom);
 }
 
-void virDomainObjListFree(virDomainObjListPtr vms)
-{
-    unsigned int i;
-
-    if (!vms)
-        return;
-
-    for (i = 0 ; i < vms->count ; i++)
-        virDomainObjFree(vms->objs[i]);
-
-    VIR_FREE(vms->objs);
-    vms->count = 0;
-}
-
-
 static virDomainObjPtr virDomainObjNew(virConnectPtr conn)
 {
     virDomainObjPtr domain;
@@ -601,6 +628,7 @@ virDomainObjPtr virDomainAssignDef(virConnectPtr conn,
                                    const virDomainDefPtr def)
 {
     virDomainObjPtr domain;
+    char uuidstr[VIR_UUID_STRING_BUFLEN];
 
     if ((domain = virDomainFindByUUID(doms, def->uuid))) {
         if (!virDomainIsActive(domain)) {
@@ -615,49 +643,34 @@ virDomainObjPtr virDomainAssignDef(virConnectPtr conn,
         return domain;
     }
 
-    if (VIR_REALLOC_N(doms->objs, doms->count + 1) < 0) {
-        virReportOOMError(conn);
-        return NULL;
-    }
-
     if (!(domain = virDomainObjNew(conn)))
         return NULL;
-
     domain->def = def;
 
-    doms->objs[doms->count] = domain;
-    doms->count++;
+    virUUIDFormat(def->uuid, uuidstr);
+    if (virHashAddEntry(doms->objs, uuidstr, domain) < 0) {
+        VIR_FREE(domain);
+        virReportOOMError(conn);
+        return NULL;
+    }
 
     return domain;
 }
 
+/*
+ * The caller must hold a lock  on the driver owning 'doms',
+ * and must also have locked 'dom', to ensure no one else
+ * is either waiting for 'dom' or still usingn it
+ */
 void virDomainRemoveInactive(virDomainObjListPtr doms,
                              virDomainObjPtr dom)
 {
-    unsigned int i;
+    char uuidstr[VIR_UUID_STRING_BUFLEN];
+    virUUIDFormat(dom->def->uuid, uuidstr);
 
     virDomainObjUnlock(dom);
 
-    for (i = 0 ; i < doms->count ; i++) {
-        virDomainObjLock(doms->objs[i]);
-        if (doms->objs[i] == dom) {
-            virDomainObjUnlock(doms->objs[i]);
-            virDomainObjFree(doms->objs[i]);
-
-            if (i < (doms->count - 1))
-                memmove(doms->objs + i, doms->objs + i + 1,
-                        sizeof(*(doms->objs)) * (doms->count - (i + 1)));
-
-            if (VIR_REALLOC_N(doms->objs, doms->count - 1) < 0) {
-                ; /* Failure to reduce memory allocation isn't fatal */
-            }
-            doms->count--;
-
-            break;
-        }
-        virDomainObjUnlock(doms->objs[i]);
-    }
-
+    virHashRemoveEntry(doms->objs, uuidstr, virDomainObjListDeallocator);
 }
 
 
@@ -4784,7 +4797,7 @@ static virDomainObjPtr virDomainLoadStatus(virConnectPtr conn,
 {
     char *statusFile = NULL;
     virDomainObjPtr obj = NULL;
-    virDomainObjPtr tmp = NULL;
+    char uuidstr[VIR_UUID_STRING_BUFLEN];
 
     if ((statusFile = virDomainConfigFile(conn, statusDir, name)) == NULL)
         goto error;
@@ -4792,23 +4805,20 @@ static virDomainObjPtr virDomainLoadStatus(virConnectPtr conn,
     if (!(obj = virDomainObjParseFile(conn, caps, statusFile)))
         goto error;
 
-    tmp = virDomainFindByName(doms, obj->def->name);
-    if (tmp) {
-        virDomainObjUnlock(obj);
+    virUUIDFormat(obj->def->uuid, uuidstr);
+
+    if (virHashLookup(doms->objs, uuidstr) != NULL) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
                              _("unexpected domain %s already exists"),
                              obj->def->name);
         goto error;
     }
 
-    if (VIR_REALLOC_N(doms->objs, doms->count + 1) < 0) {
+    if (virHashAddEntry(doms->objs, uuidstr, obj) < 0) {
         virReportOOMError(conn);
         goto error;
     }
 
-    doms->objs[doms->count] = obj;
-    doms->count++;
-
     if (notify)
         (*notify)(obj, 1, opaque);
 
@@ -4995,4 +5005,106 @@ void virDomainObjUnlock(virDomainObjPtr obj)
     virMutexUnlock(&obj->lock);
 }
 
+
+static void virDomainObjListCountActive(void *payload, const char *name ATTRIBUTE_UNUSED, void *data)
+{
+    virDomainObjPtr obj = payload;
+    int *count = data;
+    virDomainObjLock(obj);
+    if (virDomainIsActive(obj))
+        (*count)++;
+    virDomainObjUnlock(obj);
+}
+
+static void virDomainObjListCountInactive(void *payload, const char *name ATTRIBUTE_UNUSED, void *data)
+{
+    virDomainObjPtr obj = payload;
+    int *count = data;
+    virDomainObjLock(obj);
+    if (!virDomainIsActive(obj))
+        (*count)++;
+    virDomainObjUnlock(obj);
+}
+
+int virDomainObjListNumOfDomains(virDomainObjListPtr doms, int active)
+{
+    int count = 0;
+    if (active)
+        virHashForEach(doms->objs, virDomainObjListCountActive, &count);
+    else
+        virHashForEach(doms->objs, virDomainObjListCountInactive, &count);
+    return count;
+}
+
+struct virDomainIDData {
+    int numids;
+    int maxids;
+    int *ids;
+};
+
+static void virDomainObjListCopyActiveIDs(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque)
+{
+    virDomainObjPtr obj = payload;
+    struct virDomainIDData *data = opaque;
+    virDomainObjLock(obj);
+    if (virDomainIsActive(obj) && data->numids < data->maxids)
+        data->ids[data->numids++] = obj->def->id;
+    virDomainObjUnlock(obj);
+}
+
+int virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
+                                 int *ids,
+                                 int maxids)
+{
+    struct virDomainIDData data = { 0, maxids, ids };
+    virHashForEach(doms->objs, virDomainObjListCopyActiveIDs, &data);
+    return data.numids;
+}
+
+struct virDomainNameData {
+    int oom;
+    int numnames;
+    int maxnames;
+    char **const names;
+};
+
+static void virDomainObjListCopyInactiveNames(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque)
+{
+    virDomainObjPtr obj = payload;
+    struct virDomainNameData *data = opaque;
+
+    if (data->oom)
+        return;
+
+    virDomainObjLock(obj);
+    if (!virDomainIsActive(obj) && data->numnames < data->maxnames) {
+        if (!(data->names[data->numnames] = strdup(obj->def->name)))
+            data->oom = 1;
+        else
+            data->numnames++;
+    }
+    virDomainObjUnlock(obj);
+}
+
+
+int virDomainObjListGetInactiveNames(virDomainObjListPtr doms,
+                                     char **const names,
+                                     int maxnames)
+{
+    struct virDomainNameData data = { 0, 0, maxnames, names };
+    int i;
+    virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, &data);
+    if (data.oom) {
+        virReportOOMError(NULL);
+        goto cleanup;
+    }
+
+    return data.numnames;
+
+cleanup:
+    for (i = 0 ; i < data.numnames ; i++)
+        VIR_FREE(data.names[i]);
+    return -1;
+}
+
 #endif /* ! PROXY */
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ff1b0cf..389e259 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -33,6 +33,7 @@
 #include "storage_encryption_conf.h"
 #include "util.h"
 #include "threads.h"
+#include "hash.h"
 
 /* Private component of virDomainXMLFlags */
 typedef enum {
@@ -640,8 +641,9 @@ struct _virDomainObj {
 typedef struct _virDomainObjList virDomainObjList;
 typedef virDomainObjList *virDomainObjListPtr;
 struct _virDomainObjList {
-    unsigned int count;
-    virDomainObjPtr *objs;
+    /* uuid string -> virDomainObj  mapping
+     * for O(1), lockless lookup-by-uuid */
+    virHashTable *objs;
 };
 
 static inline int
@@ -650,6 +652,8 @@ virDomainIsActive(virDomainObjPtr dom)
     return dom->def->id != -1;
 }
 
+int virDomainObjListInit(virDomainObjListPtr objs);
+void virDomainObjListDeinit(virDomainObjListPtr objs);
 
 virDomainObjPtr virDomainFindByID(const virDomainObjListPtr doms,
                                   int id);
@@ -672,7 +676,6 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
 void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
 void virDomainDefFree(virDomainDefPtr vm);
 void virDomainObjFree(virDomainObjPtr vm);
-void virDomainObjListFree(virDomainObjListPtr vms);
 
 virDomainObjPtr virDomainAssignDef(virConnectPtr conn,
                                    virDomainObjListPtr doms,
@@ -784,6 +787,16 @@ int virDomainVideoDefaultRAM(virDomainDefPtr def, int type);
 void virDomainObjLock(virDomainObjPtr obj);
 void virDomainObjUnlock(virDomainObjPtr obj);
 
+int virDomainObjListNumOfDomains(virDomainObjListPtr doms, int active);
+
+int virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
+                                 int *ids,
+                                 int maxids);
+int virDomainObjListGetInactiveNames(virDomainObjListPtr doms,
+                                     char **const names,
+                                     int maxnames);
+
+
 VIR_ENUM_DECL(virDomainVirt)
 VIR_ENUM_DECL(virDomainBoot)
 VIR_ENUM_DECL(virDomainFeature)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 98ea7f8..bd9d84a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -124,7 +124,6 @@ virDomainLoadAllConfigs;
 virDomainNetDefFree;
 virDomainNetTypeToString;
 virDomainObjFree;
-virDomainObjListFree;
 virDomainRemoveInactive;
 virDomainSaveXML;
 virDomainSaveConfig;
@@ -147,6 +146,11 @@ virDomainObjLock;
 virDomainObjUnlock;
 virDomainStateTypeToString;
 virDomainStateTypeFromString;
+virDomainObjListGetInactiveNames;
+virDomainObjListGetActiveIDs;
+virDomainObjListNumOfDomains;
+virDomainObjListInit;
+virDomainObjListDeinit;
 
 
 # domain_event.h
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index ef97364..4f0787b 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -220,31 +220,21 @@ cleanup:
 
 static int lxcListDomains(virConnectPtr conn, int *ids, int nids) {
     lxc_driver_t *driver = conn->privateData;
-    int got = 0, i;
+    int n;
 
     lxcDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count && got < nids ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (virDomainIsActive(driver->domains.objs[i]))
-            ids[got++] = driver->domains.objs[i]->def->id;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListGetActiveIDs(&driver->domains, ids, nids);
     lxcDriverUnlock(driver);
 
-    return got;
+    return n;
 }
 
 static int lxcNumDomains(virConnectPtr conn) {
     lxc_driver_t *driver = conn->privateData;
-    int n = 0, i;
+    int n;
 
     lxcDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (virDomainIsActive(driver->domains.objs[i]))
-            n++;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListNumOfDomains(&driver->domains, 1);
     lxcDriverUnlock(driver);
 
     return n;
@@ -253,43 +243,22 @@ static int lxcNumDomains(virConnectPtr conn) {
 static int lxcListDefinedDomains(virConnectPtr conn,
                                  char **const names, int nnames) {
     lxc_driver_t *driver = conn->privateData;
-    int got = 0, i;
+    int n;
 
     lxcDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count && got < nnames ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (!virDomainIsActive(driver->domains.objs[i])) {
-            if (!(names[got++] = strdup(driver->domains.objs[i]->def->name))) {
-                virReportOOMError(conn);
-                virDomainObjUnlock(driver->domains.objs[i]);
-                goto cleanup;
-            }
-        }
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListGetInactiveNames(&driver->domains, names, nnames);
     lxcDriverUnlock(driver);
 
-    return got;
-
- cleanup:
-    for (i = 0 ; i < got ; i++)
-        VIR_FREE(names[i]);
-    lxcDriverUnlock(driver);
-    return -1;
+    return n;
 }
 
 
 static int lxcNumDefinedDomains(virConnectPtr conn) {
     lxc_driver_t *driver = conn->privateData;
-    int n = 0, i;
+    int n;
 
     lxcDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (!virDomainIsActive(driver->domains.objs[i]))
-            n++;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListNumOfDomains(&driver->domains, 0);
     lxcDriverUnlock(driver);
 
     return n;
@@ -898,27 +867,15 @@ static void lxcMonitorEvent(int watch,
                             int events ATTRIBUTE_UNUSED,
                             void *data)
 {
-    lxc_driver_t *driver = data;
-    virDomainObjPtr vm = NULL;
+    lxc_driver_t *driver = lxc_driver;
+    virDomainObjPtr vm = data;
     virDomainEventPtr event = NULL;
-    unsigned int i;
 
     lxcDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjPtr tmpvm = driver->domains.objs[i];
-        virDomainObjLock(tmpvm);
-        if (tmpvm->monitorWatch == watch) {
-            vm = tmpvm;
-            break;
-        }
-        virDomainObjUnlock(tmpvm);
-    }
-    if (!vm) {
-        virEventRemoveHandle(watch);
-        goto cleanup;
-    }
+    virDomainObjLock(vm);
+    lxcDriverUnlock(driver);
 
-    if (vm->monitor != fd) {
+    if (vm->monitor != fd || vm->monitorWatch != watch) {
         virEventRemoveHandle(watch);
         goto cleanup;
     }
@@ -936,11 +893,12 @@ static void lxcMonitorEvent(int watch,
     }
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
-    if (event)
+    virDomainObjUnlock(vm);
+    if (event) {
+        lxcDriverLock(driver);
         lxcDomainEventQueue(driver, event);
-    lxcDriverUnlock(driver);
+        lxcDriverUnlock(driver);
+    }
 }
 
 
@@ -1572,9 +1530,40 @@ static int lxcCheckNetNsSupport(void)
 }
 
 
+struct lxcAutostartData {
+    lxc_driver_t *driver;
+    virConnectPtr conn;
+};
+
+static void
+lxcAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque)
+{
+    virDomainObjPtr vm = payload;
+    const struct lxcAutostartData *data = opaque;
+
+    virDomainObjLock(vm);
+    if (vm->autostart &&
+        !virDomainIsActive(vm)) {
+        int ret = lxcVmStart(data->conn, data->driver, vm);
+        if (ret < 0) {
+            virErrorPtr err = virGetLastError();
+            VIR_ERROR(_("Failed to autostart VM '%s': %s\n"),
+                      vm->def->name,
+                      err ? err->message : "");
+        } else {
+            virDomainEventPtr event =
+                virDomainEventNewFromObj(vm,
+                                         VIR_DOMAIN_EVENT_STARTED,
+                                         VIR_DOMAIN_EVENT_STARTED_BOOTED);
+            if (event)
+                lxcDomainEventQueue(data->driver, event);
+        }
+    }
+    virDomainObjUnlock(vm);
+}
+
 static void
 lxcAutostartConfigs(lxc_driver_t *driver) {
-    unsigned int i;
     /* XXX: Figure out a better way todo this. The domain
      * startup code needs a connection handle in order
      * to lookup the bridge associated with a virtual
@@ -1583,39 +1572,65 @@ lxcAutostartConfigs(lxc_driver_t *driver) {
     virConnectPtr conn = virConnectOpen("lxc:///");
     /* Ignoring NULL conn which is mostly harmless here */
 
+    struct lxcAutostartData data = { driver, conn };
+
     lxcDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjPtr vm = driver->domains.objs[i];
-        virDomainObjLock(vm);
-        if (vm->autostart &&
-            !virDomainIsActive(vm)) {
-            int ret = lxcVmStart(conn, driver, vm);
-            if (ret < 0) {
-                virErrorPtr err = virGetLastError();
-                VIR_ERROR(_("Failed to autostart VM '%s': %s\n"),
-                          vm->def->name,
-                          err ? err->message : "");
-            } else {
-                virDomainEventPtr event =
-                    virDomainEventNewFromObj(vm,
-                                             VIR_DOMAIN_EVENT_STARTED,
-                                             VIR_DOMAIN_EVENT_STARTED_BOOTED);
-                if (event)
-                    lxcDomainEventQueue(driver, event);
-            }
-        }
-        virDomainObjUnlock(vm);
-    }
+    virHashForEach(driver->domains.objs, lxcAutostartDomain, &data);
     lxcDriverUnlock(driver);
 
     if (conn)
         virConnectClose(conn);
 }
 
+static void
+lxcReconnectVM(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque)
+{
+    virDomainObjPtr vm = payload;
+    lxc_driver_t *driver = opaque;
+    char *config = NULL;
+    virDomainDefPtr tmp;
+
+    virDomainObjLock(vm);
+    if ((vm->monitor = lxcMonitorClient(NULL, driver, vm)) < 0) {
+        goto cleanup;
+    }
+
+    /* Read pid from controller */
+    if ((virFileReadPid(lxc_driver->stateDir, vm->def->name, &vm->pid)) != 0) {
+        close(vm->monitor);
+        vm->monitor = -1;
+        goto cleanup;
+    }
+
+    if ((config = virDomainConfigFile(NULL,
+                                      driver->stateDir,
+                                      vm->def->name)) == NULL)
+        goto cleanup;
+
+    /* Try and load the live config */
+    tmp = virDomainDefParseFile(NULL, driver->caps, config, 0);
+    VIR_FREE(config);
+    if (tmp) {
+        vm->newDef = vm->def;
+        vm->def = tmp;
+    }
+
+    if (vm->pid != 0) {
+        vm->def->id = vm->pid;
+        vm->state = VIR_DOMAIN_RUNNING;
+    } else {
+        vm->def->id = -1;
+        close(vm->monitor);
+        vm->monitor = -1;
+    }
+
+cleanup:
+    virDomainObjUnlock(vm);
+}
+
 
 static int lxcStartup(int privileged)
 {
-    unsigned int i;
     char *ld;
     int rc;
 
@@ -1647,6 +1662,9 @@ static int lxcStartup(int privileged)
         goto cleanup;
     }
 
+    if (virDomainObjListInit(&lxc_driver->domains) < 0)
+        goto cleanup;
+
     if (VIR_ALLOC(lxc_driver->domainEventCallbacks) < 0)
         goto cleanup;
     if (!(lxc_driver->domainEventQueue = virDomainEventQueueNew()))
@@ -1681,50 +1699,7 @@ static int lxcStartup(int privileged)
                                 0, NULL, NULL) < 0)
         goto cleanup;
 
-    for (i = 0 ; i < lxc_driver->domains.count ; i++) {
-        virDomainObjPtr vm = lxc_driver->domains.objs[i];
-        char *config = NULL;
-        virDomainDefPtr tmp;
-
-        virDomainObjLock(vm);
-        if ((vm->monitor = lxcMonitorClient(NULL, lxc_driver, vm)) < 0) {
-            virDomainObjUnlock(vm);
-            continue;
-        }
-
-        /* Read pid from controller */
-        if ((rc = virFileReadPid(lxc_driver->stateDir, vm->def->name, &vm->pid)) != 0) {
-            close(vm->monitor);
-            vm->monitor = -1;
-            virDomainObjUnlock(vm);
-            continue;
-        }
-
-        if ((config = virDomainConfigFile(NULL,
-                                          lxc_driver->stateDir,
-                                          vm->def->name)) == NULL) {
-            virDomainObjUnlock(vm);
-            continue;
-        }
-
-        /* Try and load the live config */
-        tmp = virDomainDefParseFile(NULL, lxc_driver->caps, config, 0);
-        VIR_FREE(config);
-        if (tmp) {
-            vm->newDef = vm->def;
-            vm->def = tmp;
-        }
-
-        if (vm->pid != 0) {
-            vm->def->id = vm->pid;
-            vm->state = VIR_DOMAIN_RUNNING;
-        } else {
-            vm->def->id = -1;
-            close(vm->monitor);
-            vm->monitor = -1;
-        }
-        virDomainObjUnlock(vm);
-    }
+    virHashForEach(lxc_driver->domains.objs, lxcReconnectVM, lxc_driver);
 
     lxcDriverUnlock(lxc_driver);
     return 0;
@@ -1780,7 +1755,7 @@ static int lxcShutdown(void)
         return(-1);
 
     lxcDriverLock(lxc_driver);
-    virDomainObjListFree(&lxc_driver->domains);
+    virDomainObjListDeinit(&lxc_driver->domains);
 
     virDomainEventCallbackListFree(lxc_driver->domainEventCallbacks);
     virDomainEventQueueFree(lxc_driver->domainEventQueue);
@@ -1809,19 +1784,13 @@ static int lxcShutdown(void)
  */
 static int
 lxcActive(void) {
-    unsigned int i;
-    int active = 0;
+    int active;
 
     if (lxc_driver == NULL)
         return(0);
 
     lxcDriverLock(lxc_driver);
-    for (i = 0 ; i < lxc_driver->domains.count ; i++) {
-        virDomainObjLock(lxc_driver->domains.objs[i]);
-        if (virDomainIsActive(lxc_driver->domains.objs[i]))
-            active = 1;
-        virDomainObjUnlock(lxc_driver->domains.objs[i]);
-    }
+    active = virDomainObjListNumOfDomains(&lxc_driver->domains, 1);
     lxcDriverUnlock(lxc_driver);
 
     return active;
diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c
index 9bcd5c3..0b807ad 100644
--- a/src/opennebula/one_driver.c
+++ b/src/opennebula/one_driver.c
@@ -178,32 +178,22 @@ return_point:
 static int oneListDomains(virConnectPtr conn, int *ids, int nids)
 {
     one_driver_t *driver = (one_driver_t *)conn->privateData;
-    int got = 0, i;
+    int n;
 
     oneDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count && got < nids ; i++){
-        virDomainObjLock(driver->domains.objs[i]);
-        if (virDomainIsActive(driver->domains.objs[i]))
-            ids[got++] = driver->domains.objs[i]->def->id;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListGetActiveIDs(&driver->domains, ids, nids);
     oneDriverUnlock(driver);
 
-    return got;
+    return n;
 }
 
 static int oneNumDomains(virConnectPtr conn)
 {
     one_driver_t *driver = (one_driver_t *)conn->privateData;
-    int n = 0, i;
+    int n;
 
     oneDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++){
-        virDomainObjLock(driver->domains.objs[i]);
-        if (virDomainIsActive(driver->domains.objs[i]))
-            n++;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListNumOfDomains(&driver->domains, 1);
     oneDriverUnlock(driver);
 
     return n;
@@ -212,44 +202,22 @@ static int oneNumDomains(virConnectPtr conn)
 static int oneListDefinedDomains(virConnectPtr conn,
                                  char **const names, int nnames) {
     one_driver_t *driver = (one_driver_t *)conn->privateData;
-    int got = 0, i;
+    int n;
 
     oneDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count && got < nnames ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (!virDomainIsActive(driver->domains.objs[i])) {
-            if (!(names[got++] = strdup(driver->domains.objs[i]->def->name))) {
-                virReportOOMError(conn);
-                virDomainObjUnlock(driver->domains.objs[i]);
-                goto cleanup;
-            }
-        }
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListGetInactiveNames(&driver->domains, names, nnames);
     oneDriverUnlock(driver);
 
-    return got;
-
-cleanup:
-    for (i = 0 ; i < got ; i++)
-        VIR_FREE(names[i]);
-    oneDriverUnlock(driver);
-
-    return -1;
+    return n;
 }
 
 static int oneNumDefinedDomains(virConnectPtr conn)
 {
     one_driver_t *driver = (one_driver_t *)conn->privateData;
-    int n = 0, i;
+    int n;
 
     oneDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++){
-        virDomainObjLock(driver->domains.objs[i]);
-        if (!virDomainIsActive(driver->domains.objs[i]))
-            n++;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListNumOfDomains(&driver->domains, 0);
     oneDriverUnlock(driver);
 
     return n;
@@ -647,6 +615,12 @@ static int oneStartup(int privileged ATTRIBUTE_UNUSED){
         return -1;
     }
 
+    if (virDomainObjListInit(&one_driver->domains) < 0) {
+        virMutexDestroy(&one_driver->lock);
+        VIR_FREE(one_driver);
+        return -1;
+    }
+
     c_oneStart();
     oneDriverLock(one_driver);
     one_driver->nextid=1;
@@ -665,7 +639,7 @@ static int oneShutdown(void){
         return(-1);
 
     oneDriverLock(one_driver);
-    virDomainObjListFree(&one_driver->domains);
+    virDomainObjListDeinit(&one_driver->domains);
 
     virCapabilitiesFree(one_driver->caps);
     oneDriverUnlock(one_driver);
@@ -677,19 +651,13 @@ static int oneShutdown(void){
 }
 
 static int oneActive(void){
-    unsigned int i;
     int active = 0;
 
     if (one_driver == NULL)
         return(0);
 
     oneDriverLock(one_driver);
-    for (i = 0 ; i < one_driver->domains.count ; i++) {
-        virDomainObjLock(one_driver->domains.objs[i]);
-        if (virDomainIsActive(one_driver->domains.objs[i]))
-            active = 1;
-        virDomainObjUnlock(one_driver->domains.objs[i]);
-    }
+    active = virDomainObjListNumOfDomains(&one_driver->domains, 1);
     oneDriverUnlock(one_driver);
 
     return active;
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 403f537..c928afb 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -421,7 +421,7 @@ openvzFreeDriver(struct openvz_driver *driver)
     if (!driver)
         return;
 
-    virDomainObjListFree(&driver->domains);
+    virDomainObjListDeinit(&driver->domains);
     virCapabilitiesFree(driver->caps);
 }
 
@@ -509,11 +509,10 @@ int openvzLoadDomains(struct openvz_driver *driver) {
         openvzReadNetworkConf(NULL, dom->def, veid);
         openvzReadFSConf(NULL, dom->def, veid);
 
-        if (VIR_REALLOC_N(driver->domains.objs,
-                          driver->domains.count + 1) < 0)
+        virUUIDFormat(dom->def->uuid, uuidstr);
+        if (virHashAddEntry(driver->domains.objs, uuidstr, dom) < 0)
             goto no_memory;
 
-        driver->domains.objs[driver->domains.count++] = dom;
         dom = NULL;
     }
 
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index f64ad1e..b0092cd 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1168,6 +1168,9 @@ static virDrvOpenStatus openvzOpen(virConnectPtr conn,
         return VIR_DRV_OPEN_ERROR;
     }
 
+    if (virDomainObjListInit(&driver->domains) < 0)
+        goto cleanup;
+
     if (!(driver->caps = openvzCapsInit()))
         goto cleanup;
 
@@ -1247,18 +1250,13 @@ static int openvzListDomains(virConnectPtr conn, int *ids, int nids) {
 
 static int openvzNumDomains(virConnectPtr conn) {
     struct openvz_driver *driver = conn->privateData;
-    int nactive = 0, i;
+    int n;
 
     openvzDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (virDomainIsActive(driver->domains.objs[i]))
-            nactive++;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListNumOfDomains(&driver->domains, 1);
     openvzDriverUnlock(driver);
 
-    return nactive;
+    return n;
 }
 
 static int openvzListDefinedDomains(virConnectPtr conn,
@@ -1350,18 +1348,13 @@ Version: 2.2
 
 static int openvzNumDefinedDomains(virConnectPtr conn) {
     struct openvz_driver *driver =  conn->privateData;
-    int ninactive = 0, i;
+    int n;
 
     openvzDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (!virDomainIsActive(driver->domains.objs[i]))
-            ninactive++;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListNumOfDomains(&driver->domains, 0);
     openvzDriverUnlock(driver);
 
-    return ninactive;
+    return n;
 }
 
 static virDriver openvzDriver = {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d37b184..f977247 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -201,9 +201,42 @@ qemudLogReadFD(virConnectPtr conn, const char* logDir, const char* name, off_t p
 }
 
 
+struct qemuAutostartData {
+    struct qemud_driver *driver;
+    virConnectPtr conn;
+};
+static void
+qemuAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque)
+{
+    virDomainObjPtr vm = payload;
+    struct qemuAutostartData *data = opaque;
+
+    virDomainObjLock(vm);
+    if (vm->autostart &&
+        !virDomainIsActive(vm)) {
+        int ret;
+
+        virResetLastError();
+        ret = qemudStartVMDaemon(data->conn, data->driver, vm, NULL, -1);
+        if (ret < 0) {
+            virErrorPtr err = virGetLastError();
+            VIR_ERROR(_("Failed to autostart VM '%s': %s\n"),
+                      vm->def->name,
+                      err ? err->message : "");
+        } else {
+            virDomainEventPtr event =
+                virDomainEventNewFromObj(vm,
+                                         VIR_DOMAIN_EVENT_STARTED,
+                                         VIR_DOMAIN_EVENT_STARTED_BOOTED);
+            if (event)
+                qemuDomainEventQueue(data->driver, event);
+        }
+    }
+    virDomainObjUnlock(vm);
+}
+
 static void
 qemudAutostartConfigs(struct qemud_driver *driver) {
-    unsigned int i;
     /* XXX: Figure out a better way todo this. The domain
      * startup code needs a connection handle in order
      * to lookup the bridge associated with a virtual
@@ -213,33 +246,10 @@ qemudAutostartConfigs(struct qemud_driver *driver) {
                                         "qemu:///system" :
                                         "qemu:///session");
     /* Ignoring NULL conn which is mostly harmless here */
+    struct qemuAutostartData data = { driver, conn };
 
     qemuDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjPtr vm = driver->domains.objs[i];
-        virDomainObjLock(vm);
-        if (vm->autostart &&
-            !virDomainIsActive(vm)) {
-            int ret;
-
-            virResetLastError();
-            ret = qemudStartVMDaemon(conn, driver, vm, NULL, -1);
-            if (ret < 0) {
-                virErrorPtr err = virGetLastError();
-                VIR_ERROR(_("Failed to autostart VM '%s': %s\n"),
-                          vm->def->name,
-                          err ? err->message : "");
-            } else {
-                virDomainEventPtr event =
-                    virDomainEventNewFromObj(vm,
-                                             VIR_DOMAIN_EVENT_STARTED,
-                                             VIR_DOMAIN_EVENT_STARTED_BOOTED);
-                if (event)
-                    qemuDomainEventQueue(driver, event);
-            }
-        }
-        virDomainObjUnlock(vm);
-    }
+    virHashForEach(driver->domains.objs, qemuAutostartDomain, &data);
     qemuDriverUnlock(driver);
 
     if (conn)
@@ -284,7 +294,6 @@ cleanup:
 
 
 static int qemudOpenMonitor(virConnectPtr conn,
-                            struct qemud_driver* driver,
                             virDomainObjPtr vm,
                             int reconnect);
 
@@ -293,13 +302,16 @@ static int qemudOpenMonitor(virConnectPtr conn,
  * Open an existing VM's monitor, re-detect VCPU threads
  * and re-reserve the security labels in use
  */
-static int
-qemuReconnectDomain(struct qemud_driver *driver,
-                    virDomainObjPtr obj)
+static void
+qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque)
 {
     int rc;
+    virDomainObjPtr obj = payload;
+    struct qemud_driver *driver = opaque;
+
+    virDomainObjLock(obj);
 
-    if ((rc = qemudOpenMonitor(NULL, driver, obj, 1)) != 0) {
+    if ((rc = qemudOpenMonitor(NULL, obj, 1)) != 0) {
         VIR_ERROR(_("Failed to reconnect monitor for %s: %d\n"),
                   obj->def->name, rc);
         goto error;
@@ -313,15 +325,20 @@ qemuReconnectDomain(struct qemud_driver *driver,
         driver->securityDriver &&
         driver->securityDriver->domainReserveSecurityLabel &&
         driver->securityDriver->domainReserveSecurityLabel(NULL, obj) < 0)
-        return -1;
+        goto error;
 
     if (obj->def->id >= driver->nextvmid)
         driver->nextvmid = obj->def->id + 1;
 
-    return 0;
+    virDomainObjUnlock(obj);
+    return;
 
 error:
-    return -1;
+    /* We can't get the monitor back, so must kill the VM
+     * to remove danger of it ending up running twice if
+     * user tries to start it again later */
+    qemudShutdownVMDaemon(NULL, driver, obj);
+    virDomainObjUnlock(obj);
 }
 
 /**
@@ -333,20 +350,7 @@ error:
 static void
 qemuReconnectDomains(struct qemud_driver *driver)
 {
-    int i;
-
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjPtr obj = driver->domains.objs[i];
-
-        virDomainObjLock(obj);
-        if (qemuReconnectDomain(driver, obj) < 0) {
-            /* If we can't get the monitor back, then kill the VM
-             * so user has ability to start it again later without
-             * danger of ending up running twice */
-            qemudShutdownVMDaemon(NULL, driver, obj);
-        }
-        virDomainObjUnlock(obj);
-    }
+    virHashForEach(driver->domains.objs, qemuReconnectDomain, driver);
 }
 
 
@@ -438,8 +442,11 @@ qemudStartup(int privileged) {
     /* Don't have a dom0 so start from 1 */
     qemu_driver->nextvmid = 1;
 
+    if (virDomainObjListInit(&qemu_driver->domains) < 0)
+        goto out_of_memory;
+
     /* Init callback list */
-    if(VIR_ALLOC(qemu_driver->domainEventCallbacks) < 0)
+    if (VIR_ALLOC(qemu_driver->domainEventCallbacks) < 0)
         goto out_of_memory;
     if (!(qemu_driver->domainEventQueue = virDomainEventQueueNew()))
         goto out_of_memory;
@@ -679,21 +686,14 @@ qemudReload(void) {
  */
 static int
 qemudActive(void) {
-    unsigned int i;
     int active = 0;
 
     if (!qemu_driver)
         return 0;
 
+    /* XXX having to iterate here is not great because it requires many locks */
     qemuDriverLock(qemu_driver);
-    for (i = 0 ; i < qemu_driver->domains.count ; i++) {
-        virDomainObjPtr vm = qemu_driver->domains.objs[i];
-        virDomainObjLock(vm);
-        if (virDomainIsActive(vm))
-            active = 1;
-        virDomainObjUnlock(vm);
-    }
-
+    active = virDomainObjListNumOfDomains(&qemu_driver->domains, 1);
     qemuDriverUnlock(qemu_driver);
     return active;
 }
@@ -713,7 +713,7 @@ qemudShutdown(void) {
     pciDeviceListFree(NULL, qemu_driver->activePciHostdevs);
     virCapabilitiesFree(qemu_driver->caps);
 
-    virDomainObjListFree(&qemu_driver->domains);
+    virDomainObjListDeinit(&qemu_driver->domains);
 
     VIR_FREE(qemu_driver->securityDriverName);
     VIR_FREE(qemu_driver->logDir);
@@ -913,7 +913,6 @@ qemudCheckMonitorPrompt(virConnectPtr conn ATTRIBUTE_UNUSED,
 
 static int
 qemudOpenMonitorCommon(virConnectPtr conn,
-                       struct qemud_driver* driver,
                        virDomainObjPtr vm,
                        int monfd,
                        int reconnect)
@@ -952,7 +951,7 @@ qemudOpenMonitorCommon(virConnectPtr conn,
     if ((vm->monitorWatch = virEventAddHandle(vm->monitor,
                                               VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR,
                                               qemudDispatchVMEvent,
-                                              driver, NULL)) < 0)
+                                              vm, NULL)) < 0)
         return -1;
 
     return 0;
@@ -960,7 +959,6 @@ qemudOpenMonitorCommon(virConnectPtr conn,
 
 static int
 qemudOpenMonitorUnix(virConnectPtr conn,
-                     struct qemud_driver* driver,
                      virDomainObjPtr vm,
                      const char *monitor,
                      int reconnect)
@@ -1008,7 +1006,7 @@ qemudOpenMonitorUnix(virConnectPtr conn,
         goto error;
     }
 
-    if (qemudOpenMonitorCommon(conn, driver, vm, monfd, reconnect) < 0)
+    if (qemudOpenMonitorCommon(conn, vm, monfd, reconnect) < 0)
         goto error;
 
     return 0;
@@ -1020,7 +1018,6 @@ error:
 
 static int
 qemudOpenMonitorPty(virConnectPtr conn,
-                    struct qemud_driver* driver,
                     virDomainObjPtr vm,
                     const char *monitor,
                     int reconnect)
@@ -1033,7 +1030,7 @@ qemudOpenMonitorPty(virConnectPtr conn,
         return -1;
     }
 
-    if (qemudOpenMonitorCommon(conn, driver, vm, monfd, reconnect) < 0)
+    if (qemudOpenMonitorCommon(conn, vm, monfd, reconnect) < 0)
         goto error;
 
     return 0;
@@ -1045,17 +1042,16 @@ error:
 
 static int
 qemudOpenMonitor(virConnectPtr conn,
-                 struct qemud_driver *driver,
                  virDomainObjPtr vm,
                  int reconnect)
 {
     switch (vm->monitor_chr->type) {
     case VIR_DOMAIN_CHR_TYPE_UNIX:
-        return qemudOpenMonitorUnix(conn, driver, vm,
+        return qemudOpenMonitorUnix(conn, vm,
                                     vm->monitor_chr->data.nix.path,
                                     reconnect);
     case VIR_DOMAIN_CHR_TYPE_PTY:
-        return qemudOpenMonitorPty(conn, driver, vm,
+        return qemudOpenMonitorPty(conn, vm,
                                    vm->monitor_chr->data.file.path,
                                    reconnect);
     default:
@@ -1177,7 +1173,7 @@ qemudWaitForMonitor(virConnectPtr conn,
         return -1;
     }
 
-    if (qemudOpenMonitor(conn, driver, vm, 0) < 0)
+    if (qemudOpenMonitor(conn, vm, 0) < 0)
         return -1;
 
     return 0;
@@ -2255,28 +2251,22 @@ retry:
 
 static void
 qemudDispatchVMEvent(int watch, int fd, int events, void *opaque) {
-    struct qemud_driver *driver = opaque;
-    virDomainObjPtr vm = NULL;
+    struct qemud_driver *driver = qemu_driver;
+    virDomainObjPtr vm = opaque;
     virDomainEventPtr event = NULL;
-    unsigned int i;
     int quit = 0, failed = 0;
 
+    /* XXX Normally we have to lock the driver first, to protect
+     * against someone adding/removing the domain. We know,
+     * however, then if we're getting data in this callback
+     * the VM must be running. Nowhere is allowed to remove
+     * a domain while it is running, so it is safe to not
+     * lock the driver here... */
     qemuDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjPtr tmpvm = driver->domains.objs[i];
-        virDomainObjLock(tmpvm);
-        if (virDomainIsActive(tmpvm) &&
-            tmpvm->monitorWatch == watch) {
-            vm = tmpvm;
-            break;
-        }
-        virDomainObjUnlock(tmpvm);
-    }
-
-    if (!vm)
-        goto cleanup;
+    virDomainObjLock(vm);
+    qemuDriverUnlock(driver);
 
-    if (vm->monitor != fd) {
+    if (vm->monitor != fd || vm->monitorWatch != watch) {
         failed = 1;
     } else {
         if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR))
@@ -2302,12 +2292,12 @@ qemudDispatchVMEvent(int watch, int fd, int events, void *opaque) {
         }
     }
 
-cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
-    if (event)
+    virDomainObjUnlock(vm);
+    if (event) {
+        qemuDriverLock(driver);
         qemuDomainEventQueue(driver, event);
-    qemuDriverUnlock(driver);
+        qemuDriverUnlock(driver);
+    }
 }
 
 
@@ -2637,31 +2627,21 @@ qemudGetHostname (virConnectPtr conn)
 
 static int qemudListDomains(virConnectPtr conn, int *ids, int nids) {
     struct qemud_driver *driver = conn->privateData;
-    int got = 0, i;
+    int n;
 
     qemuDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count && got < nids ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (virDomainIsActive(driver->domains.objs[i]))
-            ids[got++] = driver->domains.objs[i]->def->id;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListGetActiveIDs(&driver->domains, ids, nids);
     qemuDriverUnlock(driver);
 
-    return got;
+    return n;
 }
 
 static int qemudNumDomains(virConnectPtr conn) {
     struct qemud_driver *driver = conn->privateData;
-    int n = 0, i;
+    int n;
 
     qemuDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (virDomainIsActive(driver->domains.objs[i]))
-            n++;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListNumOfDomains(&driver->domains, 1);
     qemuDriverUnlock(driver);
 
     return n;
@@ -4059,39 +4039,20 @@ cleanup:
 static int qemudListDefinedDomains(virConnectPtr conn,
                             char **const names, int nnames) {
     struct qemud_driver *driver = conn->privateData;
-    int got = 0, i;
+    int n;
 
     qemuDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count && got < nnames ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (!virDomainIsActive(driver->domains.objs[i])) {
-            if (!(names[got++] = strdup(driver->domains.objs[i]->def->name))) {
-                virReportOOMError(conn);
-                virDomainObjUnlock(driver->domains.objs[i]);
-                goto cleanup;
-            }
-        }
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
-
+    n = virDomainObjListGetInactiveNames(&driver->domains, names, nnames);
     qemuDriverUnlock(driver);
-    return got;
-
- cleanup:
-    for (i = 0 ; i < got ; i++)
-        VIR_FREE(names[i]);
-    qemuDriverUnlock(driver);
-    return -1;
+    return n;
 }
 
 static int qemudNumDefinedDomains(virConnectPtr conn) {
     struct qemud_driver *driver = conn->privateData;
-    int n = 0, i;
+    int n;
 
     qemuDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++)
-        if (!virDomainIsActive(driver->domains.objs[i]))
-            n++;
+    n = virDomainObjListNumOfDomains(&driver->domains, 0);
     qemuDriverUnlock(driver);
 
     return n;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 0541a73..919a2f6 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -338,6 +338,9 @@ static int testOpenDefault(virConnectPtr conn) {
         goto error;
     }
 
+    if (virDomainObjListInit(&privconn->domains) < 0)
+        goto error;
+
     memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo));
 
     // Numa setup
@@ -419,7 +422,7 @@ static int testOpenDefault(virConnectPtr conn) {
     return VIR_DRV_OPEN_SUCCESS;
 
 error:
-    virDomainObjListFree(&privconn->domains);
+    virDomainObjListDeinit(&privconn->domains);
     virNetworkObjListFree(&privconn->networks);
     virInterfaceObjListFree(&privconn->ifaces);
     virStoragePoolObjListFree(&privconn->pools);
@@ -569,6 +572,9 @@ static int testOpenFromFile(virConnectPtr conn,
     testDriverLock(privconn);
     conn->privateData = privconn;
 
+    if (virDomainObjListInit(&privconn->domains) < 0)
+        goto error;
+
     if (!(privconn->caps = testBuildCapabilities(conn)))
         goto error;
 
@@ -897,7 +903,7 @@ static int testOpenFromFile(virConnectPtr conn,
     VIR_FREE(pools);
     if (fd != -1)
         close(fd);
-    virDomainObjListFree(&privconn->domains);
+    virDomainObjListDeinit(&privconn->domains);
     virNetworkObjListFree(&privconn->networks);
     virInterfaceObjListFree(&privconn->ifaces);
     virStoragePoolObjListFree(&privconn->pools);
@@ -966,7 +972,7 @@ static int testClose(virConnectPtr conn)
     testConnPtr privconn = conn->privateData;
     testDriverLock(privconn);
     virCapabilitiesFree(privconn->caps);
-    virDomainObjListFree(&privconn->domains);
+    virDomainObjListDeinit(&privconn->domains);
     virNetworkObjListFree(&privconn->networks);
     virInterfaceObjListFree(&privconn->ifaces);
     virStoragePoolObjListFree(&privconn->pools);
@@ -1036,15 +1042,13 @@ static char *testGetCapabilities (virConnectPtr conn)
 static int testNumOfDomains(virConnectPtr conn)
 {
     testConnPtr privconn = conn->privateData;
-    unsigned int numActive = 0, i;
+    int count;
 
     testDriverLock(privconn);
-    for (i = 0 ; i < privconn->domains.count ; i++)
-        if (virDomainIsActive(privconn->domains.objs[i]))
-            numActive++;
+    count = virDomainObjListNumOfDomains(&privconn->domains, 1);
     testDriverUnlock(privconn);
 
-    return numActive;
+    return count;
 }
 
 static virDomainPtr
@@ -1173,15 +1177,10 @@ static int testListDomains (virConnectPtr conn,
                             int maxids)
 {
     testConnPtr privconn = conn->privateData;
-    unsigned int n = 0, i;
+    int n;
 
     testDriverLock(privconn);
-    for (i = 0 ; i < privconn->domains.count && n < maxids ; i++) {
-        virDomainObjLock(privconn->domains.objs[i]);
-        if (virDomainIsActive(privconn->domains.objs[i]))
-            ids[n++] = privconn->domains.objs[i]->def->id;
-        virDomainObjUnlock(privconn->domains.objs[i]);
-    }
+    n = virDomainObjListGetActiveIDs(&privconn->domains, ids, maxids);
     testDriverUnlock(privconn);
 
     return n;
@@ -1852,47 +1851,28 @@ cleanup:
 
 static int testNumOfDefinedDomains(virConnectPtr conn) {
     testConnPtr privconn = conn->privateData;
-    unsigned int numInactive = 0, i;
+    int count;
 
     testDriverLock(privconn);
-    for (i = 0 ; i < privconn->domains.count ; i++) {
-        virDomainObjLock(privconn->domains.objs[i]);
-        if (!virDomainIsActive(privconn->domains.objs[i]))
-            numInactive++;
-        virDomainObjUnlock(privconn->domains.objs[i]);
-    }
+    count = virDomainObjListNumOfDomains(&privconn->domains, 0);
     testDriverUnlock(privconn);
 
-    return numInactive;
+    return count;
 }
 
 static int testListDefinedDomains(virConnectPtr conn,
                                   char **const names,
                                   int maxnames) {
+
     testConnPtr privconn = conn->privateData;
-    unsigned int n = 0, i;
+    int n;
 
     testDriverLock(privconn);
     memset(names, 0, sizeof(*names)*maxnames);
-    for (i = 0 ; i < privconn->domains.count && n < maxnames ; i++) {
-        virDomainObjLock(privconn->domains.objs[i]);
-        if (!virDomainIsActive(privconn->domains.objs[i]) &&
-            !(names[n++] = strdup(privconn->domains.objs[i]->def->name))) {
-            virDomainObjUnlock(privconn->domains.objs[i]);
-            goto no_memory;
-        }
-        virDomainObjUnlock(privconn->domains.objs[i]);
-    }
+    n = virDomainObjListGetInactiveNames(&privconn->domains, names, maxnames);
     testDriverUnlock(privconn);
 
     return n;
-
-no_memory:
-    virReportOOMError(conn);
-    for (n = 0 ; n < maxnames ; n++)
-        VIR_FREE(names[n]);
-    testDriverUnlock(privconn);
-    return -1;
 }
 
 static virDomainPtr testDomainDefineXML(virConnectPtr conn,
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 9a7fe42..80cf477 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -114,10 +114,32 @@ static int umlMonitorCommand (virConnectPtr conn,
 
 static struct uml_driver *uml_driver = NULL;
 
+struct umlAutostartData {
+    struct uml_driver *driver;
+    virConnectPtr conn;
+};
+
+static void
+umlAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque)
+{
+    virDomainObjPtr vm = payload;
+    const struct umlAutostartData *data = opaque;
+
+    virDomainObjLock(vm);
+    if (vm->autostart &&
+        !virDomainIsActive(vm)) {
+        virResetLastError();
+        if (umlStartVMDaemon(data->conn, data->driver, vm) < 0) {
+            virErrorPtr err = virGetLastError();
+            VIR_ERROR(_("Failed to autostart VM '%s': %s"),
+                      vm->def->name, err->message);
+        }
+    }
+    virDomainObjUnlock(vm);
+}
 
 static void
 umlAutostartConfigs(struct uml_driver *driver) {
-    unsigned int i;
     /* XXX: Figure out a better way todo this. The domain
      * startup code needs a connection handle in order
      * to lookup the bridge associated with a virtual
@@ -128,15 +150,9 @@ umlAutostartConfigs(struct uml_driver *driver) {
                                         "uml:///session");
     /* Ignoring NULL conn which is mostly harmless here */
 
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        if (driver->domains.objs[i]->autostart &&
-            !virDomainIsActive(driver->domains.objs[i]) &&
-            umlStartVMDaemon(conn, driver, driver->domains.objs[i]) < 0) {
-            virErrorPtr err = virGetLastError();
-            VIR_ERROR(_("Failed to autostart VM '%s': %s"),
-                     driver->domains.objs[i]->def->name, err->message);
-        }
-    }
+    struct umlAutostartData data = { driver, conn };
+
+    virHashForEach(driver->domains.objs, umlAutostartDomain, &data);
 
     if (conn)
         virConnectClose(conn);
@@ -320,6 +336,9 @@ umlStartup(int privileged) {
     uml_driver->nextvmid = 1;
     uml_driver->inotifyWatch = -1;
 
+    if (virDomainObjListInit(&uml_driver->domains) < 0)
+        goto error;
+
     userdir = virGetUserDirectory(NULL, uid);
     if (!userdir)
         goto error;
@@ -449,24 +468,30 @@ umlReload(void) {
  */
 static int
 umlActive(void) {
-    unsigned int i;
     int active = 0;
 
     if (!uml_driver)
         return 0;
 
     umlDriverLock(uml_driver);
-    for (i = 0 ; i < uml_driver->domains.count ; i++) {
-        virDomainObjLock(uml_driver->domains.objs[i]);
-        if (virDomainIsActive(uml_driver->domains.objs[i]))
-            active = 1;
-        virDomainObjUnlock(uml_driver->domains.objs[i]);
-    }
+    active = virDomainObjListNumOfDomains(&uml_driver->domains, 1);
     umlDriverUnlock(uml_driver);
 
     return active;
 }
 
+static void
+umlShutdownOneVM(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque)
+{
+    virDomainObjPtr dom = payload;
+    struct uml_driver *driver = opaque;
+
+    virDomainObjLock(dom);
+    if (virDomainIsActive(dom))
+        umlShutdownVMDaemon(NULL, driver, dom);
+    virDomainObjUnlock(dom);
+}
+
 /**
  * umlShutdown:
  *
@@ -474,8 +499,6 @@ umlActive(void) {
  */
 static int
 umlShutdown(void) {
-    unsigned int i;
-
     if (!uml_driver)
         return -1;
 
@@ -485,16 +508,11 @@ umlShutdown(void) {
     close(uml_driver->inotifyFD);
     virCapabilitiesFree(uml_driver->caps);
 
-    /* shutdown active VMs */
-    for (i = 0 ; i < uml_driver->domains.count ; i++) {
-        virDomainObjPtr dom = uml_driver->domains.objs[i];
-        virDomainObjLock(dom);
-        if (virDomainIsActive(dom))
-            umlShutdownVMDaemon(NULL, uml_driver, dom);
-        virDomainObjUnlock(dom);
-    }
+    /* shutdown active VMs
+     * XXX allow them to stay around & reconnect */
+    virHashForEach(uml_driver->domains.objs, umlShutdownOneVM, uml_driver);
 
-    virDomainObjListFree(&uml_driver->domains);
+    virDomainObjListDeinit(&uml_driver->domains);
 
     VIR_FREE(uml_driver->logDir);
     VIR_FREE(uml_driver->configDir);
@@ -1145,30 +1163,20 @@ umlGetHostname (virConnectPtr conn)
 
 static int umlListDomains(virConnectPtr conn, int *ids, int nids) {
     struct uml_driver *driver = conn->privateData;
-    int got = 0, i;
+    int n;
 
     umlDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count && got < nids ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (virDomainIsActive(driver->domains.objs[i]))
-            ids[got++] = driver->domains.objs[i]->def->id;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListGetActiveIDs(&driver->domains, ids, nids);
     umlDriverUnlock(driver);
 
-    return got;
+    return n;
 }
 static int umlNumDomains(virConnectPtr conn) {
     struct uml_driver *driver = conn->privateData;
-    int n = 0, i;
+    int n;
 
     umlDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (virDomainIsActive(driver->domains.objs[i]))
-            n++;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListNumOfDomains(&driver->domains, 1);
     umlDriverUnlock(driver);
 
     return n;
@@ -1481,42 +1489,21 @@ cleanup:
 static int umlListDefinedDomains(virConnectPtr conn,
                             char **const names, int nnames) {
     struct uml_driver *driver = conn->privateData;
-    int got = 0, i;
+    int n;
 
     umlDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count && got < nnames ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (!virDomainIsActive(driver->domains.objs[i])) {
-            if (!(names[got++] = strdup(driver->domains.objs[i]->def->name))) {
-                virReportOOMError(conn);
-                virDomainObjUnlock(driver->domains.objs[i]);
-                goto cleanup;
-            }
-        }
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListGetInactiveNames(&driver->domains, names, nnames);
     umlDriverUnlock(driver);
 
-    return got;
-
- cleanup:
-    for (i = 0 ; i < got ; i++)
-        VIR_FREE(names[i]);
-    umlDriverUnlock(driver);
-    return -1;
+    return n;
 }
 
 static int umlNumDefinedDomains(virConnectPtr conn) {
     struct uml_driver *driver = conn->privateData;
-    int n = 0, i;
+    int n;
 
     umlDriverLock(driver);
-    for (i = 0 ; i < driver->domains.count ; i++) {
-        virDomainObjLock(driver->domains.objs[i]);
-        if (!virDomainIsActive(driver->domains.objs[i]))
-            n++;
-        virDomainObjUnlock(driver->domains.objs[i]);
-    }
+    n = virDomainObjListNumOfDomains(&driver->domains, 0);
     umlDriverUnlock(driver);
 
     return n;
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 4f43901..4741c57 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -103,7 +103,6 @@ typedef struct {
     virMutex lock;
     int version;
 
-    virDomainObjList domains;
     virCapsPtr caps;
 
     IVirtualBox *vboxObj;
@@ -485,7 +484,6 @@ static void vboxUninitialize(vboxGlobalData *data) {
     if (data->pFuncs)
         data->pFuncs->pfnComUninitialize();
 
-    virDomainObjListFree(&data->domains);
     virCapabilitiesFree(data->caps);
 #if VBOX_API_VERSION == 2002
     /* No domainEventCallbacks in 2.2.* version */
-- 
1.6.2.5




More information about the libvir-list mailing list