[libvirt PATCH 02/11] qemu_snapshot: revert: always restart QEMU process for running VM

Peter Krempa pkrempa at redhat.com
Mon Nov 22 11:54:43 UTC 2021


On Mon, Nov 22, 2021 at 12:32:03 +0100, Pavel Hrdina wrote:
> On Tue, Nov 16, 2021 at 03:17:56PM +0100, Peter Krempa wrote:
> > On Mon, Nov 15, 2021 at 17:22:45 +0100, Pavel Hrdina wrote:
> > > Our compatibility check code isn't complete and there are cases where it
> > > fails to detect incompatible configuration and the revert fails. In
> > > addition future support for external snapshot will always require
> > > restarting the QEMU process.
> > > 
> > > To unify the behavior drop the compatibility check code and always
> > > restart the QEMU process.
> > > 
> > > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > > ---
> > >  src/qemu/qemu_snapshot.c | 66 ++++++----------------------------------
> > >  1 file changed, 10 insertions(+), 56 deletions(-)
> > > 
> > > diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> > > index 3d6ec490ab..661f74146c 100644
> > > --- a/src/qemu/qemu_snapshot.c
> > > +++ b/src/qemu/qemu_snapshot.c
> > > @@ -1989,62 +1989,16 @@ qemuSnapshotRevert(virDomainObj *vm,
> > 
> > [...]
> > 
> > >              /* Transitions 5, 6, 8, 9 */
> > > -            /* If using VM GenID, there is no way currently to change
> > > -             * the genid for the running guest, so set an error,
> > > -             * mark as incompatible, and don't allow change of genid
> > > -             * if the revert force flag would start the guest again. */
> > > -            if (compatible && config->genidRequested) {
> > > -                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > > -                               _("domain genid update requires restart"));
> > > -                compatible = false;
> > > -                start_flags &= ~VIR_QEMU_PROCESS_START_GEN_VMID;
> > 
> > We still need this bit. If genid is requested we must ensure that the
> > new start of the VM will regenerate it to ensure that the guest can
> > detect the reversion.
> 
> At the beginning of the function qemuSnapshotRevert() there is this
> line:
> 
>     unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID;
> 
> so generating of VMID is enabled by default. This part of code disabled
> the VMID regeneration if FORCE flag had to be used to revert the
> snapshot. That change was introduced by commit <e5d7064be02a0105b7c>
> but it doesn't make sense to me.
> 
> The commit messages states:
> 
>     If the the snapshot revert involves a forced revert option, then
>     let's not cause startup to change the genid flag in order to signify
>     that we're still running the same/previous guest and not some
>     snapshot reversion.
> 
> but how we would run the same guest if this code is reverting to
> different snapshot?

Oh, I didn't actually notice that this is masking out the
VIR_QEMU_PROCESS_START_GEN_VMID flag, so yeah it can be removed
completely.




More information about the libvir-list mailing list