[PATCH 0/2] qemu: Fix two checkpoint redefine issues
Peter Krempa (2): qemu: checkpoint: Allow checkpoint redefine for offline VMs virDomainCheckpointRedefinePrep: Set 'current' checkpoint if there isn't any src/conf/checkpoint_conf.c | 4 ++++ src/qemu/qemu_checkpoint.c | 20 +++++++++++--------- 2 files changed, 15 insertions(+), 9 deletions(-) -- 2.25.1
Skip the liveness and capability checks when redefining checkpoints as we don't need qemu interactions to update the metadata. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 3a510c9780..8340addf81 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -647,16 +647,18 @@ qemuCheckpointCreateXML(virDomainPtr domain, update_current = false; } - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot create checkpoint for inactive domain")); - return NULL; - } + if (!redefine) { + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cannot create checkpoint for inactive domain")); + return NULL; + } - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("incremental backup is not supported yet")); - return NULL; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("incremental backup is not supported yet")); + return NULL; + } } if (!(def = virDomainCheckpointDefParseString(xmlDesc, driver->xmlopt, -- 2.25.1
On 4/1/20 8:10 AM, Peter Krempa wrote:
Skip the liveness and capability checks when redefining checkpoints as we don't need qemu interactions to update the metadata.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
When redefining checkpoints from scratch we'd not set the 'current' checkpoint if there wasn't any. This meant that the code wasn't ever able to set a 'current' checkpoint as any other one looks up if the parent of the redefined checkpoint is current. Since the backup code then requires the current checkpoint to start the lookup we'd not be able to perform a backup after restoring the checkpoint hierarchy. Reported-by: Eyal Shenitzky <eshenitz@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index 26bcfc16b7..d557fada49 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -550,6 +550,10 @@ virDomainCheckpointRedefinePrep(virDomainObjPtr vm, *update_current = true; } + /* set the first redefined checkpoint as current */ + if (virDomainCheckpointGetCurrent(vm->checkpoints) == NULL) + *update_current = true; + other = virDomainCheckpointFindByName(vm->checkpoints, def->parent.name); if (other) { otherdef = virDomainCheckpointObjGetDef(other); -- 2.25.1
On 4/1/20 8:10 AM, Peter Krempa wrote:
When redefining checkpoints from scratch we'd not set the 'current' checkpoint if there wasn't any. This meant that the code wasn't ever able to set a 'current' checkpoint as any other one looks up if the parent of the redefined checkpoint is current.
Since the backup code then requires the current checkpoint to start the lookup we'd not be able to perform a backup after restoring the checkpoint hierarchy.
Reported-by: Eyal Shenitzky <eshenitz@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index 26bcfc16b7..d557fada49 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -550,6 +550,10 @@ virDomainCheckpointRedefinePrep(virDomainObjPtr vm, *update_current = true; }
+ /* set the first redefined checkpoint as current */ + if (virDomainCheckpointGetCurrent(vm->checkpoints) == NULL) + *update_current = true; + other = virDomainCheckpointFindByName(vm->checkpoints, def->parent.name); if (other) { otherdef = virDomainCheckpointObjGetDef(other);
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (2)
-
Eric Blake -
Peter Krempa