snapshot: make virDomainSnapshotObjList opaque

We were failing to react to allocation failure when initializing
a snapshot object list.  Changing things to store a pointer
instead of a complete object adds one more possible point of
allocation failure, but at the same time, will make it easier to
react to failure now, as well as making it easier for a future
patch to split all virDomainSnapshotPtr handling into a separate
file, as I continue to add even more snapshot code.

Luckily, there was only one client outside of domain_conf.c that
was actually peeking inside the object, and a new wrapper function
was easy.

* src/conf/domain_conf.h (_virDomainObj): Use a pointer.
(virDomainSnapshotObjListInit): Rename.
(virDomainSnapshotObjListFree, virDomainSnapshotForEach): New
declarations.
(_virDomainSnapshotObjList): Move definitions...
* src/conf/domain_conf.c: ...here.
(virDomainSnapshotObjListInit, virDomainSnapshotObjListDeinit):
Rename...
(virDomainSnapshotObjListNew, virDomainSnapshotObjListFree): ...to
these.
(virDomainSnapshotForEach): New function.
(virDomainObjDispose, virDomainListPopulate): Adjust callers.
* src/qemu/qemu_domain.c (qemuDomainSnapshotDiscard)
(qemuDomainSnapshotDiscardAllMetadata): Likewise.
* src/qemu/qemu_migration.c (qemuMigrationIsAllowed): Likewise.
* src/qemu/qemu_driver.c (qemuDomainSnapshotLoad)
(qemuDomainUndefineFlags, qemuDomainSnapshotCreateXML)
(qemuDomainSnapshotListNames, qemuDomainSnapshotNum)
(qemuDomainListAllSnapshots)
(qemuDomainSnapshotListChildrenNames)
(qemuDomainSnapshotNumChildren)
(qemuDomainSnapshotListAllChildren)
(qemuDomainSnapshotLookupByName, qemuDomainSnapshotGetParent)
(qemuDomainSnapshotGetXMLDesc, qemuDomainSnapshotIsCurrent)
(qemuDomainSnapshotHasMetadata, qemuDomainRevertToSnapshot)
(qemuDomainSnapshotDelete): Likewise.
* src/libvirt_private.syms (domain_conf.h): Export new function.

2 files changed, 56 insertions, 30 deletions

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 816a9c773..822d309e0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -661,6 +661,15 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode,
 #define VIR_DOMAIN_XML_WRITE_FLAGS  VIR_DOMAIN_XML_SECURE
 #define VIR_DOMAIN_XML_READ_FLAGS   VIR_DOMAIN_XML_INACTIVE
 
+struct _virDomainSnapshotObjList {
+    /* name string -> virDomainSnapshotObj  mapping
+     * for O(1), lockless lookup-by-name */
+    virHashTable *objs;
+
+    virDomainSnapshotObj metaroot; /* Special parent of all root snapshots */
+};
+
+
 static virClassPtr virDomainObjClass;
 static void virDomainObjDispose(void *obj);
 
@@ -1692,8 +1701,6 @@ void virDomainDefFree(virDomainDefPtr def)
     VIR_FREE(def);
 }
 
-static void virDomainSnapshotObjListDeinit(virDomainSnapshotObjListPtr snapshots);
-
 static void virDomainObjDispose(void *obj)
 {
     virDomainObjPtr dom = obj;
@@ -1707,7 +1714,7 @@ static void virDomainObjDispose(void *obj)
 
     virMutexDestroy(&dom->lock);
 
-    virDomainSnapshotObjListDeinit(&dom->snapshots);
+    virDomainSnapshotObjListFree(dom->snapshots);
 }
 
 
@@ -1721,31 +1728,33 @@ virDomainObjPtr virDomainObjNew(virCapsPtr caps)
     if (!(domain = virObjectNew(virDomainObjClass)))
         return NULL;
 
-    if (caps->privateDataAllocFunc &&
-        !(domain->privateData = (caps->privateDataAllocFunc)())) {
-        virReportOOMError();
-        VIR_FREE(domain);
-        return NULL;
-    }
-    domain->privateDataFreeFunc = caps->privateDataFreeFunc;
-
     if (virMutexInit(&domain->lock) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("cannot initialize mutex"));
-        if (domain->privateDataFreeFunc)
-            (domain->privateDataFreeFunc)(domain->privateData);
         VIR_FREE(domain);
         return NULL;
     }
 
+    if (caps->privateDataAllocFunc &&
+        !(domain->privateData = (caps->privateDataAllocFunc)())) {
+        virReportOOMError();
+        goto error;
+    }
+    domain->privateDataFreeFunc = caps->privateDataFreeFunc;
+
+    if (!(domain->snapshots = virDomainSnapshotObjListNew()))
+        goto error;
+
     virDomainObjLock(domain);
     virDomainObjSetState(domain, VIR_DOMAIN_SHUTOFF,
                                  VIR_DOMAIN_SHUTOFF_UNKNOWN);
 
-    virDomainSnapshotObjListInit(&domain->snapshots);
-
     VIR_DEBUG("obj=%p", domain);
     return domain;
+
+error:
+    virObjectUnref(domain);
+    return NULL;
 }
 
 void virDomainObjAssignDef(virDomainObjPtr domain,
@@ -14694,18 +14703,29 @@ virDomainSnapshotObjListDataFree(void *payload,
     virDomainSnapshotObjFree(obj);
 }
 
-int virDomainSnapshotObjListInit(virDomainSnapshotObjListPtr snapshots)
+virDomainSnapshotObjListPtr
+virDomainSnapshotObjListNew(void)
 {
+    virDomainSnapshotObjListPtr snapshots;
+    if (VIR_ALLOC(snapshots) < 0) {
+        virReportOOMError();
+        return NULL;
+    }
     snapshots->objs = virHashCreate(50, virDomainSnapshotObjListDataFree);
-    if (!snapshots->objs)
-        return -1;
-    return 0;
+    if (!snapshots->objs) {
+        VIR_FREE(snapshots);
+        return NULL;
+    }
+    return snapshots;
 }
 
-static void
-virDomainSnapshotObjListDeinit(virDomainSnapshotObjListPtr snapshots)
+void
+virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots)
 {
+    if (!snapshots)
+        return;
     virHashFree(snapshots->objs);
+    VIR_FREE(snapshots);
 }
 
 struct virDomainSnapshotNameData {
@@ -14826,6 +14846,14 @@ void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
     virHashRemoveEntry(snapshots->objs, snapshot->def->name);
 }
 
+int
+virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
+                         virHashIterator iter,
+                         void *data)
+{
+    return virHashForEach(snapshots->objs, iter, data);
+}
+
 /* Run iter(data) on all direct children of snapshot, while ignoring all
  * other entries in snapshots.  Return the number of children
  * visited.  No particular ordering is guaranteed.  */
@@ -15747,7 +15775,7 @@ virDomainListPopulate(void *payload,
 
     /* filter by snapshot existence */
     if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_SNAPSHOT)) {
-        int nsnap = virDomainSnapshotObjListNum(&vm->snapshots, NULL, 0);
+        int nsnap = virDomainSnapshotObjListNum(vm->snapshots, NULL, 0);
         if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT) && nsnap > 0) ||
               (MATCH(VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT) && nsnap <= 0)))
             goto cleanup;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0c3824ec7..603364129 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1761,13 +1761,9 @@ struct _virDomainSnapshotObj {
 
 typedef struct _virDomainSnapshotObjList virDomainSnapshotObjList;
 typedef virDomainSnapshotObjList *virDomainSnapshotObjListPtr;
-struct _virDomainSnapshotObjList {
-    /* name string -> virDomainSnapshotObj  mapping
-     * for O(1), lockless lookup-by-name */
-    virHashTable *objs;
 
-    virDomainSnapshotObj metaroot; /* Special parent of all root snapshots */
-};
+virDomainSnapshotObjListPtr virDomainSnapshotObjListNew(void);
+void virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots);
 
 typedef enum {
     VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE = 1 << 0,
@@ -1790,7 +1786,6 @@ int virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapshot,
 virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr snapshots,
                                                    const virDomainSnapshotDefPtr def);
 
-int virDomainSnapshotObjListInit(virDomainSnapshotObjListPtr objs);
 int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
                                      virDomainSnapshotObjPtr from,
                                      char **const names, int maxnames,
@@ -1802,6 +1797,9 @@ virDomainSnapshotObjPtr virDomainSnapshotFindByName(const virDomainSnapshotObjLi
                                                     const char *name);
 void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
                                     virDomainSnapshotObjPtr snapshot);
+int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
+                             virHashIterator iter,
+                             void *data);
 int virDomainSnapshotForEachChild(virDomainSnapshotObjPtr snapshot,
                                   virHashIterator iter,
                                   void *data);
@@ -1835,7 +1833,7 @@ struct _virDomainObj {
     virDomainDefPtr def; /* The current definition */
     virDomainDefPtr newDef; /* New definition to activate at shutdown */
 
-    virDomainSnapshotObjList snapshots;
+    virDomainSnapshotObjListPtr snapshots;
     virDomainSnapshotObjPtr current_snapshot;
 
     bool hasManagedSave;