Re: [PATCH RFC] migration: warn about non-migratable configurations unless '--no-migration' was specified

[Daniel P . Berrangé]

On Mon, Apr 19, 2021 at 06:11:47PM +0100, Dr. David Alan Gilbert wrote:
> * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > On Fri, Apr 16, 2021 at 09:33:28AM +0200, Vitaly Kuznetsov wrote:
> > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> > > 
> > > > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > >> On Thu, Apr 15, 2021 at 05:44:02PM +0200, Vitaly Kuznetsov wrote:
> > > >> > When a migration blocker is added nothing is reported to the user,
> > > >> > inability to migrate such guest may come as a late surprise. As a 
> > > >> > bare
> > > >> > minimum, we can print a warning. To not pollute the output for 
> > > >> > those, who
> > > >> > have no intention to migrate their guests, introduce '--no-migration'
> > > >> > option which both block the migration and eliminates warning from
> > > >> 
> > > >> I wonder how this is actually going to work in practice ?
> > > >> 
> > > >> At the time libvirt starts a guest, it has no idea whether the guest
> > > >> is likely to need migration 3, 6, 12, 24 months in to the future.
> > > >> 
> > > >> IOW, we can't use a --no-migration flag and will be stuck with these
> > > >> warnings no mtter what.
> > > >> 
> > > >> Is it possible to query the migration blockers via QMP ?
> > > >
> > > > It's possible to query the currently active ones, as of 6.0; from my
> > > > commit  3af8554bd068576b0399087583df48518a2a98f6 it appears in the
> > > > output of query-migrate in the 'blocked-reasons' list.
> > > >
> > > > The HMP equivalent is a64aec725ea0b26fa4e44f8b8b8c72be9aaa4230 showing:
> > > >
> > > >     (qemu) info migrate
> > > >     globals:
> > > >     store-global-state: on
> > > >     only-migratable: off
> > > >     send-configuration: on
> > > >     send-section-footer: on
> > > >     decompress-error-check: on
> > > >     clear-bitmap-shift: 18
> > > >     Outgoing migration blocked:
> > > >       Migration is disabled when VirtFS export path '/home' is mounted 
> > > > in the guest using mount_tag 'fs'
> > > >       non-migratable device: 0000:00:01.2/1/usb-serial
> > > >     
> > > 
> > > FWIW, this patch makes '--no-migration' an 'ultimate big hammer' so not
> > > matter how many blockers are there, the output will look like:
> > > 
> > > (qemu) info migrate
> > > globals:
> > > store-global-state: on
> > > only-migratable: off
> > > send-configuration: on
> > > send-section-footer: on
> > > decompress-error-check: on
> > > clear-bitmap-shift: 18
> > > Outgoing migration blocked:
> > >   Guest is not migratable ('--no-migration' used)
> > 
> > I would change that.  I expect "--no-migration" to only mean
> > "live migration not really needed", not "live migration should be
> > blocked".
> > 
> > However, I still don't think libvirt should say "live migration
> > not needed" unconditionally (because this isn't always true).  In
> > that case, we would need a different mechanism to silence the
> > warnings somehow.
> > 
> > I would make live migration policy an enum, just to make sure
> > we are explicit about the requirements:
> > 
> > - UNKNOWN: this is the current state in QEMU 6.0, where we don't
> >   really know what the user expects.
> >   This can be the default on existing versioned machine types,
> >   just for compatibility.
> >   I suggest making this print warnings for every migration
> >   blocker (like this patch does).
> >   I suggest deprecating this behavior as soon as we can.
> > 
> > - PREFERRED: try to make the VM migratable when possible, but
> >   don't print a warning or error out if migration is blocked.
> >   This seems to be the behavior expected by libvirt today.
> > 
> > - NOT_NEEDED: live migration is not needed, and QEMU is free to
> >   enable features that block live migration or change guest ABI.
> >   We can probably make this the default on machine types that
> >   never supported live migration.
> 
> I suggest you could do this by adding:
>   -warn-none-migratable
>   -no-warn-none-migratable
> 
> and then argue about defaults another time.

If we're going to add new args, lets at least future proof our
approach with an extensible option that we can wire into QMP
too later

  -migratable  none|preferred|required 

and letting us add extra key/value pairs to tune it if desired.

> > - REQUIRED: live migration is required, and adding a migration
> >   blocker would be a fatal error.
> >   This is already implemented by --only-migratable.
> >   I suggest making this the default on versioned machine types
> >   after a few releases, and after deprecating UNKNOWN.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|