On 12/19/19 4:52 PM, Michael Weiser wrote:
> Hello Cole,
>
> I've done a bit of work on the issue of saved memory state not becoming
> part of snapshots:
>
> On 10/25/19 2:28 PM, Michael Weiser wrote:
>
>> * Run/Revert of a snapshot should be rejected if the VM is in the
>> 'Saved' state, aka has been 'virsh managedsave'. This should be done at
>> the libvirt level for completeness IMO. Maybe the API needs a flag to
>> override this behavior if users know what they are doing
>
> VIR_ERR_SNAPSHOT_REVERT_RISKY seems to be meant for exactly this kind of
> thing and can be overridden using VIR_DOMAIN_SNAPSHOT_REVERT_FORCE. A
> number of cases are already handled that way in
> src/qemu/qemu_driver.c:qemuDomainRevertToSnapshot().
>
> I've added this small patch to my local install of libvirt:
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index bad2fb52f3..560e35beba 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -16578,6 +16578,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> _("must respawn qemu to start inactive snapshot"));
> goto endjob;
> }
> + if (vm->hasManagedSave &&
> + !(snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
> + snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED)) {
> + virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
> + _("revert to snapshot while there is a managed "
> + "saved state will cause corruption when run, "
> + "remove saved state first"));
> + goto endjob;
> + }
> }
>
> if (snap->def->dom) {
>
> Without additional changes it produces the following error message
> dialog from virt-manager when trying to restore a non-running snapshot
> while there's saved state:
>
> Error running snapshot 'snapshot1': revert requires force: revert to snapshot while there is a managed saved state will cause corruption when run, remove saved state first
>
> Traceback (most recent call last):
> File "/usr/share/virt-manager/virtManager/asyncjob.py", line 75, in cb_wrapper
> callback(asyncjob, *args, **kwargs)
> File "/usr/share/virt-manager/virtManager/asyncjob.py", line 111, in tmpcb
> callback(*args, **kwargs)
> File "/usr/share/virt-manager/virtManager/object/libvirtobject.py", line 66, in newfn
> ret = fn(self, *args, **kwargs)
> File "/usr/share/virt-manager/virtManager/object/domain.py", line 1055, in revert_to_snapshot
> self._backend.revertToSnapshot(snap.get_backend())
> File "/usr/lib64/python3.6/site-packages/libvirt.py", line 2088, in revertToSnapshot
> if ret == -1: raise libvirtError ('virDomainRevertToSnapshot() failed', dom=self)
> libvirt.libvirtError: revert requires force: revert to snapshot while there is a managed saved state will cause corruption when run, remove saved state first
>
> Do you think this could go upstream as-is (plus a proper commit message
> of course)?
>
Ah nice, yes that looks good to me, I didn't know about the RISKY flag.
Other folks are the experts in this area though. Please propose that as
a patch to libvir-list@xxxxxxxxxx and CC me. A lot of people will be
offline for the holidays for a while so if you don't get a response
until mid january don't be surprised.
- Cole
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
