[virt-tools-list] [virt-manager RFC PATCH 0/2] Warn about saved state behaviour with snapshots

[Michael Weiser]

On Mon, Dec 23, 2019 at 04:12:06PM -0500, Cole Robinson wrote:
`
> > 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 at redhat.com 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.

Here's the thread for reference:

https://www.redhat.com/archives/libvir-list/2019-December/msg01456.html
https://www.redhat.com/archives/libvir-list/2020-January/msg00017.html
https://www.redhat.com/archives/libvir-list/2020-January/msg00068.html
-- 
Michael