If we are certain that the checkpoint creation failed we remove the metadata from the list. To allow reusing this in the backup code add a new helper and export it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 22 ++++++++++++++++++++-- src/qemu/qemu_checkpoint.h | 4 ++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 97bc97bb8e..2fa5c1ae00 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -400,6 +400,24 @@ qemuCheckpointCreateCommon(virQEMUDriverPtr driver, } +/** + * qemuCheckpointRollbackMetadata: + * @vm: domain object + * @chk: checkpoint object + * + * If @chk is not null remove the @chk object from the list of checkpoints of @vm. + */ +void +qemuCheckpointRollbackMetadata(virDomainObjPtr vm, + virDomainMomentObjPtr chk) +{ + if (!chk) + return; + + virDomainCheckpointObjListRemove(vm->checkpoints, chk); +} + + static virDomainMomentObjPtr qemuCheckpointCreate(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -415,7 +433,7 @@ qemuCheckpointCreate(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); rc = qemuMonitorTransaction(qemuDomainGetMonitor(vm), &actions); if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) { - virDomainCheckpointObjListRemove(vm->checkpoints, chk); + qemuCheckpointRollbackMetadata(vm, chk); return NULL; } @@ -441,7 +459,7 @@ qemuCheckpointCreateFinalize(virQEMUDriverPtr driver, virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to save metadata for checkpoint %s"), chk->def->name); - virDomainCheckpointObjListRemove(vm->checkpoints, chk); + qemuCheckpointRollbackMetadata(vm, chk); return -1; } diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h index 00548beec9..eb85611ea6 100644 --- a/src/qemu/qemu_checkpoint.h +++ b/src/qemu/qemu_checkpoint.h @@ -67,3 +67,7 @@ qemuCheckpointCreateFinalize(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg, virDomainMomentObjPtr chk, bool update_current); + +void +qemuCheckpointRollbackMetadata(virDomainObjPtr vm, + virDomainMomentObjPtr chk); -- 2.24.1

We insert the checkpoint metadata into the list of checkpoints prior to actually creating the on-disk bits. If the 'transaction' or any other steps done between inserting the checkpoint and creating the on-disk data fail we'd end up with an unusable checkpoint that would vanish after libvirtd restart. Prevent this by rolling back the metadata if we didn't actually take and record the checkpoint. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index c0445e0869..e7358082d5 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -812,6 +812,8 @@ qemuBackupBegin(virDomainObjPtr vm, if (!(actions = virJSONValueNewArray())) goto endjob; + /* The 'chk' checkpoint must be rolled back if the transaction command + * which creates it on disk is not executed or fails */ if (chkdef) { if (qemuCheckpointCreateCommon(priv->driver, vm, &chkdef, &actions, &chk) < 0) @@ -857,9 +859,11 @@ qemuBackupBegin(virDomainObjPtr vm, job_started = true; qemuBackupDiskStarted(vm, dd, ndd); - if (chk && - qemuCheckpointCreateFinalize(priv->driver, vm, cfg, chk, true) < 0) - goto endjob; + if (chk) { + virDomainMomentObjPtr tmpchk = g_steal_pointer(&chk); + if (qemuCheckpointCreateFinalize(priv->driver, vm, cfg, tmpchk, true) < 0) + goto endjob; + } if (pull) { if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, QEMU_ASYNC_JOB_BACKUP) < 0) @@ -880,6 +884,10 @@ qemuBackupBegin(virDomainObjPtr vm, endjob: qemuBackupDiskDataCleanup(vm, dd, ndd); + + /* if 'chk' is non-NULL here it's a failure and it must be rolled back */ + qemuCheckpointRollbackMetadata(vm, chk); + if (!job_started && nbd_running && qemuDomainObjEnterMonitorAsync(priv->driver, vm, QEMU_ASYNC_JOB_BACKUP) < 0) { ignore_value(qemuMonitorNBDServerStop(priv->mon)); -- 2.24.1