All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-6.0] migration: Drop redundant query-migrate result @blocked
@ 2021-04-19 16:27 Markus Armbruster
  2021-04-19 16:32 ` Peter Maydell
  0 siblings, 1 reply; 4+ messages in thread
From: Markus Armbruster @ 2021-04-19 16:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, dgilbert, peterx

Result @blocked is true when and only when result @blocked-reasons is
present.  It's always non-empty when present.  @blocked is redundant;
drop.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/migration.json   |  7 +++----
 migration/migration.c | 29 +++++++++++++----------------
 monitor/hmp-cmds.c    |  2 +-
 3 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 9bf0bc4d25..7a5bdf9a0d 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -224,9 +224,9 @@
 #        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)
+# @blocked-reasons: A list of reasons an outgoing migration is blocked.
+#                   Present and non-empty when migration is blocked.
+#                   (since 6.0)
 #
 # Since: 0.14
 ##
@@ -241,7 +241,6 @@
            '*setup-time': 'int',
            '*cpu-throttle-percentage': 'int',
            '*error-desc': 'str',
-           'blocked': 'bool',
            '*blocked-reasons': ['str'],
            '*postcopy-blocktime' : 'uint32',
            '*postcopy-vcpu-blocktime': ['uint32'],
diff --git a/migration/migration.c b/migration/migration.c
index 8ca034136b..fdadee290e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1073,27 +1073,24 @@ static void populate_vfio_info(MigrationInfo *info)
 static void fill_source_migration_info(MigrationInfo *info)
 {
     MigrationState *s = migrate_get_current();
+    GSList *cur_blocker = migration_blockers;
 
-    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);
+    /*
+     * 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);
-        }
+    while (cur_blocker) {
+        QAPI_LIST_PREPEND(info->blocked_reasons,
+                          g_strdup(error_get_pretty(cur_blocker->data)));
+        cur_blocker = g_slist_next(cur_blocker);
     }
+    info->has_blocked_reasons = info->blocked_reasons != NULL;
 
     switch (s->state) {
     case MIGRATION_STATUS_NONE:
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 0ad5b77477..d9bef63373 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -224,7 +224,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 
     migration_global_dump(mon);
 
-    if (info->blocked) {
+    if (info->blocked_reasons) {
         strList *reasons = info->blocked_reasons;
         monitor_printf(mon, "Outgoing migration blocked:\n");
         while (reasons) {
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH for-6.0] migration: Drop redundant query-migrate result @blocked
  2021-04-19 16:27 [PATCH for-6.0] migration: Drop redundant query-migrate result @blocked Markus Armbruster
@ 2021-04-19 16:32 ` Peter Maydell
  2021-04-19 17:26   ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2021-04-19 16:32 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: QEMU Developers, Peter Xu, Dr. David Alan Gilbert

On Mon, 19 Apr 2021 at 17:27, Markus Armbruster <armbru@redhat.com> wrote:
>
> Result @blocked is true when and only when result @blocked-reasons is
> present.  It's always non-empty when present.  @blocked is redundant;
> drop.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

"for-6.0" needs to be accompanied by a justification of why it's
important to go in the release at this point...

thanks
-- PMM


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH for-6.0] migration: Drop redundant query-migrate result @blocked
  2021-04-19 16:32 ` Peter Maydell
@ 2021-04-19 17:26   ` Dr. David Alan Gilbert
  2021-04-20  4:27     ` Markus Armbruster
  0 siblings, 1 reply; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2021-04-19 17:26 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Markus Armbruster, Peter Xu, QEMU Developers

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Mon, 19 Apr 2021 at 17:27, Markus Armbruster <armbru@redhat.com> wrote:
> >
> > Result @blocked is true when and only when result @blocked-reasons is
> > present.  It's always non-empty when present.  @blocked is redundant;
> > drop.
> >
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>

So I'm OK with it in principal and I think the code is OK, so

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> "for-6.0" needs to be accompanied by a justification of why it's
> important to go in the release at this point...

I guess the argument is that when we hit 6.0 it becomes API and removing
the 'blocked' becomes a matter of deprecation which is a pain.

Hmm; I agree it's the right change, but I'm not sure I can justify it
this late in the release.

Dave

> thanks
> -- PMM
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH for-6.0] migration: Drop redundant query-migrate result @blocked
  2021-04-19 17:26   ` Dr. David Alan Gilbert
@ 2021-04-20  4:27     ` Markus Armbruster
  0 siblings, 0 replies; 4+ messages in thread
From: Markus Armbruster @ 2021-04-20  4:27 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: Peter Maydell, QEMU Developers, Peter Xu

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

> * Peter Maydell (peter.maydell@linaro.org) wrote:
>> On Mon, 19 Apr 2021 at 17:27, Markus Armbruster <armbru@redhat.com> wrote:
>> >
>> > Result @blocked is true when and only when result @blocked-reasons is
>> > present.  It's always non-empty when present.  @blocked is redundant;
>> > drop.
>> >
>> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> So I'm OK with it in principal and I think the code is OK, so
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
>> "for-6.0" needs to be accompanied by a justification of why it's
>> important to go in the release at this point...

You're right.  My bad.

> I guess the argument is that when we hit 6.0 it becomes API and removing
> the 'blocked' becomes a matter of deprecation which is a pain.

Correct.

> Hmm; I agree it's the right change, but I'm not sure I can justify it
> this late in the release.

If we decide taking it out is too late, we should at least deprecate it
in 6.0.  I'll post the patch, so you guys can pick the one you like
better.



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-04-20  4:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-19 16:27 [PATCH for-6.0] migration: Drop redundant query-migrate result @blocked Markus Armbruster
2021-04-19 16:32 ` Peter Maydell
2021-04-19 17:26   ` Dr. David Alan Gilbert
2021-04-20  4:27     ` Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.