[libvirt] [PATCH 08/16] snapshot: Access snapshot def directly when needed

Eric Blake eblake at redhat.com
Wed Mar 20 05:40:57 UTC 2019


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.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 src/conf/virdomainsnapshotobj.h     |  6 +++++
 src/conf/snapshot_conf.c            | 41 +++++++++++++++++------------
 src/conf/virdomainsnapshotobjlist.c | 17 +++++++-----
 src/qemu/qemu_domain.c              |  6 ++---
 4 files changed, 43 insertions(+), 27 deletions(-)

diff --git a/src/conf/virdomainsnapshotobj.h b/src/conf/virdomainsnapshotobj.h
index 0981ea4c25..8f96bfded9 100644
--- a/src/conf/virdomainsnapshotobj.h
+++ b/src/conf/virdomainsnapshotobj.h
@@ -52,4 +52,10 @@ void virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from,
 void virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot,
                                 virDomainSnapshotObjPtr parent);

+static inline virDomainSnapshotDefPtr
+virDomainSnapshotObjGetDef(virDomainSnapshotObjPtr obj)
+{
+    return obj->def;
+}
+
 #endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H */
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index c692d36bd1..aec23f111c 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -456,8 +456,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,
@@ -467,7 +469,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 "
@@ -476,15 +478,15 @@ 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;
+                def->dom = otherdef->dom;
+                otherdef->dom = NULL;
             }
         }
     }
@@ -909,7 +911,9 @@ virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def)
 bool
 virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap)
 {
-    return virDomainSnapshotDefIsExternal(snap->def);
+    virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(snap);
+
+    return virDomainSnapshotDefIsExternal(def);
 }

 int
@@ -923,6 +927,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
 {
     virDomainSnapshotDefPtr def = *defptr;
     virDomainSnapshotObjPtr other;
+    virDomainSnapshotDefPtr otherdef;
     bool check_if_stolen;

     /* Prevent circular chains */
@@ -940,15 +945,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);
@@ -958,12 +964,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;
+        if (check_if_stolen && def->dom && !otherdef->dom) {
+            otherdef->dom = def->dom;
             def->dom = NULL;
         }
         return -1;
@@ -977,8 +984,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;
+        virDomainSnapshotDefFree(otherdef);
+        otherdef = def;
         *defptr = NULL;
         *snap = other;
     }
diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c
index 9538521ab3..ec670ff5c2 100644
--- a/src/conf/virdomainsnapshotobjlist.c
+++ b/src/conf/virdomainsnapshotobjlist.c
@@ -171,8 +171,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);
 }


@@ -230,7 +231,7 @@ static void virDomainSnapshotObjFree(virDomainSnapshotObjPtr snapshot)

     VIR_DEBUG("obj=%p", snapshot);

-    virDomainSnapshotDefFree(snapshot->def);
+    virDomainSnapshotDefFree(virDomainSnapshotObjGetDef(snapshot));
     VIR_FREE(snapshot);
 }

@@ -316,15 +317,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/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b1a84d3914..e693a3b122 100644
--- 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);
-- 
2.20.1




More information about the libvir-list mailing list