diff mbox series

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

Message ID 20210415154402.28424-1-vkuznets@redhat.com (mailing list archive)
State New, archived
Headers show
Series [RFC] migration: warn about non-migratable configurations unless '--no-migration' was specified | expand

Commit Message

Vitaly Kuznetsov April 15, 2021, 3:44 p.m. UTC
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

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 include/qapi/qmp/qerror.h |  3 +++
 include/sysemu/sysemu.h   |  1 +
 migration/migration.c     | 18 +++++++++++++++++-
 qemu-options.hx           |  7 +++++++
 softmmu/globals.c         |  1 +
 softmmu/vl.c              |  3 +++
 6 files changed, 32 insertions(+), 1 deletion(-)

Comments

Daniel P. Berrangé April 15, 2021, 4:04 p.m. UTC | #1
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 ?

Libvirt recently introduced a new API 'virDomainGetMessages' which
lets us report a list of human targetted message strings against
a guest. We use it for reporting when an operation has tainted
a guest, and also use it for reporting when a deprecated QEMU
feature is used.  We could use it to report any migration
blockers that exist.

These are visible from 'virsh dominfo $guestname' and could also
be displayed by a mgmt application.

NB, the messages are intentionally declared opaque strings, so
mgmt apps shouldn't try to parse them. They merely know whether
the count is non-zero for any given message class.


Regards,
Daniel
Eduardo Habkost April 15, 2021, 4:30 p.m. UTC | #2
On Thu, Apr 15, 2021 at 12:04 PM 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.

I believe the libvirt API could be extended so applications can
indicate that migration is not a required feature. This would let QEMU
automatically enable useful but non-migration-friendly features. For
example, I would expect "-cpu host" to become the default CPU model if
--no-migration is specified.

>
> IOW, we can't use a --no-migration flag and will be stuck with these
> warnings no mtter what.

The expected behavior of the libvirt API is to create migratable VMs
by default, isn't it? This means it would be valid for libvirt to use
"--only-migratable" by default.

If libvirt can't use "--only-migratable" by default, I would say it's
a deficiency of the libvirt API that needs to be addressed.

>
> Is it possible to query the migration blockers via QMP ?

I don't think it is, but we can add that if it's useful for libvirt.

>
> Libvirt recently introduced a new API 'virDomainGetMessages' which
> lets us report a list of human targetted message strings against
> a guest. We use it for reporting when an operation has tainted
> a guest, and also use it for reporting when a deprecated QEMU
> feature is used.  We could use it to report any migration
> blockers that exist.
>
> These are visible from 'virsh dominfo $guestname' and could also
> be displayed by a mgmt application.

Cool!

Will the API include all warnings printed by QEMU to stderr?

>
> NB, the messages are intentionally declared opaque strings, so
> mgmt apps shouldn't try to parse them. They merely know whether
> the count is non-zero for any given message class.
>
>
> 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 :|
>

--
Eduardo
Daniel P. Berrangé April 15, 2021, 4:40 p.m. UTC | #3
On Thu, Apr 15, 2021 at 12:30:11PM -0400, Eduardo Habkost wrote:
> On Thu, Apr 15, 2021 at 12:04 PM 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.
> 
> I believe the libvirt API could be extended so applications can
> indicate that migration is not a required feature. This would let QEMU
> automatically enable useful but non-migration-friendly features. For
> example, I would expect "-cpu host" to become the default CPU model if
> --no-migration is specified.
> 
> >
> > IOW, we can't use a --no-migration flag and will be stuck with these
> > warnings no mtter what.
> 
> The expected behavior of the libvirt API is to create migratable VMs
> by default, isn't it? This means it would be valid for libvirt to use
> "--only-migratable" by default.

Yes & no.  Libvirt attempts to explicitly configure the guest so
that it has a stable ABI, and thus can theoretically be migrated
We doesn't try to secondguess which QEMU features may or may not
be migratable though.

> If libvirt can't use "--only-migratable" by default, I would say it's
> a deficiency of the libvirt API that needs to be addressed.

It is valid to boot a guest with an attached PCI device, which will
make it non-migratable, but later hot-unplug the PCI device before
starting the migration.

> > Is it possible to query the migration blockers via QMP ?
> 
> I don't think it is, but we can add that if it's useful for libvirt.

I think it would be useful.

> > Libvirt recently introduced a new API 'virDomainGetMessages' which
> > lets us report a list of human targetted message strings against
> > a guest. We use it for reporting when an operation has tainted
> > a guest, and also use it for reporting when a deprecated QEMU
> > feature is used.  We could use it to report any migration
> > blockers that exist.
> >
> > These are visible from 'virsh dominfo $guestname' and could also
> > be displayed by a mgmt application.
> 
> Cool!
> 
> Will the API include all warnings printed by QEMU to stderr?

No, we don't look at stderr - that's just going into a logfile.

When I refer to deprecated featurs here, I'm talking specifically
about stuff we see from the QAPI schema. Right now that means
CPU models and machine types with the deprecated flag. It can be
extended to other devices later.

Regards,
Daniel
Daniel P. Berrangé April 15, 2021, 5:07 p.m. UTC | #4
On Thu, Apr 15, 2021 at 05:40:40PM +0100, Daniel P. Berrangé wrote:
> On Thu, Apr 15, 2021 at 12:30:11PM -0400, Eduardo Habkost wrote:
> > On Thu, Apr 15, 2021 at 12:04 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > Is it possible to query the migration blockers via QMP ?
> > 
> > I don't think it is, but we can add that if it's useful for libvirt.
> 
> I think it would be useful.

To expand on how it could be used by a libvirt client if we could wire
this up in our message reporting API.

A client app using libvirt python API would do:

    msgs = dom.getMessages(libvirt.VIR_DOMAIN_MESSAGES_MIGBLOCKERS)

    if len(msgs) > 0:
        print("Domain %s uses features that block migration" % dom.getName())
	for msg in msgs:
	   print("   >> %s" % msg)


Migration blockers change over time, as hardware is hotplugged/unplugged,
so this isn't a one-off thing. The list of blockers is only valid at the
point in time that it is queried.

Regards,
Daniel
Dr. David Alan Gilbert April 15, 2021, 5:28 p.m. UTC | #5
* 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
    
Dave

> Libvirt recently introduced a new API 'virDomainGetMessages' which
> lets us report a list of human targetted message strings against
> a guest. We use it for reporting when an operation has tainted
> a guest, and also use it for reporting when a deprecated QEMU
> feature is used.  We could use it to report any migration
> blockers that exist.
> 
> These are visible from 'virsh dominfo $guestname' and could also
> be displayed by a mgmt application.
> 
> NB, the messages are intentionally declared opaque strings, so
> mgmt apps shouldn't try to parse them. They merely know whether
> the count is non-zero for any given message class.
> 
> 
> 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 :|
Vitaly Kuznetsov April 16, 2021, 7:33 a.m. UTC | #6
"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)
Eduardo Habkost April 16, 2021, 4:28 p.m. UTC | #7
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.

- 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.
Markus Armbruster April 17, 2021, 9:33 a.m. UTC | #8
Eduardo Habkost <ehabkost@redhat.com> writes:

[...]
> 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.
>
> - REQUIRED: live migration is required, and adding a migration
>   blocker would be a fatal error.

To be precise: would be an error.  During initial configuration, the
error is fatal, i.e. exit(1).  In a QMP command, it's not; only the
command fails.

>   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.

Makes sense to me (but of course we can still quibble about the names).
Markus Armbruster April 17, 2021, 9:35 a.m. UTC | #9
Vitaly Kuznetsov <vkuznets@redhat.com> writes:

> 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

Sure this justifies its own command line option?  Can we make it a
parameter of an existing option?  We have too many options, and options
starting "no-" are often awkward.

> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  include/qapi/qmp/qerror.h |  3 +++
>  include/sysemu/sysemu.h   |  1 +
>  migration/migration.c     | 18 +++++++++++++++++-
>  qemu-options.hx           |  7 +++++++
>  softmmu/globals.c         |  1 +
>  softmmu/vl.c              |  3 +++
>  6 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index 596fce0c54e7..2e1563c72f83 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -50,6 +50,9 @@
   /*
    * These macros will go away, please don't use in new code, and do not
    * add new ones!
    */
   ...
>  #define QERR_MISSING_PARAMETER \
>      "Parameter '%s' is missing"
>  
> +#define QERR_NO_MIGRATION \
> +    "Guest is not migratable ('--no-migration' used)"
> +
>  #define QERR_PERMISSION_DENIED \
>      "Insufficient permission to perform this operation"
>  

Please don't add new macros here.

Looks like you use QERR_NO_MIGRATION only in migration/migration.c.  You
can add a macro there.  Or simply duplicate the error string, which I'd
find easier to read.  Up to you.

> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 8fae667172ac..c65cd5d5a336 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -9,6 +9,7 @@
>  /* vl.c */
>  
>  extern int only_migratable;
> +extern int no_migration;
>  extern const char *qemu_name;
>  extern QemuUUID qemu_uuid;
>  extern bool qemu_uuid_set;
> diff --git a/migration/migration.c b/migration/migration.c
> index ca8b97baa5ac..29a8480ced54 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1077,7 +1077,9 @@ static void fill_source_migration_info(MigrationInfo *info)
>      info->blocked = migration_is_blocked(NULL);
>      info->has_blocked_reasons = info->blocked;
>      info->blocked_reasons = NULL;
> -    if (info->blocked) {
> +    if (no_migration) {
> +        QAPI_LIST_PREPEND(info->blocked_reasons, g_strdup(QERR_NO_MIGRATION));
> +    } else if (info->blocked) {
>          GSList *cur_blocker = migration_blockers;
>  
>          /*

Apropos blocked-reasons.  migration.json has:

    # @blocked: True if outgoing migration is blocked (since 6.0)
    #
    # @blocked-reasons: A list of reasons an outgoing migration is blocked (since 6.0)
    [...]
              'blocked': 'bool',
              '*blocked-reasons': ['str'],

Can "blocked-reasons" be absent or empty when "blocked" is true?

If not, then "blocked" is redundant, and should be dropped before we
release 6.0.

Else, the documentation should spell it out.  No need to rush that.

The patch was not cc'ed to me.  I might have caught it earlier...

[...]
Peter Maydell April 18, 2021, 3:53 p.m. UTC | #10
On Thu, 15 Apr 2021 at 16:46, Vitaly Kuznetsov <vkuznets@redhat.com> 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'm not a fan. For a lot of people and configurations this
is going to be "add an extra complaint from QEMU to a previously
working configuration". We add too many of those already.

thanks
-- PMM
Markus Armbruster April 19, 2021, 7:26 a.m. UTC | #11
Markus Armbruster <armbru@redhat.com> writes:

> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>
>> 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
>
> Sure this justifies its own command line option?  Can we make it a
> parameter of an existing option?  We have too many options, and options
> starting "no-" are often awkward.

We already have

    -only-migratable     allow only migratable devices

which fails any migration blocker, not just devices.
Markus Armbruster April 19, 2021, 3:46 p.m. UTC | #12
Markus Armbruster <armbru@redhat.com> writes:

[...]

> Apropos blocked-reasons.  migration.json has:
>
>     # @blocked: True if outgoing migration is blocked (since 6.0)
>     #
>     # @blocked-reasons: A list of reasons an outgoing migration is blocked (since 6.0)
>     [...]
>               'blocked': 'bool',
>               '*blocked-reasons': ['str'],
>
> Can "blocked-reasons" be absent or empty when "blocked" is true?

No.

From fill_source_migration_info():

        info->blocked = migration_is_blocked(NULL);
        info->has_blocked_reasons = info->blocked;
        info->blocked_reasons = NULL;
        if (info->blocked) {
            GSList *cur_blocker = migration_blockers;

            /*
             * There are two types of reasons a migration might be blocked;
             * a) devices marked in VMState as non-migratable, and
             * b) Explicit migration blockers
             * We need to add both of them here.
             */
            qemu_savevm_non_migratable_list(&info->blocked_reasons);

            while (cur_blocker) {
                QAPI_LIST_PREPEND(info->blocked_reasons,
                                  g_strdup(error_get_pretty(cur_blocker->data)));
                cur_blocker = g_slist_next(cur_blocker);
            }
        }

where

    bool migration_is_blocked(Error **errp)
    {
        if (qemu_savevm_state_blocked(errp)) {
            return true;
        }

        if (migration_blockers) {
            error_propagate(errp, error_copy(migration_blockers->data));
            return true;
        }

        return false;
    }

and

    bool qemu_savevm_state_blocked(Error **errp)
    {
        SaveStateEntry *se;

        QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
            if (se->vmsd && se->vmsd->unmigratable) {
                error_setg(errp, "State blocked by non-migratable device '%s'",
                           se->idstr);
                return true;
            }
        }
        return false;
    }

    void qemu_savevm_non_migratable_list(strList **reasons)
    {
        SaveStateEntry *se;

        QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
            if (se->vmsd && se->vmsd->unmigratable) {
                QAPI_LIST_PREPEND(*reasons,
                                  g_strdup_printf("non-migratable device: %s",
                                                  se->idstr));
            }
        }
    }

info->blocked is "non-migratable devices exist, or migration blockers
exist".

info->blocked_reasons has one entry per non-migratable device, and one
entry per migration blocker.

> If not, then "blocked" is redundant, and should be dropped before we
> release 6.0.

It is, and it should.

> Else, the documentation should spell it out.  No need to rush that.
>
> The patch was not cc'ed to me.  I might have caught it earlier...

"The patch" is commit 3af8554bd0 "migration: Add blocker information"

>
> [...]
Eduardo Habkost April 19, 2021, 4:28 p.m. UTC | #13
On Sun, Apr 18, 2021 at 11:54 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 15 Apr 2021 at 16:46, Vitaly Kuznetsov <vkuznets@redhat.com> 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'm not a fan. For a lot of people and configurations this
> is going to be "add an extra complaint from QEMU to a previously
> working configuration". We add too many of those already.

I agree that warning with machine types that never supported live
migration would be useless noise, but warning if using an explicit
versioned machine type sounds like a reasonable default (as long as
the warnings includes clear instructions on how to silence them).

--
Eduardo
Daniel P. Berrangé April 19, 2021, 4:37 p.m. UTC | #14
On Mon, Apr 19, 2021 at 12:28:04PM -0400, Eduardo Habkost wrote:
> On Sun, Apr 18, 2021 at 11:54 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Thu, 15 Apr 2021 at 16:46, Vitaly Kuznetsov <vkuznets@redhat.com> 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'm not a fan. For a lot of people and configurations this
> > is going to be "add an extra complaint from QEMU to a previously
> > working configuration". We add too many of those already.
> 
> I agree that warning with machine types that never supported live
> migration would be useless noise, but warning if using an explicit
> versioned machine type sounds like a reasonable default (as long as
> the warnings includes clear instructions on how to silence them).

Libvirt will always expand a machine type alias into a versioned
machine type, because stable ABI is useful even if never migrating,
because it ensures the guest OS doesn't see hardware changes that
may trigger license re-activation

At the same time a large portion of users will never care about
live migration/save/restore, or they do care but will hotunplug
the problems devices before attempting a migration.

IMHO tieing messages to versioned machine types is not desirable
as a strategy by default.

Warning about migration compatibility should be an explicit opt-in

Regards,
Daniel
Daniel P. Berrangé April 19, 2021, 4:42 p.m. UTC | #15
On Fri, Apr 16, 2021 at 12:28:01PM -0400, Eduardo Habkost 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.
> 
> - 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.

I'm not a fan of tieing migration behaviour to machine type
versioning as they are independant concepts. It is valid to
want to use versioned machine types even if you never migrate,
in order to keep stable guest ABI to avoid license activation
checks in guest OS.

Changing --only-migratable to a "--migration-policy preferred|required|none"
is reasonable, but I think we should just have a fixed global default for
it rather than trying to second-guess intentions.

Regards,
Daniel
Eduardo Habkost April 19, 2021, 4:48 p.m. UTC | #16
On Mon, Apr 19, 2021 at 12:42 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Apr 16, 2021 at 12:28:01PM -0400, Eduardo Habkost 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.
> >
> > - 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.
>
> I'm not a fan of tieing migration behaviour to machine type
> versioning as they are independant concepts. It is valid to
> want to use versioned machine types even if you never migrate,
> in order to keep stable guest ABI to avoid license activation
> checks in guest OS.
>
> Changing --only-migratable to a "--migration-policy preferred|required|none"
> is reasonable, but I think we should just have a fixed global default for
> it rather than trying to second-guess intentions.

Right, if we agree to make it opt-in we probably don't even need a
warning mode. `--only-migratable` should be enough. This means UNKNOWN
from my list above wouldn't exist, and the existing default is already
PREFERRED (which is the one expected by libvirt today).

--
Eduardo
Dr. David Alan Gilbert April 19, 2021, 5:11 p.m. UTC | #17
* 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.

Dave

> - 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.
> 
> -- 
> Eduardo
Daniel P. Berrangé April 19, 2021, 5:15 p.m. UTC | #18
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
Daniel P. Berrangé April 19, 2021, 5:17 p.m. UTC | #19
On Mon, Apr 19, 2021 at 06:15:56PM +0100, Daniel P. Berrangé wrote:
> On Mon, Apr 19, 2021 at 06:11:47PM +0100, Dr. David Alan Gilbert wrote:
> > * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > > 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.

Having said that, we potentially don't need a dedicated arg if we
just make  'migratable=none|preferred|required' be a property of
the machine type and hook everything off that

> 
> > > - 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
Dr. David Alan Gilbert April 19, 2021, 6:47 p.m. UTC | #20
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Mon, Apr 19, 2021 at 06:15:56PM +0100, Daniel P. Berrangé wrote:
> > On Mon, Apr 19, 2021 at 06:11:47PM +0100, Dr. David Alan Gilbert wrote:
> > > * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > > > 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.
> 
> Having said that, we potentially don't need a dedicated arg if we
> just make  'migratable=none|preferred|required' be a property of
> the machine type and hook everything off that

I think my only difficulty with that is that I don't find any of those
3 words 'obvious'.

Dave

> > 
> > > > - 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 :|
Eduardo Habkost April 19, 2021, 7:32 p.m. UTC | #21
On Mon, Apr 19, 2021 at 07:47:34PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > On Mon, Apr 19, 2021 at 06:15:56PM +0100, Daniel P. Berrangé wrote:
> > > On Mon, Apr 19, 2021 at 06:11:47PM +0100, Dr. David Alan Gilbert wrote:
> > > > * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > > > > 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.
> > 
> > Having said that, we potentially don't need a dedicated arg if we
> > just make  'migratable=none|preferred|required' be a property of
> > the machine type and hook everything off that
> 
> I think my only difficulty with that is that I don't find any of those
> 3 words 'obvious'.

Any suggestions of replacements for those 3 words?

Would the descriptions below be enough to clarify their meaning
in documentation?

- NONE: live migration is not needed, and device or machine code
  is allowed to enable features that block live migration or
  change guest ABI.
  (Not implemented yet)

- PREFERRED: machine and device code should try to make the VM
  migratable when possible, but won't emit a warning or error out
  if migration is blocked.
  (Current default behavior)

- REQUIRED: live migration support is required, and adding a
  migration blocker will be an error.
  (Implemented today by --only-migratable)
Dr. David Alan Gilbert April 20, 2021, 11:51 a.m. UTC | #22
* Eduardo Habkost (ehabkost@redhat.com) wrote:
> On Mon, Apr 19, 2021 at 07:47:34PM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > On Mon, Apr 19, 2021 at 06:15:56PM +0100, Daniel P. Berrangé wrote:
> > > > On Mon, Apr 19, 2021 at 06:11:47PM +0100, Dr. David Alan Gilbert wrote:
> > > > > * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > > > > > 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.
> > > 
> > > Having said that, we potentially don't need a dedicated arg if we
> > > just make  'migratable=none|preferred|required' be a property of
> > > the machine type and hook everything off that
> > 
> > I think my only difficulty with that is that I don't find any of those
> > 3 words 'obvious'.
> 
> Any suggestions of replacements for those 3 words?
> 
> Would the descriptions below be enough to clarify their meaning
> in documentation?

I prefer things that are fairly obvious without needing to look at the
documentation until you want the detail.

> - NONE: live migration is not needed, and device or machine code
>   is allowed to enable features that block live migration or
>   change guest ABI.
>   (Not implemented yet)
> 
> - PREFERRED: machine and device code should try to make the VM
>   migratable when possible, but won't emit a warning or error out
>   if migration is blocked.
>   (Current default behavior)
> 
> - REQUIRED: live migration support is required, and adding a
>   migration blocker will be an error.
>   (Implemented today by --only-migratable)

How about
  -migratable blocked
     Live migration is not allowed; an outbound migration will fail

  -migratable allowed
     Live migration is allowed, but some devices/options may block
     it if they're unable to migrate [current default]

  -migratable warn
      Live migration is allowed, but if some device/option is
      unable to migrate, migration will be blocked and a warning
      printed

  -migratable required
      Live migration is allowed, attempting to add a device or
      enable an option that can't migrate will fail. [--only-migratable]
      
Dave

> -- 
> Eduardo
Eduardo Habkost April 20, 2021, 1:48 p.m. UTC | #23
On Tue, Apr 20, 2021 at 12:51:43PM +0100, Dr. David Alan Gilbert wrote:
> * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > On Mon, Apr 19, 2021 at 07:47:34PM +0100, Dr. David Alan Gilbert wrote:
> > > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > > On Mon, Apr 19, 2021 at 06:15:56PM +0100, Daniel P. Berrangé wrote:
> > > > > On Mon, Apr 19, 2021 at 06:11:47PM +0100, Dr. David Alan Gilbert wrote:
> > > > > > * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > > > > > > 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.
> > > > 
> > > > Having said that, we potentially don't need a dedicated arg if we
> > > > just make  'migratable=none|preferred|required' be a property of
> > > > the machine type and hook everything off that
> > > 
> > > I think my only difficulty with that is that I don't find any of those
> > > 3 words 'obvious'.
> > 
> > Any suggestions of replacements for those 3 words?
> > 
> > Would the descriptions below be enough to clarify their meaning
> > in documentation?
> 
> I prefer things that are fairly obvious without needing to look at the
> documentation until you want the detail.
> 
> > - NONE: live migration is not needed, and device or machine code
> >   is allowed to enable features that block live migration or
> >   change guest ABI.
> >   (Not implemented yet)
> > 
> > - PREFERRED: machine and device code should try to make the VM
> >   migratable when possible, but won't emit a warning or error out
> >   if migration is blocked.
> >   (Current default behavior)
> > 
> > - REQUIRED: live migration support is required, and adding a
> >   migration blocker will be an error.
> >   (Implemented today by --only-migratable)
> 
> How about
>   -migratable blocked
>      Live migration is not allowed; an outbound migration will fail

"none" and NOT_NEEDED above were about letting QEMU automatically
enable features that block live migration or change guest ABI.

If that's implied by "blocked", I'd like to get that documented
explicitly.  If that's not implied by "blocked", I don't
understand what's the use case for "blocked".


> 
>   -migratable allowed
>      Live migration is allowed, but some devices/options may block
>      it if they're unable to migrate [current default]

"preferred" above was about QEMU trying to keep live migration
working as much as possible.  That's something we all expect QEMU
to do, but it's not documented anywhere.

If that's implied by "allowed", I'd like to get that documented
explicitly.  If that's not implied by "allowed", then we have a
problem.


> 
>   -migratable warn
>       Live migration is allowed, but if some device/option is
>       unable to migrate, migration will be blocked and a warning
>       printed

This makes sense, but I don't think we need to support this use
case.

> 
>   -migratable required
>       Live migration is allowed, attempting to add a device or
>       enable an option that can't migrate will fail. [--only-migratable]

This matches "required" above.
Dr. David Alan Gilbert April 20, 2021, 2:10 p.m. UTC | #24
* Eduardo Habkost (ehabkost@redhat.com) wrote:
> On Tue, Apr 20, 2021 at 12:51:43PM +0100, Dr. David Alan Gilbert wrote:
> > * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > > On Mon, Apr 19, 2021 at 07:47:34PM +0100, Dr. David Alan Gilbert wrote:
> > > > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > > > On Mon, Apr 19, 2021 at 06:15:56PM +0100, Daniel P. Berrangé wrote:
> > > > > > On Mon, Apr 19, 2021 at 06:11:47PM +0100, Dr. David Alan Gilbert wrote:
> > > > > > > * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > > > > > > > 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.
> > > > > 
> > > > > Having said that, we potentially don't need a dedicated arg if we
> > > > > just make  'migratable=none|preferred|required' be a property of
> > > > > the machine type and hook everything off that
> > > > 
> > > > I think my only difficulty with that is that I don't find any of those
> > > > 3 words 'obvious'.
> > > 
> > > Any suggestions of replacements for those 3 words?
> > > 
> > > Would the descriptions below be enough to clarify their meaning
> > > in documentation?
> > 
> > I prefer things that are fairly obvious without needing to look at the
> > documentation until you want the detail.
> > 
> > > - NONE: live migration is not needed, and device or machine code
> > >   is allowed to enable features that block live migration or
> > >   change guest ABI.
> > >   (Not implemented yet)
> > > 
> > > - PREFERRED: machine and device code should try to make the VM
> > >   migratable when possible, but won't emit a warning or error out
> > >   if migration is blocked.
> > >   (Current default behavior)
> > > 
> > > - REQUIRED: live migration support is required, and adding a
> > >   migration blocker will be an error.
> > >   (Implemented today by --only-migratable)
> > 
> > How about
> >   -migratable blocked
> >      Live migration is not allowed; an outbound migration will fail
> 
> "none" and NOT_NEEDED above were about letting QEMU automatically
> enable features that block live migration or change guest ABI.
> 
> If that's implied by "blocked", I'd like to get that documented
> explicitly.  If that's not implied by "blocked", I don't
> understand what's the use case for "blocked".

My 'blocked' is stronger - migration is hard disabled by a blocker
always; it's for (rare) cases where the user wants to stop a migration
happening, even if qemu believes it can do it.

> > 
> >   -migratable allowed
> >      Live migration is allowed, but some devices/options may block
> >      it if they're unable to migrate [current default]
> 
> "preferred" above was about QEMU trying to keep live migration
> working as much as possible.  That's something we all expect QEMU
> to do, but it's not documented anywhere.
> 
> If that's implied by "allowed", I'd like to get that documented
> explicitly.  If that's not implied by "allowed", then we have a
> problem.

My difficulty by your definition is I don't understand what
'working as much as possible' means - that's the current behaviour
as I understand it.   I think mine is more explicit.

> 
> > 
> >   -migratable warn
> >       Live migration is allowed, but if some device/option is
> >       unable to migrate, migration will be blocked and a warning
> >       printed
> 
> This makes sense, but I don't think we need to support this use
> case.

I thought that was exactly what Vitaly's patch tried to do?

> > 
> >   -migratable required
> >       Live migration is allowed, attempting to add a device or
> >       enable an option that can't migrate will fail. [--only-migratable]
> 
> This matches "required" above.

Dave


> -- 
> Eduardo
Daniel P. Berrangé April 20, 2021, 2:15 p.m. UTC | #25
On Tue, Apr 20, 2021 at 03:10:44PM +0100, Dr. David Alan Gilbert wrote:
> * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > On Tue, Apr 20, 2021 at 12:51:43PM +0100, Dr. David Alan Gilbert wrote:
> > > I prefer things that are fairly obvious without needing to look at the
> > > documentation until you want the detail.
> > > 
> > > > - NONE: live migration is not needed, and device or machine code
> > > >   is allowed to enable features that block live migration or
> > > >   change guest ABI.
> > > >   (Not implemented yet)
> > > > 
> > > > - PREFERRED: machine and device code should try to make the VM
> > > >   migratable when possible, but won't emit a warning or error out
> > > >   if migration is blocked.
> > > >   (Current default behavior)
> > > > 
> > > > - REQUIRED: live migration support is required, and adding a
> > > >   migration blocker will be an error.
> > > >   (Implemented today by --only-migratable)
> > > 
> > > How about
> > >   -migratable blocked
> > >      Live migration is not allowed; an outbound migration will fail
> > 
> > "none" and NOT_NEEDED above were about letting QEMU automatically
> > enable features that block live migration or change guest ABI.
> > 
> > If that's implied by "blocked", I'd like to get that documented
> > explicitly.  If that's not implied by "blocked", I don't
> > understand what's the use case for "blocked".
> 
> My 'blocked' is stronger - migration is hard disabled by a blocker
> always; it's for (rare) cases where the user wants to stop a migration
> happening, even if qemu believes it can do it.
> 
> > > 
> > >   -migratable allowed
> > >      Live migration is allowed, but some devices/options may block
> > >      it if they're unable to migrate [current default]
> > 
> > "preferred" above was about QEMU trying to keep live migration
> > working as much as possible.  That's something we all expect QEMU
> > to do, but it's not documented anywhere.
> > 
> > If that's implied by "allowed", I'd like to get that documented
> > explicitly.  If that's not implied by "allowed", then we have a
> > problem.
> 
> My difficulty by your definition is I don't understand what
> 'working as much as possible' means - that's the current behaviour
> as I understand it.   I think mine is more explicit.

I think it helps to illustrate examples. The obvious case is where
thre are new kernel features QEMU wants to use. Normally we try to
avoid using them because that reduces potential targets for
migration to those with the same kernel or newer.

So a "preferred" / "allowed" mode would avoid automagically using
new host kernel features, while a "blocked" / "none" mode would
be free to use any kernel features

> > > 
> > >   -migratable warn
> > >       Live migration is allowed, but if some device/option is
> > >       unable to migrate, migration will be blocked and a warning
> > >       printed
> > 
> > This makes sense, but I don't think we need to support this use
> > case.
> 
> I thought that was exactly what Vitaly's patch tried to do?

Yep, but I'm sceptical how useful it actually is in practice.

Whether migration is blocked at the time the VM starts is not
really important, because VM configuration can be changed
at runtime, making any warnings obsolete/inaccurate for anyone
reading them after the fact.

> 
> > > 
> > >   -migratable required
> > >       Live migration is allowed, attempting to add a device or
> > >       enable an option that can't migrate will fail. [--only-migratable]
> > 
> > This matches "required" above.
> 
> Dave
> 
> 
> > -- 
> > Eduardo
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

Regards,
Daniel
Eduardo Habkost April 20, 2021, 3:20 p.m. UTC | #26
On Tue, Apr 20, 2021 at 03:10:44PM +0100, Dr. David Alan Gilbert wrote:
> * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > On Tue, Apr 20, 2021 at 12:51:43PM +0100, Dr. David Alan Gilbert wrote:
> > > * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > > > On Mon, Apr 19, 2021 at 07:47:34PM +0100, Dr. David Alan Gilbert wrote:
> > > > > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > > > > On Mon, Apr 19, 2021 at 06:15:56PM +0100, Daniel P. Berrangé wrote:
> > > > > > > On Mon, Apr 19, 2021 at 06:11:47PM +0100, Dr. David Alan Gilbert wrote:
> > > > > > > > * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > > > > > > > > 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.
> > > > > > 
> > > > > > Having said that, we potentially don't need a dedicated arg if we
> > > > > > just make  'migratable=none|preferred|required' be a property of
> > > > > > the machine type and hook everything off that
> > > > > 
> > > > > I think my only difficulty with that is that I don't find any of those
> > > > > 3 words 'obvious'.
> > > > 
> > > > Any suggestions of replacements for those 3 words?
> > > > 
> > > > Would the descriptions below be enough to clarify their meaning
> > > > in documentation?
> > > 
> > > I prefer things that are fairly obvious without needing to look at the
> > > documentation until you want the detail.
> > > 
> > > > - NONE: live migration is not needed, and device or machine code
> > > >   is allowed to enable features that block live migration or
> > > >   change guest ABI.
> > > >   (Not implemented yet)
> > > > 
> > > > - PREFERRED: machine and device code should try to make the VM
> > > >   migratable when possible, but won't emit a warning or error out
> > > >   if migration is blocked.
> > > >   (Current default behavior)
> > > > 
> > > > - REQUIRED: live migration support is required, and adding a
> > > >   migration blocker will be an error.
> > > >   (Implemented today by --only-migratable)
> > > 
> > > How about
> > >   -migratable blocked
> > >      Live migration is not allowed; an outbound migration will fail
> > 
> > "none" and NOT_NEEDED above were about letting QEMU automatically
> > enable features that block live migration or change guest ABI.
> > 
> > If that's implied by "blocked", I'd like to get that documented
> > explicitly.  If that's not implied by "blocked", I don't
> > understand what's the use case for "blocked".
> 
> My 'blocked' is stronger - migration is hard disabled by a blocker
> always; it's for (rare) cases where the user wants to stop a migration
> happening, even if qemu believes it can do it.

The main point of my "not_needed" suggestion was to get
convenient defaults for use cases where live migration and guest
ABI stability is not needed.

I don't know yet if that's an implicit feature of your definition
of "blocked", or not.

> 
> > > 
> > >   -migratable allowed
> > >      Live migration is allowed, but some devices/options may block
> > >      it if they're unable to migrate [current default]
> > 
> > "preferred" above was about QEMU trying to keep live migration
> > working as much as possible.  That's something we all expect QEMU
> > to do, but it's not documented anywhere.
> > 
> > If that's implied by "allowed", I'd like to get that documented
> > explicitly.  If that's not implied by "allowed", then we have a
> > problem.
> 
> My difficulty by your definition is I don't understand what
> 'working as much as possible' means - that's the current behaviour
> as I understand it.   I think mine is more explicit.

There's something I don't see documented in your proposal, which
I've tried to describe it as "try to keep live migration working
as much as possible".  I'll try a more detailed description:

  - Live migration will be possible unless some specific devices
    or features are enabled.
  - Devices or features that block live migration won't be
    enabled by default.
  - Devices or features that break guest ABI won't be enabled by
    default.
  - If an existing configuration allows live migration on a given
    QEMU version, the same configuration will allow live
    migration in newer QEMU versions.
  - If an existing configuration allows live migration with a
    given machine type version, the same configuration will allow
    live migration in a newer version of that machine type.

If all of the above is implied by "allowed", I'd like to get this
documented explicitly.  If it's not, we still have a problem.
diff mbox series

Patch

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 596fce0c54e7..2e1563c72f83 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -50,6 +50,9 @@ 
 #define QERR_MISSING_PARAMETER \
     "Parameter '%s' is missing"
 
+#define QERR_NO_MIGRATION \
+    "Guest is not migratable ('--no-migration' used)"
+
 #define QERR_PERMISSION_DENIED \
     "Insufficient permission to perform this operation"
 
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 8fae667172ac..c65cd5d5a336 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -9,6 +9,7 @@ 
 /* vl.c */
 
 extern int only_migratable;
+extern int no_migration;
 extern const char *qemu_name;
 extern QemuUUID qemu_uuid;
 extern bool qemu_uuid_set;
diff --git a/migration/migration.c b/migration/migration.c
index ca8b97baa5ac..29a8480ced54 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1077,7 +1077,9 @@  static void fill_source_migration_info(MigrationInfo *info)
     info->blocked = migration_is_blocked(NULL);
     info->has_blocked_reasons = info->blocked;
     info->blocked_reasons = NULL;
-    if (info->blocked) {
+    if (no_migration) {
+        QAPI_LIST_PREPEND(info->blocked_reasons, g_strdup(QERR_NO_MIGRATION));
+    } else if (info->blocked) {
         GSList *cur_blocker = migration_blockers;
 
         /*
@@ -2048,6 +2050,10 @@  void migrate_init(MigrationState *s)
 
 int migrate_add_blocker(Error *reason, Error **errp)
 {
+    if (!no_migration) {
+        warn_report("Guest won't be migratable: %s", error_get_pretty(reason));
+    }
+
     if (only_migratable) {
         error_propagate_prepend(errp, error_copy(reason),
                                 "disallowing migration blocker "
@@ -2155,6 +2161,11 @@  bool migration_is_blocked(Error **errp)
         return true;
     }
 
+    if (no_migration) {
+        error_setg(errp, QERR_NO_MIGRATION);
+        return true;
+    }
+
     if (migration_blockers) {
         error_propagate(errp, error_copy(migration_blockers->data));
         return true;
@@ -2198,6 +2209,11 @@  static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
         return true;
     }
 
+    if (no_migration) {
+        error_setg(errp, QERR_NO_MIGRATION);
+        return false;
+    }
+
     if (migration_is_running(s->state)) {
         error_setg(errp, QERR_MIGRATION_ACTIVE);
         return false;
diff --git a/qemu-options.hx b/qemu-options.hx
index fd21002bd61d..3443130273e9 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4234,6 +4234,13 @@  SRST
     an unmigratable state.
 ERST
 
+DEF("no-migration", 0, QEMU_OPTION_no_migration, \
+    "-no-migration     disallow migration\n", QEMU_ARCH_ALL)
+SRST
+``-no-migration``
+    Disallow migration. Don't warn about non-migratable configurations.
+ERST
+
 DEF("nodefaults", 0, QEMU_OPTION_nodefaults, \
     "-nodefaults     don't create default devices\n", QEMU_ARCH_ALL)
 SRST
diff --git a/softmmu/globals.c b/softmmu/globals.c
index 7d0fc811835a..bb0d892df307 100644
--- a/softmmu/globals.c
+++ b/softmmu/globals.c
@@ -59,6 +59,7 @@  int boot_menu;
 bool boot_strict;
 uint8_t *boot_splash_filedata;
 int only_migratable; /* turn it off unless user states otherwise */
+int no_migration;
 int icount_align_option;
 
 /* The bytes in qemu_uuid are in the order specified by RFC4122, _not_ in the
diff --git a/softmmu/vl.c b/softmmu/vl.c
index aadb52613888..9a6535e594c3 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3339,6 +3339,9 @@  void qemu_init(int argc, char **argv, char **envp)
             case QEMU_OPTION_only_migratable:
                 only_migratable = 1;
                 break;
+            case QEMU_OPTION_no_migration:
+                no_migration = 1;
+                break;
             case QEMU_OPTION_nodefaults:
                 has_defaults = 0;
                 break;