On Tue, Jun 18, 2019 at 22:47:53 -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.
At this point I'm lacking the interlocking with snapshots. It may be
posted as a separate patch, but none of this code should go in unless
that one is ACK'd.
Signed-off-by: Eric Blake <eblake(a)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 e128dc169b..1364227e42 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8809,45 +8809,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;
I was complaining about this in the previous version.
+ 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) {
We definitely need interlocking as this will not work across the backing
chain as it seems to reference the 'node'
+ 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);
}
}
}
[...]
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5fa319b656..0cdb2dd1e2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
[...]
@@ -17016,6 +17017,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;
optimization:
break;
+ }
+ }
+ }
+ ret = 0;
+
+ cleanup:
pointless label
+ return ret;
+}
static virDomainCheckpointPtr
qemuDomainCheckpointCreateXML(virDomainPtr domain,
[...]
@@ -17094,10 +17152,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)
Looks like it's refactoring code from previous patch.
goto endjob;
if (update_current) {
virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
[...]