[libvirt] [PATCH 0/2] qemu: Sanitize update of 'current' checkpoint/snapshot

See 1/2. Peter Krempa (2): qemu: snapshot: Don't update current snapshot until we're done qemu: checkpoint: Don't update current checkpoint until we are done src/qemu/qemu_checkpoint.c | 7 ------- src/qemu/qemu_driver.c | 19 ------------------- 2 files changed, 26 deletions(-) -- 2.21.0

Since commit f105627992e we store whether a snapshot is current globally rather than locally in the snapshot object. This means that we don't have to unset the current snapshot prior to taking/reverting the snapshot and we can do it only when everything is done tsuccessfully. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8751145ea0..95fe844c34 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16001,13 +16001,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (!redefine && VIR_STRDUP(snap->def->parent_name, current->def->name) < 0) goto endjob; - if (update_current) { - virDomainSnapshotSetCurrent(vm->snapshots, NULL); - if (qemuDomainSnapshotWriteMetadata(vm, current, - driver->caps, driver->xmlopt, - cfg->snapshotDir) < 0) - goto endjob; - } } /* actually do the snapshot */ @@ -16480,7 +16473,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainObjPtr vm = NULL; int ret = -1; virDomainMomentObjPtr snap = NULL; - virDomainMomentObjPtr current = NULL; virDomainSnapshotDefPtr snapdef; virObjectEventPtr event = NULL; virObjectEventPtr event2 = NULL; @@ -16580,17 +16572,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } } - current = virDomainSnapshotGetCurrent(vm->snapshots); - if (current) { - virDomainSnapshotSetCurrent(vm->snapshots, NULL); - if (qemuDomainSnapshotWriteMetadata(vm, current, - driver->caps, driver->xmlopt, - cfg->snapshotDir) < 0) - goto endjob; - /* XXX Should we restore the current snapshot after this point - * in the failure cases where we know there was no change? */ - } - if (snap->def->dom) { config = virDomainDefCopy(snap->def->dom, caps, driver->xmlopt, priv->qemuCaps, true); -- 2.21.0

On Mon, Sep 30, 2019 at 04:42:45PM +0200, Peter Krempa wrote:
Since commit f105627992e we store whether a snapshot is current globally rather than locally in the snapshot object.
This means that we don't have to unset the current snapshot prior to taking/reverting the snapshot and we can do it only when everything is done tsuccessfully.
*successfully
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 19 ------------------- 1 file changed, 19 deletions(-)
Jano

Similarly to the snapshot code there's no reason to modify current checkpoint until we are done creating the new one. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 0d90bbec14..8856a90ce8 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -412,13 +412,6 @@ qemuCheckpointCreateXML(virDomainPtr domain, if (!redefine && VIR_STRDUP(chk->def->parent_name, other->def->name) < 0) goto endjob; - if (update_current) { - virDomainCheckpointSetCurrent(vm->checkpoints, NULL); - if (qemuCheckpointWriteMetadata(vm, other, - driver->caps, driver->xmlopt, - cfg->checkpointDir) < 0) - goto endjob; - } } /* actually do the checkpoint */ -- 2.21.0

On 9/30/19 4:42 PM, Peter Krempa wrote:
See 1/2.
Peter Krempa (2): qemu: snapshot: Don't update current snapshot until we're done qemu: checkpoint: Don't update current checkpoint until we are done
src/qemu/qemu_checkpoint.c | 7 ------- src/qemu/qemu_driver.c | 19 ------------------- 2 files changed, 26 deletions(-)
Nice diff stat. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (3)
-
Ján Tomko
-
Michal Privoznik
-
Peter Krempa