[PATCH 0/2] qemu: Fix setting of 'current' checkpoint/snapshot
See patch 1 Peter Krempa (2): qemu: snapshot: Write metadata of previously-'current' snapshot on update qemu: checkpoint: Write metadata of previously-'current' checkpoint on update src/qemu/qemu_checkpoint.c | 35 +++++++++++++++++++++++++++++++++-- src/qemu/qemu_snapshot.c | 38 +++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 5 deletions(-) -- 2.28.0
Whether a snapshot definition is considered 'current' or active is stored in the metadata XML libvirt writes when we create metadata. This means that if we are changing the 'current' snapshot we must re-write the metadata of the previously 'current' snapshot to update the field to prevent having multiple active snapshots. Unfortunately the snapshot creation code didn't do this properly, which resulted in the following error: error : qemuDomainSnapshotLoad:430 : internal error: Too many snapshots claiming to be current for domain snapshot-test being printed if libvirtd was terminated and restarted. Introduce qemuSnapshotSetCurrent which writes out the old snapshot's metadata when updating the current snapshot. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 057b94e5b8..6ed2ce838c 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -47,6 +47,36 @@ VIR_LOG_INIT("qemu.qemu_snapshot"); +/** + * qemuSnapshotSetCurrent: Set currently active snapshot + * + * @vm: domain object + * @newcurrent: snapshot object to set as current/active + * + * Sets @newcurrent as the 'current' snapshot of @vm. This helper ensures that + * the snapshot which was 'current' previously is updated. + */ +static void +qemuSnapshotSetCurrent(virDomainObjPtr vm, + virDomainMomentObjPtr newcurrent) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + virDomainMomentObjPtr oldcurrent = virDomainSnapshotGetCurrent(vm->snapshots); + + virDomainSnapshotSetCurrent(vm->snapshots, newcurrent); + + /* we need to write out metadata for the old snapshot to update the + * 'active' property */ + if (oldcurrent && + oldcurrent != newcurrent) { + if (qemuDomainSnapshotWriteMetadata(vm, oldcurrent, driver->xmlopt, cfg->snapshotDir) < 0) + VIR_WARN("failed to update old current snapshot"); + } +} + + /* Looks up snapshot object from VM and name */ virDomainMomentObjPtr qemuSnapObjFromName(virDomainObjPtr vm, @@ -1781,7 +1811,8 @@ qemuSnapshotCreateXML(virDomainPtr domain, endjob: if (snapshot && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) { if (update_current) - virDomainSnapshotSetCurrent(vm->snapshots, snap); + qemuSnapshotSetCurrent(vm, snap); + if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->xmlopt, cfg->snapshotDir) < 0) { @@ -2233,7 +2264,7 @@ qemuSnapshotRevert(virDomainObjPtr vm, cleanup: if (ret == 0) { - virDomainSnapshotSetCurrent(vm->snapshots, snap); + qemuSnapshotSetCurrent(vm, snap); if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->xmlopt, cfg->snapshotDir) < 0) { @@ -2350,7 +2381,8 @@ qemuSnapshotDelete(virDomainObjPtr vm, if (rem.err < 0) goto endjob; if (rem.found) { - virDomainSnapshotSetCurrent(vm->snapshots, snap); + qemuSnapshotSetCurrent(vm, snap); + if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->xmlopt, -- 2.28.0
Similarly to previous commit dealing with snapshots we must rewrite the metadata of the previously-'current' checkpoint when changing which checkpoint is considered 'current'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 8dce7d8756..e8d18b2e02 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -41,6 +41,36 @@ VIR_LOG_INIT("qemu.qemu_checkpoint"); +/** + * qemuCheckpointSetCurrent: Set currently active checkpoint + * + * @vm: domain object + * @newcurrent: checkpoint object to set as current/active + * + * Sets @newcurrent as the 'current' checkpoint of @vm. This helper ensures that + * the checkpoint which was 'current' previously is updated. + */ +static void +qemuCheckpointSetCurrent(virDomainObjPtr vm, + virDomainMomentObjPtr newcurrent) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + virDomainMomentObjPtr oldcurrent = virDomainCheckpointGetCurrent(vm->checkpoints); + + virDomainCheckpointSetCurrent(vm->checkpoints, newcurrent); + + /* we need to write out metadata for the old checkpoint to update the + * 'active' property */ + if (oldcurrent && + oldcurrent != newcurrent) { + if (qemuCheckpointWriteMetadata(vm, oldcurrent, driver->xmlopt, cfg->checkpointDir) < 0) + VIR_WARN("failed to update old current checkpoint"); + } +} + + /* Looks up the domain object from checkpoint and unlocks the * driver. The returned domain object is locked and ref'd and the * caller must call virDomainObjEndAPI() on it. */ @@ -506,7 +536,7 @@ qemuCheckpointCreateFinalize(virQEMUDriverPtr driver, bool update_current) { if (update_current) - virDomainCheckpointSetCurrent(vm->checkpoints, chk); + qemuCheckpointSetCurrent(vm, chk); if (qemuCheckpointWriteMetadata(vm, chk, driver->xmlopt, @@ -848,7 +878,8 @@ qemuCheckpointDelete(virDomainObjPtr vm, if (rem.err < 0) goto endjob; if (rem.found) { - virDomainCheckpointSetCurrent(vm->checkpoints, chk); + qemuCheckpointSetCurrent(vm, chk); + if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY) { if (qemuCheckpointWriteMetadata(vm, chk, driver->xmlopt, -- 2.28.0
On a Monday in 2020, Peter Krempa wrote:
See patch 1
Peter Krempa (2): qemu: snapshot: Write metadata of previously-'current' snapshot on update qemu: checkpoint: Write metadata of previously-'current' checkpoint on update
src/qemu/qemu_checkpoint.c | 35 +++++++++++++++++++++++++++++++++-- src/qemu/qemu_snapshot.c | 38 +++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa