[PATCH 0/3] migration: Some fix and enhancements to HMP "info migrate"

[PATCH 0/3] migration: Some fix and enhancements to HMP "info migrate"

[Peter Xu]

Patch 1 was a bug I found set capabilities, so it's pretty separate issue.

Patch 2-3 was an attempt that I made the HMP "info migrate" looks slightly
easier to read.  For me, it was almost never able to show correctly on what
I cared before in the current screen but I'll always need to scroll, as
it's normally very long.

I made it pretty condensed here, the new one looks like this now:

(qemu) info migrate
Status: postcopy-active
Time (ms): total=40504, setup=14, down=145
RAM info:
  Bandwidth (mbps): 6102.65
  Sizes (KB): psize=4, total=16777992
    transferred=37673019, remain=2136404,
    precopy=3, multifd=26108780, postcopy=11563855
  Pages: normal=9394288, zero=600672, rate_per_sec=185875
  Others: dirty_syncs=3, dirty_pages_rate=278378, postcopy_req=4078

Comments welcomed, thanks.

Peter Xu (3):
  migration: Allow caps to be set when preempt or multifd cap enabled
  migration/hmp: Dump global in "info migrate_parameters" instead
  migration/hmp: Add "info migrate -a", reorg the dump

 migration/migration-hmp-cmds.c | 162 +++++++++++++++++----------------
 migration/options.c            |   4 +-
 hmp-commands-info.hx           |   6 +-
 3 files changed, 89 insertions(+), 83 deletions(-)

-- 
2.49.0

[PATCH 1/3] migration: Allow caps to be set when preempt or multifd cap enabled

[Peter Xu]

With commit 82137e6c8c ("migration: enforce multifd and postcopy preempt to
be set before incoming"), and if postcopy preempt / multifd is enabled, one
cannot setup any capability because these checks would always fail.

(qemu) migrate_set_capability xbzrle off
Error: Postcopy preempt must be set before incoming starts

To fix it, check existing cap and only raise an error if the specific cap
changed.

Fixes: 82137e6c8c ("migration: enforce multifd and postcopy preempt to be set before incoming")
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/options.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index 3fcd577cd7..162c72cda4 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -568,7 +568,7 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
             return false;
         }
 
-        if (migrate_incoming_started()) {
+        if (!migrate_postcopy_preempt() && migrate_incoming_started()) {
             error_setg(errp,
                        "Postcopy preempt must be set before incoming starts");
             return false;
@@ -576,7 +576,7 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
     }
 
     if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) {
-        if (migrate_incoming_started()) {
+        if (!migrate_multifd() && migrate_incoming_started()) {
             error_setg(errp, "Multifd must be set before incoming starts");
             return false;
         }
-- 
2.49.0

[PATCH 2/3] migration/hmp: Dump global in "info migrate_parameters" instead

[Peter Xu]

"info migrate" is the command people would frequently use to query
migration status.  We may not want it to dump global configurations because
dumping the same things over and over won't help.

The globals are just more suitable for a parameter dump instead.  Hence
move it over.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration-hmp-cmds.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 49c26daed3..0034dbe47f 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -58,8 +58,6 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 
     info = qmp_query_migrate(NULL);
 
-    migration_global_dump(mon);
-
     if (info->blocked_reasons) {
         strList *reasons = info->blocked_reasons;
         monitor_printf(mon, "Outgoing migration blocked:\n");
@@ -235,6 +233,8 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
 {
     MigrationParameters *params;
 
+    migration_global_dump(mon);
+
     params = qmp_query_migrate_parameters(NULL);
 
     if (params) {
-- 
2.49.0

[PATCH 3/3] migration/hmp: Add "info migrate -a", reorg the dump

[Peter Xu]

I did quite some changes to the output of "info migrate".

The general rule is:

  - Put important things at the top
  - Reuse a single line when things are very relevant, hence reducing lines
    needed to show the results
  - Remove almost useless ones (e.g. "normal_bytes", while we also have
    both "page size" and "normal" pages)
  - Regroup things, so that related fields will show together
  - etc.

Before this change, it looks like (one example of a completed case):

(qemu) info migrate
globals:
store-global-state: on
only-migratable: off
send-configuration: on
send-section-footer: on
send-switchover-start: on
clear-bitmap-shift: 18
Migration status: completed
total time: 122952 ms
downtime: 76 ms
setup: 15 ms
transferred ram: 130825923 kbytes
throughput: 8717.68 mbps
remaining ram: 0 kbytes
total ram: 16777992 kbytes
duplicate: 997263 pages
normal: 32622225 pages
normal bytes: 130488900 kbytes
dirty sync count: 10
page size: 4 kbytes
multifd bytes: 117134260 kbytes
pages-per-second: 169431
postcopy request count: 5835
precopy ram: 15 kbytes
postcopy ram: 13691151 kbytes

After this change, giving a few examples:

NORMAL PRECOPY:

(qemu) info migrate
Status: active
Time (ms): total=14292, setup=13, exp_down=12223
RAM info:
  Bandwidth (mbps): 9380.51
  Sizes (KB): psize=4, total=16777992
    transferred=15697718, remain=12383520,
    precopy=2, multifd=15697713, postcopy=0
  Pages: normal=3913877, zero=599981, rate_per_sec=286769
  Others: dirty_syncs=2, dirty_pages_rate=264552

XBZRLE:

(qemu) info migrate
Status: active
Time (ms): total=43973, setup=16, exp_down=75826
RAM info:
  Bandwidth (mbps): 1496.08
  Sizes (KB): psize=4, total=16777992
    transferred=15156743, remain=12877944,
    precopy=15156768, multifd=0, postcopy=0
  Pages: normal=3780458, zero=614029, rate_per_sec=45567
  Others: dirty_syncs=2, dirty_pages_rate=128624
XBZRLE: size=67108864, transferred=0, pages=0, miss=188451
  miss_rate=0.00, encode_rate=0.00, overflow=0

POSTCOPY:

(qemu) info migrate
Status: postcopy-active
Time (ms): total=40504, setup=14, down=145
RAM info:
  Bandwidth (mbps): 6102.65
  Sizes (KB): psize=4, total=16777992
    transferred=37673019, remain=2136404,
    precopy=3, multifd=26108780, postcopy=11563855
  Pages: normal=9394288, zero=600672, rate_per_sec=185875
  Others: dirty_syncs=3, dirty_pages_rate=278378, postcopy_req=4078

COMPLETED:

(qemu) info migrate
Status: completed
Time (ms): total=43708, setup=14, down=145
RAM info:
  Bandwidth (mbps): 7464.50
  Sizes (KB): psize=4, total=16777992
    transferred=39813725, remain=0,
    precopy=3, multifd=26108780, postcopy=13704436
  Pages: normal=9928390, zero=600672, rate_per_sec=167283
  Others: dirty_syncs=3, postcopy_req=5577

INCOMING (WHEN TCP LISTENING):

(qemu) info migrate
Status: setup
Sockets: [
        tcp:0.0.0.0:12345
]

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration-hmp-cmds.c | 158 +++++++++++++++++----------------
 hmp-commands-info.hx           |   6 +-
 2 files changed, 85 insertions(+), 79 deletions(-)

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 0034dbe47f..c1c10b22ae 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -54,6 +54,7 @@ static void migration_global_dump(Monitor *mon)
 
 void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 {
+    bool show_all = qdict_get_try_bool(qdict, "all", false);
     MigrationInfo *info;
 
     info = qmp_query_migrate(NULL);
@@ -68,7 +69,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
     }
 
     if (info->has_status) {
-        monitor_printf(mon, "Migration status: %s",
+        monitor_printf(mon, "Status: %s",
                        MigrationStatus_str(info->status));
         if (info->status == MIGRATION_STATUS_FAILED && info->error_desc) {
             monitor_printf(mon, " (%s)\n", info->error_desc);
@@ -76,90 +77,111 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
             monitor_printf(mon, "\n");
         }
 
-        monitor_printf(mon, "total time: %" PRIu64 " ms\n",
-                       info->total_time);
-        if (info->has_expected_downtime) {
-            monitor_printf(mon, "expected downtime: %" PRIu64 " ms\n",
-                           info->expected_downtime);
-        }
-        if (info->has_downtime) {
-            monitor_printf(mon, "downtime: %" PRIu64 " ms\n",
-                           info->downtime);
+        if (info->total_time) {
+            monitor_printf(mon, "Time (ms): total=%" PRIu64,
+                           info->total_time);
+            if (info->has_setup_time) {
+                monitor_printf(mon, ", setup=%" PRIu64,
+                               info->setup_time);
+            }
+            if (info->has_expected_downtime) {
+                monitor_printf(mon, ", exp_down=%" PRIu64,
+                               info->expected_downtime);
+            }
+            if (info->has_downtime) {
+                monitor_printf(mon, ", down=%" PRIu64,
+                               info->downtime);
+            }
+            monitor_printf(mon, "\n");
         }
-        if (info->has_setup_time) {
-            monitor_printf(mon, "setup: %" PRIu64 " ms\n",
-                           info->setup_time);
+    }
+
+    if (info->has_socket_address) {
+        SocketAddressList *addr;
+
+        monitor_printf(mon, "Sockets: [\n");
+
+        for (addr = info->socket_address; addr; addr = addr->next) {
+            char *s = socket_uri(addr->value);
+            monitor_printf(mon, "\t%s\n", s);
+            g_free(s);
         }
+        monitor_printf(mon, "]\n");
     }
 
     if (info->ram) {
-        monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n",
-                       info->ram->transferred >> 10);
-        monitor_printf(mon, "throughput: %0.2f mbps\n",
+        monitor_printf(mon, "RAM info:\n");
+        monitor_printf(mon, "  Bandwidth (mbps): %0.2f\n",
                        info->ram->mbps);
-        monitor_printf(mon, "remaining ram: %" PRIu64 " kbytes\n",
-                       info->ram->remaining >> 10);
-        monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n",
+        monitor_printf(mon, "  Sizes (KB): psize=%" PRIu64
+                       ", total=%" PRIu64 "\n",
+                       info->ram->page_size >> 10,
                        info->ram->total >> 10);
-        monitor_printf(mon, "duplicate: %" PRIu64 " pages\n",
-                       info->ram->duplicate);
-        monitor_printf(mon, "normal: %" PRIu64 " pages\n",
-                       info->ram->normal);
-        monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
-                       info->ram->normal_bytes >> 10);
-        monitor_printf(mon, "dirty sync count: %" PRIu64 "\n",
-                       info->ram->dirty_sync_count);
-        monitor_printf(mon, "page size: %" PRIu64 " kbytes\n",
-                       info->ram->page_size >> 10);
-        monitor_printf(mon, "multifd bytes: %" PRIu64 " kbytes\n",
-                       info->ram->multifd_bytes >> 10);
-        monitor_printf(mon, "pages-per-second: %" PRIu64 "\n",
+        monitor_printf(mon, "    transferred=%" PRIu64
+                       ", remain=%" PRIu64 ",\n",
+                       info->ram->transferred >> 10,
+                       info->ram->remaining >> 10);
+        monitor_printf(mon, "    precopy=%" PRIu64
+                       ", multifd=%" PRIu64
+                       ", postcopy=%" PRIu64,
+                       info->ram->precopy_bytes >> 10,
+                       info->ram->multifd_bytes >> 10,
+                       info->ram->postcopy_bytes >> 10);
+
+        if (info->vfio) {
+            monitor_printf(mon, ", vfio=%" PRIu64,
+                           info->vfio->transferred >> 10);
+        }
+        monitor_printf(mon, "\n");
+
+        monitor_printf(mon, "  Pages: normal=%" PRIu64 ", zero=%" PRIu64
+                       ", rate_per_sec=%" PRIu64 "\n",
+                       info->ram->normal,
+                       info->ram->duplicate,
                        info->ram->pages_per_second);
+        monitor_printf(mon, "  Others: dirty_syncs=%" PRIu64,
+                       info->ram->dirty_sync_count);
 
         if (info->ram->dirty_pages_rate) {
-            monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n",
+            monitor_printf(mon, ", dirty_pages_rate=%" PRIu64,
                            info->ram->dirty_pages_rate);
         }
         if (info->ram->postcopy_requests) {
-            monitor_printf(mon, "postcopy request count: %" PRIu64 "\n",
+            monitor_printf(mon, ", postcopy_req=%" PRIu64,
                            info->ram->postcopy_requests);
         }
-        if (info->ram->precopy_bytes) {
-            monitor_printf(mon, "precopy ram: %" PRIu64 " kbytes\n",
-                           info->ram->precopy_bytes >> 10);
-        }
         if (info->ram->downtime_bytes) {
-            monitor_printf(mon, "downtime ram: %" PRIu64 " kbytes\n",
-                           info->ram->downtime_bytes >> 10);
-        }
-        if (info->ram->postcopy_bytes) {
-            monitor_printf(mon, "postcopy ram: %" PRIu64 " kbytes\n",
-                           info->ram->postcopy_bytes >> 10);
+            monitor_printf(mon, ", downtime_ram=%" PRIu64,
+                           info->ram->downtime_bytes);
         }
         if (info->ram->dirty_sync_missed_zero_copy) {
-            monitor_printf(mon,
-                           "Zero-copy-send fallbacks happened: %" PRIu64 " times\n",
+            monitor_printf(mon, ", zerocopy_fallbacks=%" PRIu64,
                            info->ram->dirty_sync_missed_zero_copy);
         }
+        monitor_printf(mon, "\n");
     }
 
     if (info->xbzrle_cache) {
-        monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
-                       info->xbzrle_cache->cache_size);
-        monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
-                       info->xbzrle_cache->bytes >> 10);
-        monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
-                       info->xbzrle_cache->pages);
-        monitor_printf(mon, "xbzrle cache miss: %" PRIu64 " pages\n",
-                       info->xbzrle_cache->cache_miss);
-        monitor_printf(mon, "xbzrle cache miss rate: %0.2f\n",
-                       info->xbzrle_cache->cache_miss_rate);
-        monitor_printf(mon, "xbzrle encoding rate: %0.2f\n",
-                       info->xbzrle_cache->encoding_rate);
-        monitor_printf(mon, "xbzrle overflow: %" PRIu64 "\n",
+        monitor_printf(mon, "XBZRLE: size=%" PRIu64
+                       ", transferred=%" PRIu64
+                       ", pages=%" PRIu64
+                       ", miss=%" PRIu64 "\n"
+                       "  miss_rate=%0.2f"
+                       ", encode_rate=%0.2f"
+                       ", overflow=%" PRIu64 "\n",
+                       info->xbzrle_cache->cache_size,
+                       info->xbzrle_cache->bytes,
+                       info->xbzrle_cache->pages,
+                       info->xbzrle_cache->cache_miss,
+                       info->xbzrle_cache->cache_miss_rate,
+                       info->xbzrle_cache->encoding_rate,
                        info->xbzrle_cache->overflow);
     }
 
+    if (!show_all) {
+        goto out;
+    }
+
     if (info->has_cpu_throttle_percentage) {
         monitor_printf(mon, "cpu throttle percentage: %" PRIu64 "\n",
                        info->cpu_throttle_percentage);
@@ -191,24 +213,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
         g_free(str);
         visit_free(v);
     }
-    if (info->has_socket_address) {
-        SocketAddressList *addr;
-
-        monitor_printf(mon, "socket address: [\n");
-
-        for (addr = info->socket_address; addr; addr = addr->next) {
-            char *s = socket_uri(addr->value);
-            monitor_printf(mon, "\t%s\n", s);
-            g_free(s);
-        }
-        monitor_printf(mon, "]\n");
-    }
-
-    if (info->vfio) {
-        monitor_printf(mon, "vfio device transferred: %" PRIu64 " kbytes\n",
-                       info->vfio->transferred >> 10);
-    }
 
+out:
     qapi_free_MigrationInfo(info);
 }
 
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index c59cd6637b..639a450ee5 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -475,9 +475,9 @@ ERST
 
     {
         .name       = "migrate",
-        .args_type  = "",
-        .params     = "",
-        .help       = "show migration status",
+        .args_type  = "all:-a",
+        .params     = "[-a]",
+        .help       = "show migration status (-a: all, dump all status)",
         .cmd        = hmp_info_migrate,
     },
 
-- 
2.49.0

Re: [PATCH 1/3] migration: Allow caps to be set when preempt or multifd cap enabled

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> With commit 82137e6c8c ("migration: enforce multifd and postcopy preempt to
> be set before incoming"), and if postcopy preempt / multifd is enabled, one
> cannot setup any capability because these checks would always fail.
> 
> (qemu) migrate_set_capability xbzrle off
> Error: Postcopy preempt must be set before incoming starts
> 
> To fix it, check existing cap and only raise an error if the specific cap
> changed.
> 
> Fixes: 82137e6c8c ("migration: enforce multifd and postcopy preempt to be set before incoming")
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>

> ---
>  migration/options.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/options.c b/migration/options.c
> index 3fcd577cd7..162c72cda4 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -568,7 +568,7 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
>              return false;
>          }
>  
> -        if (migrate_incoming_started()) {
> +        if (!migrate_postcopy_preempt() && migrate_incoming_started()) {
>              error_setg(errp,
>                         "Postcopy preempt must be set before incoming starts");
>              return false;
> @@ -576,7 +576,7 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
>      }
>  
>      if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) {
> -        if (migrate_incoming_started()) {
> +        if (!migrate_multifd() && migrate_incoming_started()) {
>              error_setg(errp, "Multifd must be set before incoming starts");
>              return false;
>          }
> -- 
> 2.49.0
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 3/3] migration/hmp: Add "info migrate -a", reorg the dump

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> I did quite some changes to the output of "info migrate".
> 
> The general rule is:
> 
>   - Put important things at the top
>   - Reuse a single line when things are very relevant, hence reducing lines
>     needed to show the results
>   - Remove almost useless ones (e.g. "normal_bytes", while we also have
>     both "page size" and "normal" pages)
>   - Regroup things, so that related fields will show together
>   - etc.
> 
> Before this change, it looks like (one example of a completed case):

The changelog should probably also show the -a difference.

Also a couple of minor ones lower down in the changelog...

> (qemu) info migrate
> globals:
> store-global-state: on
> only-migratable: off
> send-configuration: on
> send-section-footer: on
> send-switchover-start: on
> clear-bitmap-shift: 18
> Migration status: completed
> total time: 122952 ms
> downtime: 76 ms
> setup: 15 ms
> transferred ram: 130825923 kbytes
> throughput: 8717.68 mbps
> remaining ram: 0 kbytes
> total ram: 16777992 kbytes
> duplicate: 997263 pages
> normal: 32622225 pages
> normal bytes: 130488900 kbytes
> dirty sync count: 10
> page size: 4 kbytes
> multifd bytes: 117134260 kbytes
> pages-per-second: 169431
> postcopy request count: 5835
> precopy ram: 15 kbytes
> postcopy ram: 13691151 kbytes
> 
> After this change, giving a few examples:
> 
> NORMAL PRECOPY:
> 
> (qemu) info migrate
> Status: active
> Time (ms): total=14292, setup=13, exp_down=12223
> RAM info:
>   Bandwidth (mbps): 9380.51

Now lets see, I think that is actually (MB/s) - i.e.
decimal megabytes;
        s->mbps = ((double) bytes * 8.0) / transfer_time / 1000;

>   Sizes (KB): psize=4, total=16777992

and I think that is actually (KiB) i.e. 2^10 bytes

Other than those,

Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>

>     transferred=15697718, remain=12383520,
>     precopy=2, multifd=15697713, postcopy=0
>   Pages: normal=3913877, zero=599981, rate_per_sec=286769
>   Others: dirty_syncs=2, dirty_pages_rate=264552
> 
> XBZRLE:
> 
> (qemu) info migrate
> Status: active
> Time (ms): total=43973, setup=16, exp_down=75826
> RAM info:
>   Bandwidth (mbps): 1496.08
>   Sizes (KB): psize=4, total=16777992
>     transferred=15156743, remain=12877944,
>     precopy=15156768, multifd=0, postcopy=0
>   Pages: normal=3780458, zero=614029, rate_per_sec=45567
>   Others: dirty_syncs=2, dirty_pages_rate=128624
> XBZRLE: size=67108864, transferred=0, pages=0, miss=188451
>   miss_rate=0.00, encode_rate=0.00, overflow=0
> 
> POSTCOPY:
> 
> (qemu) info migrate
> Status: postcopy-active
> Time (ms): total=40504, setup=14, down=145
> RAM info:
>   Bandwidth (mbps): 6102.65
>   Sizes (KB): psize=4, total=16777992
>     transferred=37673019, remain=2136404,
>     precopy=3, multifd=26108780, postcopy=11563855
>   Pages: normal=9394288, zero=600672, rate_per_sec=185875
>   Others: dirty_syncs=3, dirty_pages_rate=278378, postcopy_req=4078
> 
> COMPLETED:
> 
> (qemu) info migrate
> Status: completed
> Time (ms): total=43708, setup=14, down=145
> RAM info:
>   Bandwidth (mbps): 7464.50
>   Sizes (KB): psize=4, total=16777992
>     transferred=39813725, remain=0,
>     precopy=3, multifd=26108780, postcopy=13704436
>   Pages: normal=9928390, zero=600672, rate_per_sec=167283
>   Others: dirty_syncs=3, postcopy_req=5577
> 
> INCOMING (WHEN TCP LISTENING):
> 
> (qemu) info migrate
> Status: setup
> Sockets: [
>         tcp:0.0.0.0:12345
> ]
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration-hmp-cmds.c | 158 +++++++++++++++++----------------
>  hmp-commands-info.hx           |   6 +-
>  2 files changed, 85 insertions(+), 79 deletions(-)
> 
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 0034dbe47f..c1c10b22ae 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -54,6 +54,7 @@ static void migration_global_dump(Monitor *mon)
>  
>  void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>  {
> +    bool show_all = qdict_get_try_bool(qdict, "all", false);
>      MigrationInfo *info;
>  
>      info = qmp_query_migrate(NULL);
> @@ -68,7 +69,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>      }
>  
>      if (info->has_status) {
> -        monitor_printf(mon, "Migration status: %s",
> +        monitor_printf(mon, "Status: %s",
>                         MigrationStatus_str(info->status));
>          if (info->status == MIGRATION_STATUS_FAILED && info->error_desc) {
>              monitor_printf(mon, " (%s)\n", info->error_desc);
> @@ -76,90 +77,111 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>              monitor_printf(mon, "\n");
>          }
>  
> -        monitor_printf(mon, "total time: %" PRIu64 " ms\n",
> -                       info->total_time);
> -        if (info->has_expected_downtime) {
> -            monitor_printf(mon, "expected downtime: %" PRIu64 " ms\n",
> -                           info->expected_downtime);
> -        }
> -        if (info->has_downtime) {
> -            monitor_printf(mon, "downtime: %" PRIu64 " ms\n",
> -                           info->downtime);
> +        if (info->total_time) {
> +            monitor_printf(mon, "Time (ms): total=%" PRIu64,
> +                           info->total_time);
> +            if (info->has_setup_time) {
> +                monitor_printf(mon, ", setup=%" PRIu64,
> +                               info->setup_time);
> +            }
> +            if (info->has_expected_downtime) {
> +                monitor_printf(mon, ", exp_down=%" PRIu64,
> +                               info->expected_downtime);
> +            }
> +            if (info->has_downtime) {
> +                monitor_printf(mon, ", down=%" PRIu64,
> +                               info->downtime);
> +            }
> +            monitor_printf(mon, "\n");
>          }
> -        if (info->has_setup_time) {
> -            monitor_printf(mon, "setup: %" PRIu64 " ms\n",
> -                           info->setup_time);
> +    }
> +
> +    if (info->has_socket_address) {
> +        SocketAddressList *addr;
> +
> +        monitor_printf(mon, "Sockets: [\n");
> +
> +        for (addr = info->socket_address; addr; addr = addr->next) {
> +            char *s = socket_uri(addr->value);
> +            monitor_printf(mon, "\t%s\n", s);
> +            g_free(s);
>          }
> +        monitor_printf(mon, "]\n");
>      }
>  
>      if (info->ram) {
> -        monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n",
> -                       info->ram->transferred >> 10);
> -        monitor_printf(mon, "throughput: %0.2f mbps\n",
> +        monitor_printf(mon, "RAM info:\n");
> +        monitor_printf(mon, "  Bandwidth (mbps): %0.2f\n",
>                         info->ram->mbps);
> -        monitor_printf(mon, "remaining ram: %" PRIu64 " kbytes\n",
> -                       info->ram->remaining >> 10);
> -        monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n",
> +        monitor_printf(mon, "  Sizes (KB): psize=%" PRIu64
> +                       ", total=%" PRIu64 "\n",
> +                       info->ram->page_size >> 10,
>                         info->ram->total >> 10);
> -        monitor_printf(mon, "duplicate: %" PRIu64 " pages\n",
> -                       info->ram->duplicate);
> -        monitor_printf(mon, "normal: %" PRIu64 " pages\n",
> -                       info->ram->normal);
> -        monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
> -                       info->ram->normal_bytes >> 10);
> -        monitor_printf(mon, "dirty sync count: %" PRIu64 "\n",
> -                       info->ram->dirty_sync_count);
> -        monitor_printf(mon, "page size: %" PRIu64 " kbytes\n",
> -                       info->ram->page_size >> 10);
> -        monitor_printf(mon, "multifd bytes: %" PRIu64 " kbytes\n",
> -                       info->ram->multifd_bytes >> 10);
> -        monitor_printf(mon, "pages-per-second: %" PRIu64 "\n",
> +        monitor_printf(mon, "    transferred=%" PRIu64
> +                       ", remain=%" PRIu64 ",\n",
> +                       info->ram->transferred >> 10,
> +                       info->ram->remaining >> 10);
> +        monitor_printf(mon, "    precopy=%" PRIu64
> +                       ", multifd=%" PRIu64
> +                       ", postcopy=%" PRIu64,
> +                       info->ram->precopy_bytes >> 10,
> +                       info->ram->multifd_bytes >> 10,
> +                       info->ram->postcopy_bytes >> 10);
> +
> +        if (info->vfio) {
> +            monitor_printf(mon, ", vfio=%" PRIu64,
> +                           info->vfio->transferred >> 10);
> +        }
> +        monitor_printf(mon, "\n");
> +
> +        monitor_printf(mon, "  Pages: normal=%" PRIu64 ", zero=%" PRIu64
> +                       ", rate_per_sec=%" PRIu64 "\n",
> +                       info->ram->normal,
> +                       info->ram->duplicate,
>                         info->ram->pages_per_second);
> +        monitor_printf(mon, "  Others: dirty_syncs=%" PRIu64,
> +                       info->ram->dirty_sync_count);
>  
>          if (info->ram->dirty_pages_rate) {
> -            monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n",
> +            monitor_printf(mon, ", dirty_pages_rate=%" PRIu64,
>                             info->ram->dirty_pages_rate);
>          }
>          if (info->ram->postcopy_requests) {
> -            monitor_printf(mon, "postcopy request count: %" PRIu64 "\n",
> +            monitor_printf(mon, ", postcopy_req=%" PRIu64,
>                             info->ram->postcopy_requests);
>          }
> -        if (info->ram->precopy_bytes) {
> -            monitor_printf(mon, "precopy ram: %" PRIu64 " kbytes\n",
> -                           info->ram->precopy_bytes >> 10);
> -        }
>          if (info->ram->downtime_bytes) {
> -            monitor_printf(mon, "downtime ram: %" PRIu64 " kbytes\n",
> -                           info->ram->downtime_bytes >> 10);
> -        }
> -        if (info->ram->postcopy_bytes) {
> -            monitor_printf(mon, "postcopy ram: %" PRIu64 " kbytes\n",
> -                           info->ram->postcopy_bytes >> 10);
> +            monitor_printf(mon, ", downtime_ram=%" PRIu64,
> +                           info->ram->downtime_bytes);
>          }
>          if (info->ram->dirty_sync_missed_zero_copy) {
> -            monitor_printf(mon,
> -                           "Zero-copy-send fallbacks happened: %" PRIu64 " times\n",
> +            monitor_printf(mon, ", zerocopy_fallbacks=%" PRIu64,
>                             info->ram->dirty_sync_missed_zero_copy);
>          }
> +        monitor_printf(mon, "\n");
>      }
>  
>      if (info->xbzrle_cache) {
> -        monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
> -                       info->xbzrle_cache->cache_size);
> -        monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
> -                       info->xbzrle_cache->bytes >> 10);
> -        monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
> -                       info->xbzrle_cache->pages);
> -        monitor_printf(mon, "xbzrle cache miss: %" PRIu64 " pages\n",
> -                       info->xbzrle_cache->cache_miss);
> -        monitor_printf(mon, "xbzrle cache miss rate: %0.2f\n",
> -                       info->xbzrle_cache->cache_miss_rate);
> -        monitor_printf(mon, "xbzrle encoding rate: %0.2f\n",
> -                       info->xbzrle_cache->encoding_rate);
> -        monitor_printf(mon, "xbzrle overflow: %" PRIu64 "\n",
> +        monitor_printf(mon, "XBZRLE: size=%" PRIu64
> +                       ", transferred=%" PRIu64
> +                       ", pages=%" PRIu64
> +                       ", miss=%" PRIu64 "\n"
> +                       "  miss_rate=%0.2f"
> +                       ", encode_rate=%0.2f"
> +                       ", overflow=%" PRIu64 "\n",
> +                       info->xbzrle_cache->cache_size,
> +                       info->xbzrle_cache->bytes,
> +                       info->xbzrle_cache->pages,
> +                       info->xbzrle_cache->cache_miss,
> +                       info->xbzrle_cache->cache_miss_rate,
> +                       info->xbzrle_cache->encoding_rate,
>                         info->xbzrle_cache->overflow);
>      }
>  
> +    if (!show_all) {
> +        goto out;
> +    }
> +
>      if (info->has_cpu_throttle_percentage) {
>          monitor_printf(mon, "cpu throttle percentage: %" PRIu64 "\n",
>                         info->cpu_throttle_percentage);
> @@ -191,24 +213,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>          g_free(str);
>          visit_free(v);
>      }
> -    if (info->has_socket_address) {
> -        SocketAddressList *addr;
> -
> -        monitor_printf(mon, "socket address: [\n");
> -
> -        for (addr = info->socket_address; addr; addr = addr->next) {
> -            char *s = socket_uri(addr->value);
> -            monitor_printf(mon, "\t%s\n", s);
> -            g_free(s);
> -        }
> -        monitor_printf(mon, "]\n");
> -    }
> -
> -    if (info->vfio) {
> -        monitor_printf(mon, "vfio device transferred: %" PRIu64 " kbytes\n",
> -                       info->vfio->transferred >> 10);
> -    }
>  
> +out:
>      qapi_free_MigrationInfo(info);
>  }
>  
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index c59cd6637b..639a450ee5 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -475,9 +475,9 @@ ERST
>  
>      {
>          .name       = "migrate",
> -        .args_type  = "",
> -        .params     = "",
> -        .help       = "show migration status",
> +        .args_type  = "all:-a",
> +        .params     = "[-a]",
> +        .help       = "show migration status (-a: all, dump all status)",
>          .cmd        = hmp_info_migrate,
>      },
>  
> -- 
> 2.49.0
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 2/3] migration/hmp: Dump global in "info migrate_parameters" instead

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> "info migrate" is the command people would frequently use to query
> migration status.  We may not want it to dump global configurations because
> dumping the same things over and over won't help.
> 
> The globals are just more suitable for a parameter dump instead.  Hence
> move it over.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

I think this is a *little* odd, since 'info migrate_parameters'
is a list of the things you can set with 'migrate_set_parameters'
Perhaps it would be better to add it under the 'info migrate -a' option
you add in the next patch?

However, one other thing, the globals stuff prints a
'globals:'  at the start,  but doesn't print anything at the end,
so if you make this change, you end up with out being able to tell
where the globals end and the parameters start, so maybe a
'parameters:' after it?

Dave

> ---
>  migration/migration-hmp-cmds.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 49c26daed3..0034dbe47f 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -58,8 +58,6 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>  
>      info = qmp_query_migrate(NULL);
>  
> -    migration_global_dump(mon);
> -
>      if (info->blocked_reasons) {
>          strList *reasons = info->blocked_reasons;
>          monitor_printf(mon, "Outgoing migration blocked:\n");
> @@ -235,6 +233,8 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
>  {
>      MigrationParameters *params;
>  
> +    migration_global_dump(mon);
> +
>      params = qmp_query_migrate_parameters(NULL);
>  
>      if (params) {
> -- 
> 2.49.0
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 1/3] migration: Allow caps to be set when preempt or multifd cap enabled

[Juraj Marcin]

On 2025-05-13 18:09, Peter Xu wrote:
> With commit 82137e6c8c ("migration: enforce multifd and postcopy preempt to
> be set before incoming"), and if postcopy preempt / multifd is enabled, one
> cannot setup any capability because these checks would always fail.
> 
> (qemu) migrate_set_capability xbzrle off
> Error: Postcopy preempt must be set before incoming starts
> 
> To fix it, check existing cap and only raise an error if the specific cap
> changed.
> 
> Fixes: 82137e6c8c ("migration: enforce multifd and postcopy preempt to be set before incoming")
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/options.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Juraj Marcin <jmarcin@redhat.com>

Re: [PATCH 2/3] migration/hmp: Dump global in "info migrate_parameters" instead

[Juraj Marcin]

On 2025-05-13 18:09, Peter Xu wrote:
> "info migrate" is the command people would frequently use to query
> migration status.  We may not want it to dump global configurations because
> dumping the same things over and over won't help.
> 
> The globals are just more suitable for a parameter dump instead.  Hence
> move it over.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration-hmp-cmds.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Juraj Marcin <jmarcin@redhat.com>

Re: [PATCH 3/3] migration/hmp: Add "info migrate -a", reorg the dump

[Juraj Marcin]

Hi Peter

On 2025-05-13 18:09, Peter Xu wrote:
> I did quite some changes to the output of "info migrate".
> 
> The general rule is:
> 
>   - Put important things at the top
>   - Reuse a single line when things are very relevant, hence reducing lines
>     needed to show the results
>   - Remove almost useless ones (e.g. "normal_bytes", while we also have
>     both "page size" and "normal" pages)
>   - Regroup things, so that related fields will show together
>   - etc.
> 
> Before this change, it looks like (one example of a completed case):
> 
> (qemu) info migrate
> globals:
> store-global-state: on
> only-migratable: off
> send-configuration: on
> send-section-footer: on
> send-switchover-start: on
> clear-bitmap-shift: 18
> Migration status: completed
> total time: 122952 ms
> downtime: 76 ms
> setup: 15 ms
> transferred ram: 130825923 kbytes
> throughput: 8717.68 mbps
> remaining ram: 0 kbytes
> total ram: 16777992 kbytes
> duplicate: 997263 pages
> normal: 32622225 pages
> normal bytes: 130488900 kbytes
> dirty sync count: 10
> page size: 4 kbytes
> multifd bytes: 117134260 kbytes
> pages-per-second: 169431
> postcopy request count: 5835
> precopy ram: 15 kbytes
> postcopy ram: 13691151 kbytes
> 
> After this change, giving a few examples:
> 
> NORMAL PRECOPY:
> 
> (qemu) info migrate
> Status: active
> Time (ms): total=14292, setup=13, exp_down=12223
> RAM info:
>   Bandwidth (mbps): 9380.51
>   Sizes (KB): psize=4, total=16777992
>     transferred=15697718, remain=12383520,
>     precopy=2, multifd=15697713, postcopy=0
>   Pages: normal=3913877, zero=599981, rate_per_sec=286769
>   Others: dirty_syncs=2, dirty_pages_rate=264552
> 
> XBZRLE:
> 
> (qemu) info migrate
> Status: active
> Time (ms): total=43973, setup=16, exp_down=75826
> RAM info:
>   Bandwidth (mbps): 1496.08
>   Sizes (KB): psize=4, total=16777992
>     transferred=15156743, remain=12877944,
>     precopy=15156768, multifd=0, postcopy=0
>   Pages: normal=3780458, zero=614029, rate_per_sec=45567
>   Others: dirty_syncs=2, dirty_pages_rate=128624
> XBZRLE: size=67108864, transferred=0, pages=0, miss=188451
>   miss_rate=0.00, encode_rate=0.00, overflow=0
> 
> POSTCOPY:
> 
> (qemu) info migrate
> Status: postcopy-active
> Time (ms): total=40504, setup=14, down=145
> RAM info:
>   Bandwidth (mbps): 6102.65
>   Sizes (KB): psize=4, total=16777992
>     transferred=37673019, remain=2136404,
>     precopy=3, multifd=26108780, postcopy=11563855
>   Pages: normal=9394288, zero=600672, rate_per_sec=185875
>   Others: dirty_syncs=3, dirty_pages_rate=278378, postcopy_req=4078
> 
> COMPLETED:
> 
> (qemu) info migrate
> Status: completed
> Time (ms): total=43708, setup=14, down=145
> RAM info:
>   Bandwidth (mbps): 7464.50
>   Sizes (KB): psize=4, total=16777992
>     transferred=39813725, remain=0,
>     precopy=3, multifd=26108780, postcopy=13704436
>   Pages: normal=9928390, zero=600672, rate_per_sec=167283
>   Others: dirty_syncs=3, postcopy_req=5577
> 
> INCOMING (WHEN TCP LISTENING):
> 
> (qemu) info migrate
> Status: setup
> Sockets: [
>         tcp:0.0.0.0:12345
> ]
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration-hmp-cmds.c | 158 +++++++++++++++++----------------
>  hmp-commands-info.hx           |   6 +-
>  2 files changed, 85 insertions(+), 79 deletions(-)
> 
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 0034dbe47f..c1c10b22ae 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -54,6 +54,7 @@ static void migration_global_dump(Monitor *mon)
>  
>  void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>  {
> +    bool show_all = qdict_get_try_bool(qdict, "all", false);
>      MigrationInfo *info;
>  
>      info = qmp_query_migrate(NULL);
> @@ -68,7 +69,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>      }
>  
>      if (info->has_status) {
> -        monitor_printf(mon, "Migration status: %s",
> +        monitor_printf(mon, "Status: %s",
>                         MigrationStatus_str(info->status));
>          if (info->status == MIGRATION_STATUS_FAILED && info->error_desc) {
>              monitor_printf(mon, " (%s)\n", info->error_desc);
> @@ -76,90 +77,111 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>              monitor_printf(mon, "\n");
>          }
>  
> -        monitor_printf(mon, "total time: %" PRIu64 " ms\n",
> -                       info->total_time);
> -        if (info->has_expected_downtime) {
> -            monitor_printf(mon, "expected downtime: %" PRIu64 " ms\n",
> -                           info->expected_downtime);
> -        }
> -        if (info->has_downtime) {
> -            monitor_printf(mon, "downtime: %" PRIu64 " ms\n",
> -                           info->downtime);
> +        if (info->total_time) {
> +            monitor_printf(mon, "Time (ms): total=%" PRIu64,
> +                           info->total_time);
> +            if (info->has_setup_time) {
> +                monitor_printf(mon, ", setup=%" PRIu64,
> +                               info->setup_time);
> +            }
> +            if (info->has_expected_downtime) {
> +                monitor_printf(mon, ", exp_down=%" PRIu64,
> +                               info->expected_downtime);
> +            }
> +            if (info->has_downtime) {
> +                monitor_printf(mon, ", down=%" PRIu64,
> +                               info->downtime);
> +            }
> +            monitor_printf(mon, "\n");
>          }
> -        if (info->has_setup_time) {
> -            monitor_printf(mon, "setup: %" PRIu64 " ms\n",
> -                           info->setup_time);
> +    }
> +
> +    if (info->has_socket_address) {
> +        SocketAddressList *addr;
> +
> +        monitor_printf(mon, "Sockets: [\n");
> +
> +        for (addr = info->socket_address; addr; addr = addr->next) {
> +            char *s = socket_uri(addr->value);
> +            monitor_printf(mon, "\t%s\n", s);
> +            g_free(s);
>          }
> +        monitor_printf(mon, "]\n");
>      }
>  
>      if (info->ram) {
> -        monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n",
> -                       info->ram->transferred >> 10);
> -        monitor_printf(mon, "throughput: %0.2f mbps\n",
> +        monitor_printf(mon, "RAM info:\n");
> +        monitor_printf(mon, "  Bandwidth (mbps): %0.2f\n",
>                         info->ram->mbps);

I think the previous name (throughput) was better suited for this
metric. IIUC '->mbps' is the actual amount of data that has been sent
over a period of time, which is exactly the definition of throughput.
Bandwidth, on the other hand, is more of a (theoretical) maximum that
could be sent.

Otherwise, it looks good to me.

Best regards

Juraj Marcin

> -        monitor_printf(mon, "remaining ram: %" PRIu64 " kbytes\n",
> -                       info->ram->remaining >> 10);
> -        monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n",
> +        monitor_printf(mon, "  Sizes (KB): psize=%" PRIu64
> +                       ", total=%" PRIu64 "\n",
> +                       info->ram->page_size >> 10,
>                         info->ram->total >> 10);
> -        monitor_printf(mon, "duplicate: %" PRIu64 " pages\n",
> -                       info->ram->duplicate);
> -        monitor_printf(mon, "normal: %" PRIu64 " pages\n",
> -                       info->ram->normal);
> -        monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
> -                       info->ram->normal_bytes >> 10);
> -        monitor_printf(mon, "dirty sync count: %" PRIu64 "\n",
> -                       info->ram->dirty_sync_count);
> -        monitor_printf(mon, "page size: %" PRIu64 " kbytes\n",
> -                       info->ram->page_size >> 10);
> -        monitor_printf(mon, "multifd bytes: %" PRIu64 " kbytes\n",
> -                       info->ram->multifd_bytes >> 10);
> -        monitor_printf(mon, "pages-per-second: %" PRIu64 "\n",
> +        monitor_printf(mon, "    transferred=%" PRIu64
> +                       ", remain=%" PRIu64 ",\n",
> +                       info->ram->transferred >> 10,
> +                       info->ram->remaining >> 10);
> +        monitor_printf(mon, "    precopy=%" PRIu64
> +                       ", multifd=%" PRIu64
> +                       ", postcopy=%" PRIu64,
> +                       info->ram->precopy_bytes >> 10,
> +                       info->ram->multifd_bytes >> 10,
> +                       info->ram->postcopy_bytes >> 10);
> +
> +        if (info->vfio) {
> +            monitor_printf(mon, ", vfio=%" PRIu64,
> +                           info->vfio->transferred >> 10);
> +        }
> +        monitor_printf(mon, "\n");
> +
> +        monitor_printf(mon, "  Pages: normal=%" PRIu64 ", zero=%" PRIu64
> +                       ", rate_per_sec=%" PRIu64 "\n",
> +                       info->ram->normal,
> +                       info->ram->duplicate,
>                         info->ram->pages_per_second);
> +        monitor_printf(mon, "  Others: dirty_syncs=%" PRIu64,
> +                       info->ram->dirty_sync_count);
>  
>          if (info->ram->dirty_pages_rate) {
> -            monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n",
> +            monitor_printf(mon, ", dirty_pages_rate=%" PRIu64,
>                             info->ram->dirty_pages_rate);
>          }
>          if (info->ram->postcopy_requests) {
> -            monitor_printf(mon, "postcopy request count: %" PRIu64 "\n",
> +            monitor_printf(mon, ", postcopy_req=%" PRIu64,
>                             info->ram->postcopy_requests);
>          }
> -        if (info->ram->precopy_bytes) {
> -            monitor_printf(mon, "precopy ram: %" PRIu64 " kbytes\n",
> -                           info->ram->precopy_bytes >> 10);
> -        }
>          if (info->ram->downtime_bytes) {
> -            monitor_printf(mon, "downtime ram: %" PRIu64 " kbytes\n",
> -                           info->ram->downtime_bytes >> 10);
> -        }
> -        if (info->ram->postcopy_bytes) {
> -            monitor_printf(mon, "postcopy ram: %" PRIu64 " kbytes\n",
> -                           info->ram->postcopy_bytes >> 10);
> +            monitor_printf(mon, ", downtime_ram=%" PRIu64,
> +                           info->ram->downtime_bytes);
>          }
>          if (info->ram->dirty_sync_missed_zero_copy) {
> -            monitor_printf(mon,
> -                           "Zero-copy-send fallbacks happened: %" PRIu64 " times\n",
> +            monitor_printf(mon, ", zerocopy_fallbacks=%" PRIu64,
>                             info->ram->dirty_sync_missed_zero_copy);
>          }
> +        monitor_printf(mon, "\n");
>      }
>  
>      if (info->xbzrle_cache) {
> -        monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
> -                       info->xbzrle_cache->cache_size);
> -        monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
> -                       info->xbzrle_cache->bytes >> 10);
> -        monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
> -                       info->xbzrle_cache->pages);
> -        monitor_printf(mon, "xbzrle cache miss: %" PRIu64 " pages\n",
> -                       info->xbzrle_cache->cache_miss);
> -        monitor_printf(mon, "xbzrle cache miss rate: %0.2f\n",
> -                       info->xbzrle_cache->cache_miss_rate);
> -        monitor_printf(mon, "xbzrle encoding rate: %0.2f\n",
> -                       info->xbzrle_cache->encoding_rate);
> -        monitor_printf(mon, "xbzrle overflow: %" PRIu64 "\n",
> +        monitor_printf(mon, "XBZRLE: size=%" PRIu64
> +                       ", transferred=%" PRIu64
> +                       ", pages=%" PRIu64
> +                       ", miss=%" PRIu64 "\n"
> +                       "  miss_rate=%0.2f"
> +                       ", encode_rate=%0.2f"
> +                       ", overflow=%" PRIu64 "\n",
> +                       info->xbzrle_cache->cache_size,
> +                       info->xbzrle_cache->bytes,
> +                       info->xbzrle_cache->pages,
> +                       info->xbzrle_cache->cache_miss,
> +                       info->xbzrle_cache->cache_miss_rate,
> +                       info->xbzrle_cache->encoding_rate,
>                         info->xbzrle_cache->overflow);
>      }
>  
> +    if (!show_all) {
> +        goto out;
> +    }
> +
>      if (info->has_cpu_throttle_percentage) {
>          monitor_printf(mon, "cpu throttle percentage: %" PRIu64 "\n",
>                         info->cpu_throttle_percentage);
> @@ -191,24 +213,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>          g_free(str);
>          visit_free(v);
>      }
> -    if (info->has_socket_address) {
> -        SocketAddressList *addr;
> -
> -        monitor_printf(mon, "socket address: [\n");
> -
> -        for (addr = info->socket_address; addr; addr = addr->next) {
> -            char *s = socket_uri(addr->value);
> -            monitor_printf(mon, "\t%s\n", s);
> -            g_free(s);
> -        }
> -        monitor_printf(mon, "]\n");
> -    }
> -
> -    if (info->vfio) {
> -        monitor_printf(mon, "vfio device transferred: %" PRIu64 " kbytes\n",
> -                       info->vfio->transferred >> 10);
> -    }
>  
> +out:
>      qapi_free_MigrationInfo(info);
>  }
>  
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index c59cd6637b..639a450ee5 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -475,9 +475,9 @@ ERST
>  
>      {
>          .name       = "migrate",
> -        .args_type  = "",
> -        .params     = "",
> -        .help       = "show migration status",
> +        .args_type  = "all:-a",
> +        .params     = "[-a]",
> +        .help       = "show migration status (-a: all, dump all status)",
>          .cmd        = hmp_info_migrate,
>      },
>  
> -- 
> 2.49.0
> 

Re: [PATCH 2/3] migration/hmp: Dump global in "info migrate_parameters" instead

[Peter Xu]

On Wed, May 14, 2025 at 01:25:05PM +0000, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > "info migrate" is the command people would frequently use to query
> > migration status.  We may not want it to dump global configurations because
> > dumping the same things over and over won't help.
> > 
> > The globals are just more suitable for a parameter dump instead.  Hence
> > move it over.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> I think this is a *little* odd, since 'info migrate_parameters'
> is a list of the things you can set with 'migrate_set_parameters'
> Perhaps it would be better to add it under the 'info migrate -a' option
> you add in the next patch?

Makes sense.

> 
> However, one other thing, the globals stuff prints a
> 'globals:'  at the start,  but doesn't print anything at the end,
> so if you make this change, you end up with out being able to tell
> where the globals end and the parameters start, so maybe a
> 'parameters:' after it?

I'll drop this patch, but only dump the globals if "-a" is specified in the
next patch.

Thanks!

-- 
Peter Xu

Re: [PATCH 3/3] migration/hmp: Add "info migrate -a", reorg the dump

[Peter Xu]

On Wed, May 14, 2025 at 04:33:16PM +0200, Juraj Marcin wrote:
> >      if (info->ram) {
> > -        monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n",
> > -                       info->ram->transferred >> 10);
> > -        monitor_printf(mon, "throughput: %0.2f mbps\n",
> > +        monitor_printf(mon, "RAM info:\n");
> > +        monitor_printf(mon, "  Bandwidth (mbps): %0.2f\n",
> >                         info->ram->mbps);
> 
> I think the previous name (throughput) was better suited for this
> metric. IIUC '->mbps' is the actual amount of data that has been sent
> over a period of time, which is exactly the definition of throughput.
> Bandwidth, on the other hand, is more of a (theoretical) maximum that
> could be sent.

Heh, it's interesting you found this change, I should have mentioned it.

I think I saw more use of the word "bandwidth" from people to describe
miration throughput, but I don't think I was able to distinguish the two
before, after checking I think you're right.

I believe you demostrated solid understanding in networking issues. :-D

I'll use "Throughput" when repost. Thanks!

-- 
Peter Xu