snapshot: Access snapshot def directly when needed

An upcoming patch will rework virDomainSnapshotObjList to be generic
for both snapshots and checkpoints; reduce the churn by adding a new
accessor virDomainSnapshotObjGetDef() which returns the
snapshot-specific definition even when the list is rewritten to
operate only on a base class, then using it at sites that that are
specific to snapshots.  Use VIR_STEAL_PTR when appropriate in the
affected lines.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index c300451e70a8733afdb22463f925faec57cf6e9c..cd5d02575714e5b4c16e218933e775558497dea0 100644 (file)
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -461,8 +461,10 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def,
     }
 
     if (other) {
-        if ((other->def->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
-             other->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED) !=
+        virDomainSnapshotDefPtr otherdef = virDomainSnapshotObjGetDef(other);
+
+        if ((otherdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
+             otherdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED) !=
             (def->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
              def->state == VIR_DOMAIN_SNAPSHOT_PAUSED)) {
             virReportError(VIR_ERR_INVALID_ARG,
@@ -472,7 +474,7 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def,
             return -1;
         }
 
-        if ((other->def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT) !=
+        if ((otherdef->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT) !=
             (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)) {
             virReportError(VIR_ERR_INVALID_ARG,
                            _("cannot change between disk only and "
@@ -481,15 +483,14 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def,
             return -1;
         }
 
-        if (other->def->dom) {
+        if (otherdef->dom) {
             if (def->dom) {
-                if (!virDomainDefCheckABIStability(other->def->dom,
+                if (!virDomainDefCheckABIStability(otherdef->dom,
                                                    def->dom, xmlopt))
                     return -1;
             } else {
                 /* Transfer the domain def */
-                def->dom = other->def->dom;
-                other->def->dom = NULL;
+                VIR_STEAL_PTR(def->dom, otherdef->dom);
             }
         }
     }
@@ -914,7 +915,9 @@ virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def)
 bool
 virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap)
 {
-    return virDomainSnapshotDefIsExternal(snap->def);
+    virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(snap);
+
+    return virDomainSnapshotDefIsExternal(def);
 }
 
 int
@@ -928,6 +931,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
 {
     virDomainSnapshotDefPtr def = *defptr;
     virDomainSnapshotObjPtr other;
+    virDomainSnapshotDefPtr otherdef;
     bool check_if_stolen;
 
     /* Prevent circular chains */
@@ -945,15 +949,16 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
                            def->parent, def->name);
             return -1;
         }
-        while (other->def->parent) {
-            if (STREQ(other->def->parent, def->name)) {
+        otherdef = virDomainSnapshotObjGetDef(other);
+        while (otherdef->parent) {
+            if (STREQ(otherdef->parent, def->name)) {
                 virReportError(VIR_ERR_INVALID_ARG,
                                _("parent %s would create cycle to %s"),
-                               other->def->name, def->name);
+                               otherdef->name, def->name);
                 return -1;
             }
             other = virDomainSnapshotFindByName(vm->snapshots,
-                                                other->def->parent);
+                                                otherdef->parent);
             if (!other) {
                 VIR_WARN("snapshots are inconsistent for %s",
                          vm->def->name);
@@ -963,14 +968,13 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
     }
 
     other = virDomainSnapshotFindByName(vm->snapshots, def->name);
-    check_if_stolen = other && other->def->dom;
+    otherdef = other ? virDomainSnapshotObjGetDef(other) : NULL;
+    check_if_stolen = other && otherdef->dom;
     if (virDomainSnapshotRedefineValidate(def, domain->uuid, other, xmlopt,
                                           flags) < 0) {
         /* revert any stealing of the snapshot domain definition */
-        if (check_if_stolen && def->dom && !other->def->dom) {
-            other->def->dom = def->dom;
-            def->dom = NULL;
-        }
+        if (check_if_stolen && def->dom && !otherdef->dom)
+            VIR_STEAL_PTR(otherdef->dom, def->dom);
         return -1;
     }
     if (other) {
@@ -982,9 +986,8 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
         /* Drop and rebuild the parent relationship, but keep all
          * child relations by reusing snap. */
         virDomainSnapshotDropParent(other);
-        virDomainSnapshotDefFree(other->def);
-        other->def = def;
-        *defptr = NULL;
+        virDomainSnapshotDefFree(otherdef);
+        VIR_STEAL_PTR(other->def, *defptr);
         *snap = other;
     }
 

diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c
index 545acd48bc620f7b6b4bf8efceee36b92e5ec270..d9d77e292ef00cbb8b353f4d62339db0ae457e42 100644 (file)
--- a/src/conf/virdomainsnapshotobjlist.c
+++ b/src/conf/virdomainsnapshotobjlist.c
@@ -168,8 +168,9 @@ virDomainSnapshotFormatOne(void *payload,
     virDomainSnapshotObjPtr snap = payload;
     struct virDomainSnapshotFormatData *data = opaque;
     return virDomainSnapshotDefFormatInternal(data->buf, data->uuidstr,
-                                              snap->def, data->caps,
-                                              data->xmlopt, data->flags);
+                                              virDomainSnapshotObjGetDef(snap),
+                                              data->caps, data->xmlopt,
+                                              data->flags);
 }
 
 
@@ -229,7 +230,7 @@ static void virDomainSnapshotObjFree(virDomainSnapshotObjPtr snapshot)
 
     VIR_DEBUG("obj=%p", snapshot);
 
-    virDomainSnapshotDefFree(snapshot->def);
+    virDomainSnapshotDefFree(virDomainSnapshotObjGetDef(snapshot));
     VIR_FREE(snapshot);
 }
 
@@ -315,15 +316,17 @@ static int virDomainSnapshotObjListCopyNames(void *payload,
         return 0;
 
     if (data->flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) {
+        virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(obj);
+
         if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE) &&
-            obj->def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF)
+            def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF)
             return 0;
         if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) &&
-            obj->def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)
+            def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)
             return 0;
         if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE) &&
-            obj->def->state != VIR_DOMAIN_SNAPSHOT_SHUTOFF &&
-            obj->def->state != VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)
+            def->state != VIR_DOMAIN_SNAPSHOT_SHUTOFF &&
+            def->state != VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)
             return 0;
     }
 

diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapshotobjlist.h
index c13a0b402691d2963c7be236601e398b16808ef1..59d76caafa55a6808b7d47c3768878e64edb9704 100644 (file)
--- a/src/conf/virdomainsnapshotobjlist.h
+++ b/src/conf/virdomainsnapshotobjlist.h
@@ -76,4 +76,11 @@ int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots,
                            virDomainSnapshotPtr **snaps,
                            unsigned int flags);
 
+/* Access the snapshot-specific definition from a given list member. */
+static inline virDomainSnapshotDefPtr
+virDomainSnapshotObjGetDef(virDomainSnapshotObjPtr obj)
+{
+    return obj->def;
+}
+
 #endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJLIST_H */

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b4f21a85c4e2257b6c01ce2272a79c57096b1787..fe2681f9536077f22656bc04bd1b06ff13750e66 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8460,12 +8460,12 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     unsigned int flags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE |
         VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL;
+    virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(snapshot);
 
     if (virDomainSnapshotGetCurrent(vm->snapshots) == snapshot)
         flags |= VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT;
     virUUIDFormat(vm->def->uuid, uuidstr);
-    newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, caps, xmlopt,
-                                        flags);
+    newxml = virDomainSnapshotDefFormat(uuidstr, def, caps, xmlopt, flags);
     if (newxml == NULL)
         return -1;
 
@@ -8477,7 +8477,7 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
         goto cleanup;
     }
 
-    if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, snapshot->def->name) < 0)
+    if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, def->name) < 0)
         goto cleanup;
 
     ret = virXMLSaveFile(snapFile, NULL, "snapshot-edit", newxml);