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 first
ancestor checkpoint (if any) that also had a bitmap. For deletion, we
need multiple calls: for each disk, if there is an ancestor checkpoint
with a bitmap, then the bitmap must be merged (including activating
the ancestor bitmap), all before deleting the bitmap from the
checkpoint being removed.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
src/qemu/qemu_domain.c | 122 ++++++++++++++++++++++++++++-------------
src/qemu/qemu_driver.c | 98 ++++++++++++++++++++++++++++++++-
2 files changed, 180 insertions(+), 40 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d71b7d1984..062a08ed97 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9255,48 +9255,97 @@ qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
bool update_parent,
bool metadata_only)
{
- char *chkFile = NULL;
- int ret = -1;
- virDomainMomentObjPtr parentchk = NULL;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virDomainMomentObjPtr parent = NULL;
+ virDomainMomentObjPtr moment;
+ virDomainCheckpointDefPtr parentdef = NULL;
+ size_t i, j;
+ VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+ VIR_AUTOFREE(char *) chkFile = 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"));
+ return -1;
}
if (virAsprintf(&chkFile, "%s/%s/%s.xml", cfg->checkpointDir,
vm->def->name, chk->def->name) < 0)
- goto cleanup;
+ return -1;
+
+ if (!metadata_only) {
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ bool success = true;
+ bool search_parents;
+ virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk);
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ parent = virDomainCheckpointFindByName(vm->checkpoints,
+ chk->def->parent_name);
+ 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 = qemuDomainDiskNodeFormatLookup(vm, disk->name);
+ /* If any ancestor checkpoint has a bitmap for the same
+ * disk, then this bitmap must be merged to the
+ * ancestor. */
+ search_parents = true;
+ for (moment = parent;
+ search_parents && moment;
+ moment = virDomainCheckpointFindByName(vm->checkpoints,
+ parentdef->parent.parent_name)) {
+ parentdef = virDomainCheckpointObjGetDef(moment);
+ for (j = 0; j < parentdef->ndisks; j++) {
+ virDomainCheckpointDiskDef *disk2;
+ VIR_AUTOPTR(virJSONValue) arr = NULL;
+
+ disk2 = &parentdef->disks[j];
+ if (STRNEQ(disk->name, disk2->name) ||
+ disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+ continue;
+ search_parents = false;
+
+ arr = virJSONValueNewArray();
+ if (!arr ||
+ virJSONValueArrayAppendString(arr, disk->bitmap) < 0) {
+ success = false;
+ break;
+ }
+ 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)
+ return -1;
+ }
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 && parent) {
+ virDomainCheckpointSetCurrent(vm->checkpoints, parent);
+ if (qemuDomainCheckpointWriteMetadata(vm, parent, 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);
}
}
}
@@ -9307,12 +9356,7 @@ qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
virDomainMomentDropParent(chk);
virDomainCheckpointObjListRemove(vm->checkpoints, chk);
- ret = 0;
-
- cleanup:
- VIR_FREE(chkFile);
- virObjectUnref(cfg);
- return ret;
+ return 0;
}
int
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a75a16492b..d0377d78a9 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"
@@ -16995,6 +16996,65 @@ qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps,
return ret;
}
+static int
+qemuDomainCheckpointAddActions(virDomainObjPtr vm,
+ virJSONValuePtr actions,
+ virDomainMomentObjPtr old_current,
+ virDomainCheckpointDefPtr def)
+{
+ size_t i, j;
+ virDomainCheckpointDefPtr olddef;
+ virDomainMomentObjPtr parent;
+ bool search_parents;
+
+ 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 = qemuDomainDiskNodeFormatLookup(vm, disk->name);
+ if (qemuMonitorJSONTransactionAdd(actions,
+ "block-dirty-bitmap-add",
+ "s:node", node,
+ "s:name", disk->bitmap,
+ "b:persistent", true,
+ NULL) < 0)
+ return -1;
+
+ /* We only want one active bitmap for a disk along the
+ * checkpoint chain, then later differential backups will
+ * merge the bitmaps (only one active) between the bounding
+ * checkpoint and the leaf checkpoint. If the same disks are
+ * involved in each checkpoint, this search terminates in one
+ * iteration; but it is also possible to have to search
+ * further than the immediate parent to find another
+ * checkpoint with a bitmap on the same disk. */
+ search_parents = true;
+ for (parent = old_current; search_parents && parent;
+ parent = virDomainCheckpointFindByName(vm->checkpoints,
+ olddef->parent.parent_name)) {
+ olddef = virDomainCheckpointObjGetDef(parent);
+ for (j = 0; j < olddef->ndisks; j++) {
+ virDomainCheckpointDiskDef *disk2;
+
+ disk2 = &olddef->disks[j];
+ if (STRNEQ(disk->name, disk2->name) ||
+ disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+ continue;
+ if (qemuMonitorJSONTransactionAdd(actions,
+ "block-dirty-bitmap-disable",
+ "s:node", node,
+ "s:name", disk2->bitmap,
+ NULL) < 0)
+ return -1;
+ search_parents = false;
+ break;
+ }
+ }
+ }
+ return 0;
+}
static virDomainCheckpointPtr
qemuDomainCheckpointCreateXML(virDomainPtr domain,
@@ -17012,6 +17072,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, NULL);
@@ -17031,11 +17094,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;
@@ -17096,7 +17166,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;
@@ -17130,6 +17208,7 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain,
qemuDomainObjEndJob(driver, vm);
cleanup:
+ virJSONValueFree(actions);
virDomainObjEndAPI(&vm);
VIR_FREE(xml);
virObjectUnref(caps);
@@ -17301,6 +17380,7 @@ qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
{
virQEMUDriverPtr driver = checkpoint->domain->conn->privateData;
virDomainObjPtr vm = NULL;
+ qemuDomainObjPrivatePtr priv;
int ret = -1;
virDomainMomentObjPtr chk = NULL;
virQEMUMomentRemove rem;
@@ -17323,6 +17403,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 = qemuCheckpointObjFromCheckpoint(vm, checkpoint)))
goto endjob;
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jul 24, 2019 at 12:56:08AM -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 first
> ancestor checkpoint (if any) that also had a bitmap. For deletion, we
> need multiple calls: for each disk, if there is an ancestor checkpoint
> with a bitmap, then the bitmap must be merged (including activating
> the ancestor bitmap), all before deleting the bitmap from the
> checkpoint being removed.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> src/qemu/qemu_domain.c | 122 ++++++++++++++++++++++++++++-------------
> src/qemu/qemu_driver.c | 98 ++++++++++++++++++++++++++++++++-
> 2 files changed, 180 insertions(+), 40 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index d71b7d1984..062a08ed97 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -9255,48 +9255,97 @@ qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
> bool update_parent,
> bool metadata_only)
> {
> - char *chkFile = NULL;
> - int ret = -1;
> - virDomainMomentObjPtr parentchk = NULL;
> - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> + virDomainMomentObjPtr parent = NULL;
> + virDomainMomentObjPtr moment;
> + virDomainCheckpointDefPtr parentdef = NULL;
> + size_t i, j;
> + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
> + VIR_AUTOFREE(char *) chkFile = 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"));
> + return -1;
> }
This semi-answerss my previous question on earlier patch. I guess we
just shouldn't add the commented out lines at all in teh first place,
given you are deleting them entirely here.
> @@ -17096,7 +17166,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;
> }
Ah, and this now answers my other question.
Makes me wonder if there's really a benefit to splitting the code across
two patches, as opposed to just one. As long as it git bisect's with a
clean built though its not a show stopper.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.