[libvirt] [PATCH v9 12/13] backup: Wire up qemu checkpoint commands over QMP

Eric Blake posted 13 patches 6 years, 7 months ago
There is a newer version of this series
[libvirt] [PATCH v9 12/13] backup: Wire up qemu checkpoint commands over QMP
Posted by Eric Blake 6 years, 7 months ago
Time to actually issue the QMP transactions that create and
delete persistent checkpoints.  For create, we only need one
transaction: inside, we visit all disks affected by the
checkpoint, and create a new enabled bitmap, as well as
disabling the bitmap of the parent checkpoint (if any).  For
deletion, we need multiple calls: if the checkpoint to be
deleted is active, we must enable the parent; then we must
merge the existing checkpoint into the parent, and finally
we can delete the checkpoint.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 src/qemu/qemu_domain.c | 105 ++++++++++++++++++++++++++++++-----------
 src/qemu/qemu_driver.c |  92 ++++++++++++++++++++++++++++++++++--
 2 files changed, 165 insertions(+), 32 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 03a143a228..2fca7bd0b8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8960,45 +8960,93 @@ qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
     char *chkFile = NULL;
     int ret = -1;
     virDomainMomentObjPtr parentchk = NULL;
+    virDomainCheckpointDefPtr parentdef = NULL;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    size_t i, j;
+    virJSONValuePtr arr = NULL;

-    if (!metadata_only) {
-        if (!virDomainObjIsActive(vm)) {
-            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-                           _("cannot remove checkpoint from inactive domain"));
-            goto cleanup;
-        } else {
-            /* TODO: Implement QMP sequence to merge bitmaps */
-            // qemuDomainObjPrivatePtr priv;
-            // priv = vm->privateData;
-            // qemuDomainObjEnterMonitor(driver, vm);
-            // /* we continue on even in the face of error */
-            // qemuMonitorDeleteCheckpoint(priv->mon, chk->def->name);
-            // ignore_value(qemuDomainObjExitMonitor(driver, vm));
-        }
+    if (!metadata_only && !virDomainObjIsActive(vm)) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot remove checkpoint from inactive domain"));
+        goto cleanup;
     }

     if (virAsprintf(&chkFile, "%s/%s/%s.xml", cfg->checkpointDir,
                     vm->def->name, chk->def->name) < 0)
         goto cleanup;

+    if (chk->def->parent_name) {
+        parentchk = virDomainCheckpointFindByName(vm->checkpoints,
+                                                  chk->def->parent_name);
+        if (!parentchk) {
+            VIR_WARN("missing parent checkpoint matching name '%s'",
+                     chk->def->parent_name);
+        }
+        parentdef = virDomainCheckpointObjGetDef(parentchk);
+    }
+
+    if (!metadata_only) {
+        qemuDomainObjPrivatePtr priv = vm->privateData;
+        bool success = true;
+        virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk);
+
+        if (qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
+            goto cleanup;
+        qemuDomainObjEnterMonitor(driver, vm);
+        for (i = 0; i < chkdef->ndisks; i++) {
+            virDomainCheckpointDiskDef *disk = &chkdef->disks[i];
+            const char *node;
+
+            if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+                continue;
+
+            node = qemuBlockNodeLookup(vm, disk->name);
+            if (parentchk) {
+                arr = virJSONValueNewArray();
+                if (!arr ||
+                    virJSONValueArrayAppendString(arr, disk->bitmap) < 0) {
+                    success = false;
+                    break;
+                }
+
+                for (j = 0; j < parentdef->ndisks; j++) {
+                    virDomainCheckpointDiskDef *disk2;
+
+                    disk2 = &parentdef->disks[j];
+                    if (STRNEQ(disk->name, disk2->name))
+                        continue;
+                    if (chk == virDomainCheckpointGetCurrent(vm->checkpoints) &&
+                        qemuMonitorEnableBitmap(priv->mon, node,
+                                                disk2->bitmap) < 0) {
+                        success = false;
+                        break;
+                    }
+                    if (qemuMonitorMergeBitmaps(priv->mon, node,
+                                                disk2->bitmap, &arr) < 0) {
+                        success = false;
+                        break;
+                    }
+                }
+            }
+            if (qemuMonitorDeleteBitmap(priv->mon, node, disk->bitmap) < 0) {
+                success = false;
+                break;
+            }
+        }
+        if (qemuDomainObjExitMonitor(driver, vm) < 0 || !success)
+            goto cleanup;
+    }
+
     if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) {
         virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
-        if (update_parent && chk->def->parent_name) {
-            parentchk = virDomainCheckpointFindByName(vm->checkpoints,
-                                                      chk->def->parent_name);
-            if (!parentchk) {
-                VIR_WARN("missing parent checkpoint matching name '%s'",
+        if (update_parent && parentchk) {
+            virDomainCheckpointSetCurrent(vm->checkpoints, parentchk);
+            if (qemuDomainCheckpointWriteMetadata(vm, parentchk, driver->caps,
+                                                  driver->xmlopt,
+                                                  cfg->checkpointDir) < 0) {
+                VIR_WARN("failed to set parent checkpoint '%s' as current",
                          chk->def->parent_name);
-            } else {
-                virDomainCheckpointSetCurrent(vm->checkpoints, parentchk);
-                if (qemuDomainCheckpointWriteMetadata(vm, parentchk, driver->caps,
-                                                      driver->xmlopt,
-                                                      cfg->checkpointDir) < 0) {
-                    VIR_WARN("failed to set parent checkpoint '%s' as current",
-                             chk->def->parent_name);
-                    virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
-                }
+                virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
             }
         }
     }
@@ -9014,6 +9062,7 @@ qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
  cleanup:
     VIR_FREE(chkFile);
     virObjectUnref(cfg);
+    virJSONValueFree(arr);
     return ret;
 }

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 702a715902..b37307abc7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -47,6 +47,7 @@
 #include "qemu_hostdev.h"
 #include "qemu_hotplug.h"
 #include "qemu_monitor.h"
+#include "qemu_monitor_json.h"
 #include "qemu_process.h"
 #include "qemu_migration.h"
 #include "qemu_migration_params.h"
@@ -16994,6 +16995,53 @@ qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps,
     return ret;
 }

+static int
+qemuDomainCheckpointAddActions(virDomainObjPtr vm,
+                               virJSONValuePtr actions,
+                               virDomainMomentObjPtr old_current,
+                               virDomainCheckpointDefPtr def)
+{
+    size_t i, j;
+    int ret = -1;
+    virDomainCheckpointDefPtr olddef;
+
+    olddef = virDomainCheckpointObjGetDef(old_current);
+    for (i = 0; i < def->ndisks; i++) {
+        virDomainCheckpointDiskDef *disk = &def->disks[i];
+        const char *node;
+
+        if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+            continue;
+        node = qemuBlockNodeLookup(vm, disk->name);
+        if (qemuMonitorJSONTransactionAdd(actions,
+                                          "block-dirty-bitmap-add",
+                                          "s:node", node,
+                                          "s:name", disk->bitmap,
+                                          "b:persistent", true,
+                                          NULL) < 0)
+            goto cleanup;
+        if (old_current) {
+            for (j = 0; j < olddef->ndisks; j++) {
+                virDomainCheckpointDiskDef *disk2;
+
+                disk2 = &olddef->disks[j];
+                if (STRNEQ(disk->name, disk2->name))
+                    continue;
+                if (disk2->type == VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP &&
+                    qemuMonitorJSONTransactionAdd(actions,
+                                                  "block-dirty-bitmap-disable",
+                                                  "s:node", node,
+                                                  "s:name", disk2->bitmap,
+                                                  NULL) < 0)
+                    goto cleanup;
+            }
+        }
+    }
+    ret = 0;
+
+ cleanup:
+    return ret;
+}

 static virDomainCheckpointPtr
 qemuDomainCheckpointCreateXML(virDomainPtr domain,
@@ -17012,6 +17060,9 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain,
     virDomainMomentObjPtr other = NULL;
     virQEMUDriverConfigPtr cfg = NULL;
     virCapsPtr caps = NULL;
+    qemuDomainObjPrivatePtr priv;
+    virJSONValuePtr actions = NULL;
+    int ret;
     VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL;

     virCheckFlags(VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE |
@@ -17033,11 +17084,18 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain,
         goto cleanup;
     }

+    priv = vm->privateData;
     cfg = virQEMUDriverGetConfig(driver);

     if (virDomainCheckpointCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0)
         goto cleanup;

+    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("qemu binary lacks persistent bitmaps support"));
+        goto cleanup;
+    }
+
     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
         goto cleanup;

@@ -17077,10 +17135,10 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain,
         def = NULL;
     }

-    current = virDomainCheckpointGetCurrent(vm->checkpoints);
-    if (current) {
+    other = virDomainCheckpointGetCurrent(vm->checkpoints);
+    if (other) {
         if (!redefine &&
-            VIR_STRDUP(chk->def->parent_name, current->def->name) < 0)
+            VIR_STRDUP(chk->def->parent_name, other->def->name) < 0)
             goto endjob;
         if (update_current) {
             virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
@@ -17097,7 +17155,15 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain,
          * makes sense, such as checking that qemu-img recognizes the
          * checkpoint bitmap name in at least one of the domain's disks?  */
     } else {
-        /* TODO: issue QMP transaction command */
+        if (!(actions = virJSONValueNewArray()))
+            goto endjob;
+        if (qemuDomainCheckpointAddActions(vm, actions, other,
+                                           virDomainCheckpointObjGetDef(chk)) < 0)
+            goto endjob;
+        qemuDomainObjEnterMonitor(driver, vm);
+        ret = qemuMonitorTransaction(priv->mon, &actions);
+        if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0)
+            goto endjob;
     }

     /* If we fail after this point, there's not a whole lot we can do;
@@ -17133,6 +17199,7 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain,
     qemuDomainObjEndJob(driver, vm);

  cleanup:
+    virJSONValueFree(actions);
     virDomainObjEndAPI(&vm);
     VIR_FREE(xml);
     virObjectUnref(caps);
@@ -17306,6 +17373,7 @@ qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
 {
     virQEMUDriverPtr driver = checkpoint->domain->conn->privateData;
     virDomainObjPtr vm = NULL;
+    qemuDomainObjPrivatePtr priv;
     int ret = -1;
     virDomainMomentObjPtr chk = NULL;
     virQEMUMomentRemove rem;
@@ -17328,6 +17396,22 @@ qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
     if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
         goto cleanup;

+    priv = vm->privateData;
+    if (!metadata_only) {
+        /* Until qemu-img supports offline bitmap deletion, we are stuck
+         * with requiring a running guest */
+        if (!virDomainObjIsActive(vm)) {
+            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                           _("cannot delete checkpoint for inactive domain"));
+            goto endjob;
+        }
+        if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("qemu binary lacks persistent bitmaps support"));
+            goto endjob;
+        }
+    }
+
     if (!(chk = qemuCheckObjFromCheckpoint(vm, checkpoint)))
         goto endjob;

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v9 12/13] backup: Wire up qemu checkpoint commands over QMP
Posted by Peter Krempa 6 years, 7 months ago
On Sat, Jul 06, 2019 at 22:56:12 -0500, Eric Blake wrote:
> Time to actually issue the QMP transactions that create and
> delete persistent checkpoints.  For create, we only need one
> transaction: inside, we visit all disks affected by the
> checkpoint, and create a new enabled bitmap, as well as
> disabling the bitmap of the parent checkpoint (if any).  For
> deletion, we need multiple calls: if the checkpoint to be
> deleted is active, we must enable the parent; then we must
> merge the existing checkpoint into the parent, and finally
> we can delete the checkpoint.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 105 ++++++++++++++++++++++++++++++-----------
>  src/qemu/qemu_driver.c |  92 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 165 insertions(+), 32 deletions(-)

[...]

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 03a143a228..2fca7bd0b8 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -8960,45 +8960,93 @@ qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
>      char *chkFile = NULL;
>      int ret = -1;
>      virDomainMomentObjPtr parentchk = NULL;
> +    virDomainCheckpointDefPtr parentdef = NULL;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    size_t i, j;
> +    virJSONValuePtr arr = NULL;
> 
> -    if (!metadata_only) {
> -        if (!virDomainObjIsActive(vm)) {
> -            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> -                           _("cannot remove checkpoint from inactive domain"));
> -            goto cleanup;
> -        } else {
> -            /* TODO: Implement QMP sequence to merge bitmaps */
> -            // qemuDomainObjPrivatePtr priv;
> -            // priv = vm->privateData;
> -            // qemuDomainObjEnterMonitor(driver, vm);
> -            // /* we continue on even in the face of error */
> -            // qemuMonitorDeleteCheckpoint(priv->mon, chk->def->name);
> -            // ignore_value(qemuDomainObjExitMonitor(driver, vm));
> -        }
> +    if (!metadata_only && !virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("cannot remove checkpoint from inactive domain"));
> +        goto cleanup;
>      }
> 
>      if (virAsprintf(&chkFile, "%s/%s/%s.xml", cfg->checkpointDir,
>                      vm->def->name, chk->def->name) < 0)
>          goto cleanup;
> 
> +    if (chk->def->parent_name) {
> +        parentchk = virDomainCheckpointFindByName(vm->checkpoints,
> +                                                  chk->def->parent_name);
> +        if (!parentchk) {
> +            VIR_WARN("missing parent checkpoint matching name '%s'",
> +                     chk->def->parent_name);
> +        }
> +        parentdef = virDomainCheckpointObjGetDef(parentchk);
> +    }
> +
> +    if (!metadata_only) {
> +        qemuDomainObjPrivatePtr priv = vm->privateData;
> +        bool success = true;
> +        virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk);
> +
> +        if (qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> +            goto cleanup;

As described in previous patch, this is undesired.

> +        qemuDomainObjEnterMonitor(driver, vm);
> +        for (i = 0; i < chkdef->ndisks; i++) {
> +            virDomainCheckpointDiskDef *disk = &chkdef->disks[i];
> +            const char *node;
> +
> +            if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
> +                continue;
> +
> +            node = qemuBlockNodeLookup(vm, disk->name);
> +            if (parentchk) {
> +                arr = virJSONValueNewArray();
> +                if (!arr ||
> +                    virJSONValueArrayAppendString(arr, disk->bitmap) < 0) {
> +                    success = false;
> +                    break;
> +                }
> +
> +                for (j = 0; j < parentdef->ndisks; j++) {
> +                    virDomainCheckpointDiskDef *disk2;
> +
> +                    disk2 = &parentdef->disks[j];
> +                    if (STRNEQ(disk->name, disk2->name))
> +                        continue;
> +                    if (chk == virDomainCheckpointGetCurrent(vm->checkpoints) &&

As outlined before in reviews to previous patches and also discussed on
IRC, the "current" checkpoint may not be the only one with active
bitmaps, so this might not work as expected.

> +                        qemuMonitorEnableBitmap(priv->mon, node,
> +                                                disk2->bitmap) < 0) {
> +                        success = false;
> +                        break;
> +                    }
> +                    if (qemuMonitorMergeBitmaps(priv->mon, node,
> +                                                disk2->bitmap, &arr) < 0) {
> +                        success = false;
> +                        break;
> +                    }
> +                }
> +            }
> +            if (qemuMonitorDeleteBitmap(priv->mon, node, disk->bitmap) < 0) {
> +                success = false;
> +                break;
> +            }
> +        }
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0 || !success)
> +            goto cleanup;
> +    }
> +
>      if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) {
>          virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
> -        if (update_parent && chk->def->parent_name) {
> -            parentchk = virDomainCheckpointFindByName(vm->checkpoints,
> -                                                      chk->def->parent_name);
> -            if (!parentchk) {
> -                VIR_WARN("missing parent checkpoint matching name '%s'",
> +        if (update_parent && parentchk) {
> +            virDomainCheckpointSetCurrent(vm->checkpoints, parentchk);
> +            if (qemuDomainCheckpointWriteMetadata(vm, parentchk, driver->caps,
> +                                                  driver->xmlopt,
> +                                                  cfg->checkpointDir) < 0) {
> +                VIR_WARN("failed to set parent checkpoint '%s' as current",
>                           chk->def->parent_name);
> -            } else {
> -                virDomainCheckpointSetCurrent(vm->checkpoints, parentchk);
> -                if (qemuDomainCheckpointWriteMetadata(vm, parentchk, driver->caps,
> -                                                      driver->xmlopt,
> -                                                      cfg->checkpointDir) < 0) {
> -                    VIR_WARN("failed to set parent checkpoint '%s' as current",
> -                             chk->def->parent_name);
> -                    virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
> -                }
> +                virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
>              }
>          }
>      }
> @@ -9014,6 +9062,7 @@ qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
>   cleanup:
>      VIR_FREE(chkFile);
>      virObjectUnref(cfg);
> +    virJSONValueFree(arr);
>      return ret;
>  }
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 702a715902..b37307abc7 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -47,6 +47,7 @@
>  #include "qemu_hostdev.h"
>  #include "qemu_hotplug.h"
>  #include "qemu_monitor.h"
> +#include "qemu_monitor_json.h"

I'd prefer if you not include this header directly here but I'm ashamed
of myself for including it in qemu_block.c

>  #include "qemu_process.h"
>  #include "qemu_migration.h"
>  #include "qemu_migration_params.h"
> @@ -16994,6 +16995,53 @@ qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps,
>      return ret;
>  }
> 
> +static int
> +qemuDomainCheckpointAddActions(virDomainObjPtr vm,
> +                               virJSONValuePtr actions,
> +                               virDomainMomentObjPtr old_current,
> +                               virDomainCheckpointDefPtr def)
> +{
> +    size_t i, j;
> +    int ret = -1;
> +    virDomainCheckpointDefPtr olddef;
> +
> +    olddef = virDomainCheckpointObjGetDef(old_current);
> +    for (i = 0; i < def->ndisks; i++) {
> +        virDomainCheckpointDiskDef *disk = &def->disks[i];
> +        const char *node;
> +
> +        if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
> +            continue;
> +        node = qemuBlockNodeLookup(vm, disk->name);
> +        if (qemuMonitorJSONTransactionAdd(actions,
> +                                          "block-dirty-bitmap-add",
> +                                          "s:node", node,
> +                                          "s:name", disk->bitmap,
> +                                          "b:persistent", true,
> +                                          NULL) < 0)
> +            goto cleanup;
> +        if (old_current) {
> +            for (j = 0; j < olddef->ndisks; j++) {
> +                virDomainCheckpointDiskDef *disk2;
> +
> +                disk2 = &olddef->disks[j];
> +                if (STRNEQ(disk->name, disk2->name))
> +                    continue;
> +                if (disk2->type == VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP &&
> +                    qemuMonitorJSONTransactionAdd(actions,
> +                                                  "block-dirty-bitmap-disable",

As we've discussed on IRC I think that this is an unnecessary
complication of things while we don't really know what the implications
of handling bitmaps accross multiple backing files are.

As long as you are okay to work around the implications and complex
lookup of the correct bitmap I don't mind adding it this way though.


> +                                                  "s:node", node,
> +                                                  "s:name", disk2->bitmap,
> +                                                  NULL) < 0)
> +                    goto cleanup;
> +            }
> +        }
> +    }
> +    ret = 0;
> +
> + cleanup:
> +    return ret;
> +}
> 
>  static virDomainCheckpointPtr
>  qemuDomainCheckpointCreateXML(virDomainPtr domain,

[...]

> @@ -17077,10 +17135,10 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain,
>          def = NULL;
>      }
> 
> -    current = virDomainCheckpointGetCurrent(vm->checkpoints);
> -    if (current) {
> +    other = virDomainCheckpointGetCurrent(vm->checkpoints);
> +    if (other) {
>          if (!redefine &&
> -            VIR_STRDUP(chk->def->parent_name, current->def->name) < 0)
> +            VIR_STRDUP(chk->def->parent_name, other->def->name) < 0)
>              goto endjob;
>          if (update_current) {
>              virDomainCheckpointSetCurrent(vm->checkpoints, NULL);

Does this hunk belong to the previous patch?

[...]

> @@ -17328,6 +17396,22 @@ qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
>      if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>          goto cleanup;
> 
> +    priv = vm->privateData;
> +    if (!metadata_only) {
> +        /* Until qemu-img supports offline bitmap deletion, we are stuck
> +         * with requiring a running guest */
> +        if (!virDomainObjIsActive(vm)) {
> +            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                           _("cannot delete checkpoint for inactive domain"));
> +            goto endjob;
> +        }
> +        if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("qemu binary lacks persistent bitmaps support"));
> +            goto endjob;
> +        }
> +    }
> +
>      if (!(chk = qemuCheckObjFromCheckpoint(vm, checkpoint)))
>          goto endjob;

The rest looks good, but here this does too much with the "current"
snapshot which in my opinion will not work properly, thus I'd like to
see a fixed version of this.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list