[PATCH 1/2] migration: Add blocker information

Dr. David Alan Gilbert (git) posted 2 patches 5 years ago
Maintainers: Juan Quintela <quintela@redhat.com>, Markus Armbruster <armbru@redhat.com>, Eric Blake <eblake@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
[PATCH 1/2] migration: Add blocker information
Posted by Dr. David Alan Gilbert (git) 5 years ago
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
Posted by Eric Blake 5 years ago
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
Posted by Peter Xu 5 years ago
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