[PATCH 7/9] conf: checkpoint: Split virDomainCheckpointRedefinePrep into two functions

[Peter Krempa]

First one prepares and validates the definition, the second one actually
either updates an existing checkpoint or assigns definition for the new
one.

This will allow driver code to add extra validation between those
steps.

Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 src/conf/checkpoint_conf.c | 30 +++++++++++++++++++++---------
 src/conf/checkpoint_conf.h | 14 +++++++++-----
 src/libvirt_private.syms   |  1 +
 src/qemu/qemu_checkpoint.c | 12 ++----------
 src/test/test_driver.c     |  9 ++++-----
 5 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
index e175917eae..a8d18928de 100644
--- a/src/conf/checkpoint_conf.c
+++ b/src/conf/checkpoint_conf.c
@@ -513,16 +513,11 @@ virDomainCheckpointDefFormat(virDomainCheckpointDefPtr def,

 int
 virDomainCheckpointRedefinePrep(virDomainObjPtr vm,
-                                virDomainCheckpointDefPtr *defptr,
-                                virDomainMomentObjPtr *chk,
-                                virDomainXMLOptionPtr xmlopt,
+                                virDomainCheckpointDefPtr def,
                                 bool *update_current)
 {
-    virDomainCheckpointDefPtr def = *defptr;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     virDomainMomentObjPtr parent = NULL;
-    virDomainMomentObjPtr other = NULL;
-    virDomainCheckpointDefPtr otherdef = NULL;

     virUUIDFormat(vm->def->uuid, uuidstr);

@@ -550,12 +545,26 @@ virDomainCheckpointRedefinePrep(virDomainObjPtr vm,
     if (virDomainCheckpointGetCurrent(vm->checkpoints) == NULL)
         *update_current = true;

+    return 0;
+}
+
+
+virDomainMomentObjPtr
+virDomainCheckpointRedefineCommit(virDomainObjPtr vm,
+                                  virDomainCheckpointDefPtr *defptr,
+                                  virDomainXMLOptionPtr xmlopt)
+{
+    virDomainCheckpointDefPtr def = *defptr;
+    virDomainMomentObjPtr other = NULL;
+    virDomainCheckpointDefPtr otherdef = NULL;
+    virDomainMomentObjPtr chk = NULL;
+
     other = virDomainCheckpointFindByName(vm->checkpoints, def->parent.name);
     if (other) {
         otherdef = virDomainCheckpointObjGetDef(other);
         if (!virDomainDefCheckABIStability(otherdef->parent.dom,
                                            def->parent.dom, xmlopt))
-            return -1;
+            return NULL;

         /* Drop and rebuild the parent relationship, but keep all
          * child relations by reusing chk.  */
@@ -563,8 +572,11 @@ virDomainCheckpointRedefinePrep(virDomainObjPtr vm,
         virObjectUnref(otherdef);
         other->def = &(*defptr)->parent;
         *defptr = NULL;
-        *chk = other;
+        chk = other;
+    } else {
+        chk = virDomainCheckpointAssignDef(vm->checkpoints, def);
+        *defptr = NULL;
     }

-    return 0;
+    return chk;
 }
diff --git a/src/conf/checkpoint_conf.h b/src/conf/checkpoint_conf.h
index f115b98c2b..631f863151 100644
--- a/src/conf/checkpoint_conf.h
+++ b/src/conf/checkpoint_conf.h
@@ -90,10 +90,14 @@ virDomainCheckpointDefFormat(virDomainCheckpointDefPtr def,
 int
 virDomainCheckpointAlignDisks(virDomainCheckpointDefPtr checkpoint);

-int virDomainCheckpointRedefinePrep(virDomainObjPtr vm,
-                                    virDomainCheckpointDefPtr *def,
-                                    virDomainMomentObjPtr *checkpoint,
-                                    virDomainXMLOptionPtr xmlopt,
-                                    bool *update_current);
+int
+virDomainCheckpointRedefinePrep(virDomainObjPtr vm,
+                                virDomainCheckpointDefPtr def,
+                                bool *update_current);
+
+virDomainMomentObjPtr
+virDomainCheckpointRedefineCommit(virDomainObjPtr vm,
+                                  virDomainCheckpointDefPtr *defptr,
+                                  virDomainXMLOptionPtr xmlopt);

 VIR_ENUM_DECL(virDomainCheckpoint);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 95e50835ad..fb6fbd1f9f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -86,6 +86,7 @@ virDomainCheckpointDefFormat;
 virDomainCheckpointDefNew;
 virDomainCheckpointDefParseString;
 virDomainCheckpointFormatConvertXMLFlags;
+virDomainCheckpointRedefineCommit;
 virDomainCheckpointRedefinePrep;
 virDomainCheckpointTypeFromString;
 virDomainCheckpointTypeToString;
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index fb76c211f8..caedfe1c55 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -377,22 +377,14 @@ qemuCheckpointRedefine(virQEMUDriverPtr driver,
                        virDomainCheckpointDefPtr *def,
                        bool *update_current)
 {
-    virDomainMomentObjPtr chk = NULL;
-
-    if (virDomainCheckpointRedefinePrep(vm, def, &chk, driver->xmlopt,
-                                        update_current) < 0)
+    if (virDomainCheckpointRedefinePrep(vm, *def, update_current) < 0)
         return NULL;

     /* XXX Should we validate that the redefined checkpoint even
      * makes sense, such as checking that qemu-img recognizes the
      * checkpoint bitmap name in at least one of the domain's disks?  */

-    if (chk)
-        return chk;
-
-    chk = virDomainCheckpointAssignDef(vm->checkpoints, *def);
-    *def = NULL;
-    return chk;
+    return virDomainCheckpointRedefineCommit(vm, def, driver->xmlopt);
 }


diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 5c02a8ebb0..cd502781e1 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -8989,9 +8989,10 @@ testDomainCheckpointCreateXML(virDomainPtr domain,
         goto cleanup;

     if (redefine) {
-        if (virDomainCheckpointRedefinePrep(vm, &def, &chk,
-                                            privconn->xmlopt,
-                                            &update_current) < 0)
+        if (virDomainCheckpointRedefinePrep(vm, def, &update_current) < 0)
+            goto cleanup;
+
+        if (!(chk = virDomainCheckpointRedefineCommit(vm, &def, privconn->xmlopt)))
             goto cleanup;
     } else {
         if (!(def->parent.dom = virDomainDefCopy(vm->def,
@@ -9002,9 +9003,7 @@ testDomainCheckpointCreateXML(virDomainPtr domain,

         if (virDomainCheckpointAlignDisks(def) < 0)
             goto cleanup;
-    }

-    if (!chk) {
         if (!(chk = virDomainCheckpointAssignDef(vm->checkpoints, def)))
             goto cleanup;

-- 
2.26.2