]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
snapshot: make virDomainSnapshotObjList opaque
authorEric Blake <eblake@redhat.com>
Tue, 14 Aug 2012 06:22:39 +0000 (00:22 -0600)
committerEric Blake <eblake@redhat.com>
Fri, 24 Aug 2012 15:51:08 +0000 (09:51 -0600)
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.

src/conf/domain_conf.c
src/conf/domain_conf.h
src/libvirt_private.syms
src/qemu/qemu_domain.c
src/qemu/qemu_driver.c
src/qemu/qemu_migration.c

index 816a9c773f59a40bdf86ce1c1428d61ce8ea7518..822d309e07617c5d54cdb8e4b6118d9982361cd3 100644 (file)
@@ -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;
index 0c3824ec7f15d3f4568f82f0db6a1744bf52e2b5..6033641294a794078332a3bf1b3d02bd4c34b9c5 100644 (file)
@@ -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;
index d91f492cc376a02dc50e1c676f5cde66b71b4824..c3396642627d2b934ac31df1c2dc8ac40d3564df 100644 (file)
@@ -482,6 +482,7 @@ virDomainSnapshotDefFree;
 virDomainSnapshotDefParseString;
 virDomainSnapshotDropParent;
 virDomainSnapshotFindByName;
+virDomainSnapshotForEach;
 virDomainSnapshotForEachChild;
 virDomainSnapshotForEachDescendant;
 virDomainSnapshotObjListGetNames;
index c47890b359631c84191077ee9d632316c2785637..0ae30b72903694841d066b1a3baefdcc02bd989b 100644 (file)
@@ -1750,7 +1750,7 @@ qemuDomainSnapshotDiscard(struct qemud_driver *driver,
 
     if (snap == vm->current_snapshot) {
         if (update_current && snap->def->parent) {
-            parentsnap = virDomainSnapshotFindByName(&vm->snapshots,
+            parentsnap = virDomainSnapshotFindByName(vm->snapshots,
                                                      snap->def->parent);
             if (!parentsnap) {
                 VIR_WARN("missing parent snapshot matching name '%s'",
@@ -1771,7 +1771,7 @@ qemuDomainSnapshotDiscard(struct qemud_driver *driver,
 
     if (unlink(snapFile) < 0)
         VIR_WARN("Failed to unlink %s", snapFile);
-    virDomainSnapshotObjListRemove(&vm->snapshots, snap);
+    virDomainSnapshotObjListRemove(vm->snapshots, snap);
 
     ret = 0;
 
@@ -1808,7 +1808,8 @@ qemuDomainSnapshotDiscardAllMetadata(struct qemud_driver *driver,
     rem.vm = vm;
     rem.metadata_only = true;
     rem.err = 0;
-    virHashForEach(vm->snapshots.objs, qemuDomainSnapshotDiscardAll, &rem);
+    virDomainSnapshotForEach(vm->snapshots, qemuDomainSnapshotDiscardAll,
+                             &rem);
 
     return rem.err;
 }
index 773a98901ebb1489ba1c231ab40538c24958a430..c1cfa5a6e3fddfc9af1a15fa129ad461430068ec 100644 (file)
@@ -483,7 +483,7 @@ static void qemuDomainSnapshotLoad(void *payload,
             continue;
         }
 
-        snap = virDomainSnapshotAssignDef(&vm->snapshots, def);
+        snap = virDomainSnapshotAssignDef(vm->snapshots, def);
         if (snap == NULL) {
             virDomainSnapshotDefFree(def);
         } else if (snap->def->current) {
@@ -502,7 +502,7 @@ static void qemuDomainSnapshotLoad(void *payload,
         vm->current_snapshot = NULL;
     }
 
-    if (virDomainSnapshotUpdateRelations(&vm->snapshots) < 0)
+    if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0)
         VIR_ERROR(_("Snapshots have inconsistent relations for domain %s"),
                   vm->def->name);
 
@@ -5629,7 +5629,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
     }
 
     if (!virDomainObjIsActive(vm) &&
-        (nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots, NULL, 0))) {
+        (nsnapshots = virDomainSnapshotObjListNum(vm->snapshots, NULL, 0))) {
         if (!(flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA)) {
             virReportError(VIR_ERR_OPERATION_INVALID,
                            _("cannot delete inactive domain with %d "
@@ -11102,7 +11102,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
                                def->name);
                 goto cleanup;
             }
-            other = virDomainSnapshotFindByName(&vm->snapshots, def->parent);
+            other = virDomainSnapshotFindByName(vm->snapshots, def->parent);
             if (!other) {
                 virReportError(VIR_ERR_INVALID_ARG,
                                _("parent %s for snapshot %s not found"),
@@ -11116,7 +11116,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
                                    other->def->name, def->name);
                     goto cleanup;
                 }
-                other = virDomainSnapshotFindByName(&vm->snapshots,
+                other = virDomainSnapshotFindByName(vm->snapshots,
                                                     other->def->parent);
                 if (!other) {
                     VIR_WARN("snapshots are inconsistent for %s",
@@ -11134,7 +11134,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
                            def->name, uuidstr);
             goto cleanup;
         }
-        other = virDomainSnapshotFindByName(&vm->snapshots, def->name);
+        other = virDomainSnapshotFindByName(vm->snapshots, def->name);
         if (other) {
             if ((other->def->state == VIR_DOMAIN_RUNNING ||
                  other->def->state == VIR_DOMAIN_PAUSED) !=
@@ -11223,7 +11223,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
 
     if (snap)
         snap->def = def;
-    else if (!(snap = virDomainSnapshotAssignDef(&vm->snapshots, def)))
+    else if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def)))
         goto cleanup;
     def = NULL;
 
@@ -11280,7 +11280,7 @@ cleanup:
             } else {
                 if (update_current)
                     vm->current_snapshot = snap;
-                other = virDomainSnapshotFindByName(&vm->snapshots,
+                other = virDomainSnapshotFindByName(vm->snapshots,
                                                     snap->def->parent);
                 snap->parent = other;
                 other->nchildren++;
@@ -11288,7 +11288,7 @@ cleanup:
                 other->first_child = snap;
             }
         } else if (snap) {
-            virDomainSnapshotObjListRemove(&vm->snapshots, snap);
+            virDomainSnapshotObjListRemove(vm->snapshots, snap);
         }
         virDomainObjUnlock(vm);
     }
@@ -11319,7 +11319,7 @@ static int qemuDomainSnapshotListNames(virDomainPtr domain, char **names,
         goto cleanup;
     }
 
-    n = virDomainSnapshotObjListGetNames(&vm->snapshots, NULL, names, nameslen,
+    n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen,
                                          flags);
 
 cleanup:
@@ -11349,7 +11349,7 @@ static int qemuDomainSnapshotNum(virDomainPtr domain,
         goto cleanup;
     }
 
-    n = virDomainSnapshotObjListNum(&vm->snapshots, NULL, flags);
+    n = virDomainSnapshotObjListNum(vm->snapshots, NULL, flags);
 
 cleanup:
     if (vm)
@@ -11379,7 +11379,7 @@ qemuDomainListAllSnapshots(virDomainPtr domain, virDomainSnapshotPtr **snaps,
         goto cleanup;
     }
 
-    n = virDomainListSnapshots(&vm->snapshots, NULL, domain, snaps, flags);
+    n = virDomainListSnapshots(vm->snapshots, NULL, domain, snaps, flags);
 
 cleanup:
     if (vm)
@@ -11412,7 +11412,7 @@ qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
+    snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name);
     if (!snap) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                        _("no domain snapshot with matching name '%s'"),
@@ -11420,7 +11420,7 @@ qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    n = virDomainSnapshotObjListGetNames(&vm->snapshots, snap, names, nameslen,
+    n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen,
                                          flags);
 
 cleanup:
@@ -11452,7 +11452,7 @@ qemuDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
+    snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name);
     if (!snap) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                        _("no domain snapshot with matching name '%s'"),
@@ -11460,7 +11460,7 @@ qemuDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    n = virDomainSnapshotObjListNum(&vm->snapshots, snap, flags);
+    n = virDomainSnapshotObjListNum(vm->snapshots, snap, flags);
 
 cleanup:
     if (vm)
@@ -11492,7 +11492,7 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
+    snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name);
     if (!snap) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                        _("no domain snapshot with matching name '%s'"),
@@ -11500,7 +11500,7 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    n = virDomainListSnapshots(&vm->snapshots, snap, snapshot->domain, snaps,
+    n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps,
                                flags);
 
 cleanup:
@@ -11531,7 +11531,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotLookupByName(virDomainPtr domain,
         goto cleanup;
     }
 
-    snap = virDomainSnapshotFindByName(&vm->snapshots, name);
+    snap = virDomainSnapshotFindByName(vm->snapshots, name);
     if (!snap) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                        _("no snapshot with matching name '%s'"), name);
@@ -11596,7 +11596,7 @@ qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
+    snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name);
     if (!snap) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                        _("no domain snapshot with matching name '%s'"),
@@ -11674,7 +11674,7 @@ static char *qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
+    snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name);
     if (!snap) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                        _("no domain snapshot with matching name '%s'"),
@@ -11712,7 +11712,7 @@ qemuDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
+    snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name);
     if (!snap) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                        _("no domain snapshot with matching name '%s'"),
@@ -11752,7 +11752,7 @@ qemuDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
+    snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name);
     if (!snap) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                        _("no domain snapshot with matching name '%s'"),
@@ -11825,7 +11825,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
+    snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name);
     if (!snap) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                        _("no domain snapshot with matching name '%s'"),
@@ -12193,7 +12193,7 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
-    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
+    snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name);
     if (!snap) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                        _("no domain snapshot with matching name '%s'"),
index f65c81a4d8008714614b17eb60f9663d63b6404d..1b21ef6ab9673958037903a171b2db4cf24513e6 100644 (file)
@@ -807,7 +807,7 @@ qemuMigrationIsAllowed(struct qemud_driver *driver, virDomainObjPtr vm,
                            "%s", _("domain is marked for auto destroy"));
             return false;
         }
-        if ((nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots, NULL,
+        if ((nsnapshots = virDomainSnapshotObjListNum(vm->snapshots, NULL,
                                                       0))) {
             virReportError(VIR_ERR_OPERATION_INVALID,
                            _("cannot migrate domain with %d snapshots"),