[PATCH 0/2] migration blocker information

[PATCH 0/2] migration blocker information

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Hi,
  This is a rework of 'migration: Provide a test for migratability',
based on Eric's feedback this is now an addition to the existing
info/query migrate commands rather than a new command.

  One other importovement is that we now list all the reasons for the
block rather than just one.

Dave

Dr. David Alan Gilbert (2):
  migration: Add blocker information
  migration: Display the migration blockers

 migration/migration.c | 25 +++++++++++++++++++++++--
 migration/savevm.c    | 13 +++++++++++++
 migration/savevm.h    |  1 +
 monitor/hmp-cmds.c    |  9 +++++++++
 qapi/migration.json   |  6 ++++++
 5 files changed, 52 insertions(+), 2 deletions(-)

-- 
2.29.2

[PATCH 1/2] migration: Add blocker information

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Modify query-migrate so that it has a flag indicating if outbound
migration is blocked, and if it is a list of reasons.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 25 +++++++++++++++++++++++--
 migration/savevm.c    | 13 +++++++++++++
 migration/savevm.h    |  1 +
 qapi/migration.json   |  6 ++++++
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 1986cb8573..21d7ab1fa4 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -141,6 +141,8 @@ enum mig_rp_message_type {
 static MigrationState *current_migration;
 static MigrationIncomingState *current_incoming;
 
+static GSList *migration_blockers;
+
 static bool migration_object_check(MigrationState *ms, Error **errp);
 static int migration_maybe_pause(MigrationState *s,
                                  int *current_active_state,
@@ -1041,6 +1043,27 @@ static void fill_source_migration_info(MigrationInfo *info)
 {
     MigrationState *s = migrate_get_current();
 
+    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);
+        }
+    }
+
     switch (s->state) {
     case MIGRATION_STATUS_NONE:
         /* no migration has happened ever */
@@ -1934,8 +1957,6 @@ void migrate_init(MigrationState *s)
     s->threshold_size = 0;
 }
 
-static GSList *migration_blockers;
-
 int migrate_add_blocker(Error *reason, Error **errp)
 {
     if (only_migratable) {
diff --git a/migration/savevm.c b/migration/savevm.c
index 4f3b69ecfc..7ddf400ca3 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1131,6 +1131,19 @@ bool qemu_savevm_state_blocked(Error **errp)
     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));
+        }
+    }
+}
+
 void qemu_savevm_state_header(QEMUFile *f)
 {
     trace_savevm_state_header();
diff --git a/migration/savevm.h b/migration/savevm.h
index ba64a7e271..ec789b161d 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -30,6 +30,7 @@
 #define QEMU_VM_SECTION_FOOTER       0x7e
 
 bool qemu_savevm_state_blocked(Error **errp);
+void qemu_savevm_non_migratable_list(strList **reasons);
 void qemu_savevm_state_setup(QEMUFile *f);
 bool qemu_savevm_state_guest_unplug_pending(void);
 int qemu_savevm_state_resume_prepare(MigrationState *s);
diff --git a/qapi/migration.json b/qapi/migration.json
index d1d9632c2a..2f9d3d7237 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -224,6 +224,10 @@
 #        only returned if VFIO device is present, migration is supported by all
 #        VFIO devices and status is 'active' or 'completed' (since 5.2)
 #
+# @blocked: True if outgoing migration is blocked (since 6.0)
+#
+# @blocked-reasons: A list of reasons an outgoing migration is blocked (since 6.0)
+#
 # Since: 0.14
 ##
 { 'struct': 'MigrationInfo',
@@ -237,6 +241,8 @@
            '*setup-time': 'int',
            '*cpu-throttle-percentage': 'int',
            '*error-desc': 'str',
+           'blocked': 'bool',
+           '*blocked-reasons': ['str'],
            '*postcopy-blocktime' : 'uint32',
            '*postcopy-vcpu-blocktime': ['uint32'],
            '*compression': 'CompressionStats',
-- 
2.29.2

Re: [PATCH 1/2] migration: Add blocker information

[Eric Blake]

On 2/2/21 7:55 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Modify query-migrate so that it has a flag indicating if outbound
> migration is blocked, and if it is a list of reasons.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  migration/migration.c | 25 +++++++++++++++++++++++--
>  migration/savevm.c    | 13 +++++++++++++
>  migration/savevm.h    |  1 +
>  qapi/migration.json   |  6 ++++++
>  4 files changed, 43 insertions(+), 2 deletions(-)
> 

> +++ b/qapi/migration.json
> @@ -224,6 +224,10 @@
>  #        only returned if VFIO device is present, migration is supported by all
>  #        VFIO devices and status is 'active' or 'completed' (since 5.2)
>  #
> +# @blocked: True if outgoing migration is blocked (since 6.0)
> +#
> +# @blocked-reasons: A list of reasons an outgoing migration is blocked (since 6.0)

A bit of redundancy here; the presence of blocked-reasons is sufficient
to tell if migration is blocked, without needing a bool.  But having the
bool is more type-friendly for machines, so I don't mind having both.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 1/2] migration: Add blocker information

[Peter Xu]

On Tue, Feb 02, 2021 at 08:34:06AM -0600, Eric Blake wrote:
> On 2/2/21 7:55 AM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Modify query-migrate so that it has a flag indicating if outbound
> > migration is blocked, and if it is a list of reasons.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  migration/migration.c | 25 +++++++++++++++++++++++--
> >  migration/savevm.c    | 13 +++++++++++++
> >  migration/savevm.h    |  1 +
> >  qapi/migration.json   |  6 ++++++
> >  4 files changed, 43 insertions(+), 2 deletions(-)
> > 
> 
> > +++ b/qapi/migration.json
> > @@ -224,6 +224,10 @@
> >  #        only returned if VFIO device is present, migration is supported by all
> >  #        VFIO devices and status is 'active' or 'completed' (since 5.2)
> >  #
> > +# @blocked: True if outgoing migration is blocked (since 6.0)
> > +#
> > +# @blocked-reasons: A list of reasons an outgoing migration is blocked (since 6.0)
> 
> A bit of redundancy here; the presence of blocked-reasons is sufficient
> to tell if migration is blocked, without needing a bool.  But having the
> bool is more type-friendly for machines, so I don't mind having both.

Agreed.

Also I'd also think we can directly export the device list to JSON then we can
have {"blockers": { "devices": {...}, "other": {...}}} so upper layer can do
more things like "would you want to unplug the device XXX to make it
migratable?" instead of parsing the error message first.

But non of the above is a big deal, I think:

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

[PATCH 2/2] migration: Display the migration blockers

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Update 'info migrate' to display migration blocking information.
If the outbound migration is not blocked, there is no change, however
if it is blocked a message is displayed with a list of reasons why,
e.g.

qemu-system-x86_64 -nographic  -smp 4 -m 4G -M pc,usb=on \
 -chardev null,id=n -device usb-serial,chardev=n \
 -virtfs local,path=/home,mount_tag=fs,security_model=none \
 -drive if=virtio,file=myimage.qcow2

(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

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 monitor/hmp-cmds.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index a48bc1e904..1f744f6041 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -224,6 +224,15 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 
     migration_global_dump(mon);
 
+    if (info->blocked) {
+        strList *reasons = info->blocked_reasons;
+        monitor_printf(mon, "Outgoing migration blocked:\n");
+        while (reasons) {
+            monitor_printf(mon, "  %s\n", reasons->value);
+            reasons = reasons->next;
+        }
+    }
+
     if (info->has_status) {
         monitor_printf(mon, "Migration status: %s",
                        MigrationStatus_str(info->status));
-- 
2.29.2

Re: [PATCH 2/2] migration: Display the migration blockers

[Eric Blake]

On 2/2/21 7:55 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Update 'info migrate' to display migration blocking information.
> If the outbound migration is not blocked, there is no change, however
> if it is blocked a message is displayed with a list of reasons why,
> e.g.
> 
> qemu-system-x86_64 -nographic  -smp 4 -m 4G -M pc,usb=on \
>  -chardev null,id=n -device usb-serial,chardev=n \
>  -virtfs local,path=/home,mount_tag=fs,security_model=none \
>  -drive if=virtio,file=myimage.qcow2
> 
> (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
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  monitor/hmp-cmds.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 

> +++ b/monitor/hmp-cmds.c
> @@ -224,6 +224,15 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>  
>      migration_global_dump(mon);
>  
> +    if (info->blocked) {

Would be slightly more accurate to gate on info->has_blocked_reasons (as
written, you are depending on the fact that patch 1 always sets blocked
and has_blocked_reasons to the same value); which again highlights the
redundancy of a bool duplicating the presence of the optional ['str']
member.  But since the code works as written,

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 2/2] migration: Display the migration blockers

[Peter Xu]

On Tue, Feb 02, 2021 at 01:55:22PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Update 'info migrate' to display migration blocking information.
> If the outbound migration is not blocked, there is no change, however
> if it is blocked a message is displayed with a list of reasons why,
> e.g.
> 
> qemu-system-x86_64 -nographic  -smp 4 -m 4G -M pc,usb=on \
>  -chardev null,id=n -device usb-serial,chardev=n \
>  -virtfs local,path=/home,mount_tag=fs,security_model=none \
>  -drive if=virtio,file=myimage.qcow2
> 
> (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
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH 0/2] migration blocker information

[Dr. David Alan Gilbert]

* Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Hi,
>   This is a rework of 'migration: Provide a test for migratability',
> based on Eric's feedback this is now an addition to the existing
> info/query migrate commands rather than a new command.
> 
>   One other importovement is that we now list all the reasons for the
> block rather than just one.
> 
> Dave

Queued

> 
> Dr. David Alan Gilbert (2):
>   migration: Add blocker information
>   migration: Display the migration blockers
> 
>  migration/migration.c | 25 +++++++++++++++++++++++--
>  migration/savevm.c    | 13 +++++++++++++
>  migration/savevm.h    |  1 +
>  monitor/hmp-cmds.c    |  9 +++++++++
>  qapi/migration.json   |  6 ++++++
>  5 files changed, 52 insertions(+), 2 deletions(-)
> 
> -- 
> 2.29.2
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK