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

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

[Peter Xu]

v2:
- Dropped patch 2, dump globals only if "-a" in the last patch [Dave]
- Keep using "Throughput" for dumping ->mbps [Juraj]
- Rearranged some more, e.g. put xbzrle/globals under -a too, added indents
  for the global dump, etc.

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

Patch 2 was an attempt that I made the HMP "info migrate" looks slightly
easier to read.  A new option "-a" is added to request a full dump,
otherwise only the important info will be dumped.

Comments welcomed, thanks.

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

 migration/migration-hmp-cmds.c | 186 +++++++++++++++++----------------
 migration/options.c            |   4 +-
 hmp-commands-info.hx           |   6 +-
 3 files changed, 101 insertions(+), 95 deletions(-)

-- 
2.49.0

[PATCH v2 1/2] 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")
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
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 v2 2/2] migration/hmp: Add "info migrate -a", reorg the dump

[Peter Xu]

A new parameter "-a" is added to "info migrate" to dump all info, while
when not specified it only dumps the important ones.  When at it, reorg
everything to make it easier to read for human.

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):

  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, sample output (default, no "-a" specified):

  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

Sample output when "-a" specified:

  Status: active
  Time (ms): total=3040, setup=4, exp_down=300
  RAM info:
    Throughput (mbps): 10.51
    Sizes (KB): psize=4, total=4211528
      transferred=3979, remain=4206452,
      precopy=3978, multifd=0, postcopy=0
    Pages: normal=992, zero=277, rate_per_sec=320
    Others: dirty_syncs=1
  Globals:
    store-global-state: on
    only-migratable: off
    send-configuration: on
    send-section-footer: on
    send-switchover-start: on
    clear-bitmap-shift: 18
  XBZRLE: size=67108864, transferred=0, pages=0, miss=188451
    miss_rate=0.00, encode_rate=0.00, overflow=0
  CPU Throttle (%): 0
  Dirty-limit Throttle (us): 0
  Dirty-limit Ring Full (us): 0
  Postcopy Blocktime (ms): 0
  Postcopy vCPU Blocktime: ...

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

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 49c26daed3..13e93d3c54 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -37,29 +37,28 @@ static void migration_global_dump(Monitor *mon)
 {
     MigrationState *ms = migrate_get_current();
 
-    monitor_printf(mon, "globals:\n");
-    monitor_printf(mon, "store-global-state: %s\n",
+    monitor_printf(mon, "Globals:\n");
+    monitor_printf(mon, "  store-global-state: %s\n",
                    ms->store_global_state ? "on" : "off");
-    monitor_printf(mon, "only-migratable: %s\n",
+    monitor_printf(mon, "  only-migratable: %s\n",
                    only_migratable ? "on" : "off");
-    monitor_printf(mon, "send-configuration: %s\n",
+    monitor_printf(mon, "  send-configuration: %s\n",
                    ms->send_configuration ? "on" : "off");
-    monitor_printf(mon, "send-section-footer: %s\n",
+    monitor_printf(mon, "  send-section-footer: %s\n",
                    ms->send_section_footer ? "on" : "off");
-    monitor_printf(mon, "send-switchover-start: %s\n",
+    monitor_printf(mon, "  send-switchover-start: %s\n",
                    ms->send_switchover_start ? "on" : "off");
-    monitor_printf(mon, "clear-bitmap-shift: %u\n",
+    monitor_printf(mon, "  clear-bitmap-shift: %u\n",
                    ms->clear_bitmap_shift);
 }
 
 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);
 
-    migration_global_dump(mon);
-
     if (info->blocked_reasons) {
         strList *reasons = info->blocked_reasons;
         monitor_printf(mon, "Outgoing migration blocked:\n");
@@ -70,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);
@@ -78,107 +77,130 @@ 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, "  Throughput (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 (!show_all) {
+        goto out;
     }
 
+    migration_global_dump(mon);
+
     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 (info->has_cpu_throttle_percentage) {
-        monitor_printf(mon, "cpu throttle percentage: %" PRIu64 "\n",
+        monitor_printf(mon, "CPU Throttle (%%): %" PRIu64 "\n",
                        info->cpu_throttle_percentage);
     }
 
     if (info->has_dirty_limit_throttle_time_per_round) {
-        monitor_printf(mon, "dirty-limit throttle time: %" PRIu64 " us\n",
+        monitor_printf(mon, "Dirty-limit Throttle (us): %" PRIu64 "\n",
                        info->dirty_limit_throttle_time_per_round);
     }
 
     if (info->has_dirty_limit_ring_full_time) {
-        monitor_printf(mon, "dirty-limit ring full time: %" PRIu64 " us\n",
+        monitor_printf(mon, "Dirty-limit Ring Full (us): %" PRIu64 "\n",
                        info->dirty_limit_ring_full_time);
     }
 
     if (info->has_postcopy_blocktime) {
-        monitor_printf(mon, "postcopy blocktime: %u\n",
+        monitor_printf(mon, "Postcopy Blocktime (ms): %" PRIu32 "\n",
                        info->postcopy_blocktime);
     }
 
@@ -189,28 +211,12 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
         visit_type_uint32List(v, NULL, &info->postcopy_vcpu_blocktime,
                               &error_abort);
         visit_complete(v, &str);
-        monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
+        monitor_printf(mon, "Postcopy vCPU Blocktime: %s\n", str);
         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 v2 2/2] migration/hmp: Add "info migrate -a", reorg the dump

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> A new parameter "-a" is added to "info migrate" to dump all info, while
> when not specified it only dumps the important ones.  When at it, reorg
> everything to make it easier to read for human.
> 
> 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.

Thanks for the update,

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

Note that you did miss the change (which would be fine as a follow up)
where I point out that I think your unit abbreviations are slightly wrong
(although I think I was wrong as well...)
I think your throughput is in Mbps (capital M or Mb/s or Mbit/s) - ie.
10^6 bits/second.

While I think all your KB are KiB not KB (i.e. 2^10 bytes).

Dave

> Before this change, it looks like (one example of a completed case):
> 
>   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, sample output (default, no "-a" specified):
> 
>   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
> 
> Sample output when "-a" specified:
> 
>   Status: active
>   Time (ms): total=3040, setup=4, exp_down=300
>   RAM info:
>     Throughput (mbps): 10.51
>     Sizes (KB): psize=4, total=4211528
>       transferred=3979, remain=4206452,
>       precopy=3978, multifd=0, postcopy=0
>     Pages: normal=992, zero=277, rate_per_sec=320
>     Others: dirty_syncs=1
>   Globals:
>     store-global-state: on
>     only-migratable: off
>     send-configuration: on
>     send-section-footer: on
>     send-switchover-start: on
>     clear-bitmap-shift: 18
>   XBZRLE: size=67108864, transferred=0, pages=0, miss=188451
>     miss_rate=0.00, encode_rate=0.00, overflow=0
>   CPU Throttle (%): 0
>   Dirty-limit Throttle (us): 0
>   Dirty-limit Ring Full (us): 0
>   Postcopy Blocktime (ms): 0
>   Postcopy vCPU Blocktime: ...
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration-hmp-cmds.c | 186 +++++++++++++++++----------------
>  hmp-commands-info.hx           |   6 +-
>  2 files changed, 99 insertions(+), 93 deletions(-)
> 
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 49c26daed3..13e93d3c54 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -37,29 +37,28 @@ static void migration_global_dump(Monitor *mon)
>  {
>      MigrationState *ms = migrate_get_current();
>  
> -    monitor_printf(mon, "globals:\n");
> -    monitor_printf(mon, "store-global-state: %s\n",
> +    monitor_printf(mon, "Globals:\n");
> +    monitor_printf(mon, "  store-global-state: %s\n",
>                     ms->store_global_state ? "on" : "off");
> -    monitor_printf(mon, "only-migratable: %s\n",
> +    monitor_printf(mon, "  only-migratable: %s\n",
>                     only_migratable ? "on" : "off");
> -    monitor_printf(mon, "send-configuration: %s\n",
> +    monitor_printf(mon, "  send-configuration: %s\n",
>                     ms->send_configuration ? "on" : "off");
> -    monitor_printf(mon, "send-section-footer: %s\n",
> +    monitor_printf(mon, "  send-section-footer: %s\n",
>                     ms->send_section_footer ? "on" : "off");
> -    monitor_printf(mon, "send-switchover-start: %s\n",
> +    monitor_printf(mon, "  send-switchover-start: %s\n",
>                     ms->send_switchover_start ? "on" : "off");
> -    monitor_printf(mon, "clear-bitmap-shift: %u\n",
> +    monitor_printf(mon, "  clear-bitmap-shift: %u\n",
>                     ms->clear_bitmap_shift);
>  }
>  
>  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);
>  
> -    migration_global_dump(mon);
> -
>      if (info->blocked_reasons) {
>          strList *reasons = info->blocked_reasons;
>          monitor_printf(mon, "Outgoing migration blocked:\n");
> @@ -70,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);
> @@ -78,107 +77,130 @@ 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, "  Throughput (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 (!show_all) {
> +        goto out;
>      }
>  
> +    migration_global_dump(mon);
> +
>      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 (info->has_cpu_throttle_percentage) {
> -        monitor_printf(mon, "cpu throttle percentage: %" PRIu64 "\n",
> +        monitor_printf(mon, "CPU Throttle (%%): %" PRIu64 "\n",
>                         info->cpu_throttle_percentage);
>      }
>  
>      if (info->has_dirty_limit_throttle_time_per_round) {
> -        monitor_printf(mon, "dirty-limit throttle time: %" PRIu64 " us\n",
> +        monitor_printf(mon, "Dirty-limit Throttle (us): %" PRIu64 "\n",
>                         info->dirty_limit_throttle_time_per_round);
>      }
>  
>      if (info->has_dirty_limit_ring_full_time) {
> -        monitor_printf(mon, "dirty-limit ring full time: %" PRIu64 " us\n",
> +        monitor_printf(mon, "Dirty-limit Ring Full (us): %" PRIu64 "\n",
>                         info->dirty_limit_ring_full_time);
>      }
>  
>      if (info->has_postcopy_blocktime) {
> -        monitor_printf(mon, "postcopy blocktime: %u\n",
> +        monitor_printf(mon, "Postcopy Blocktime (ms): %" PRIu32 "\n",
>                         info->postcopy_blocktime);
>      }
>  
> @@ -189,28 +211,12 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>          visit_type_uint32List(v, NULL, &info->postcopy_vcpu_blocktime,
>                                &error_abort);
>          visit_complete(v, &str);
> -        monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
> +        monitor_printf(mon, "Postcopy vCPU Blocktime: %s\n", str);
>          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 v2 2/2] migration/hmp: Add "info migrate -a", reorg the dump

[Peter Xu]

On Wed, May 14, 2025 at 08:29:53PM +0000, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > A new parameter "-a" is added to "info migrate" to dump all info, while
> > when not specified it only dumps the important ones.  When at it, reorg
> > everything to make it easier to read for human.
> > 
> > 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.
> 
> Thanks for the update,
> 
> Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>

Thanks for the quick comments!

> 
> Note that you did miss the change (which would be fine as a follow up)
> where I point out that I think your unit abbreviations are slightly wrong

Ouch, it's in the spam filter... :-( I would have missed that if you didn't
mention it. I would think any decent AI models would do better than this..
I have no idea how this could ever happen in 2025.

> (although I think I was wrong as well...)
> I think your throughput is in Mbps (capital M or Mb/s or Mbit/s) - ie.
> 10^6 bits/second.
> 
> While I think all your KB are KiB not KB (i.e. 2^10 bytes).

True..

Now I've read the missing reply:

https://lore.kernel.org/qemu-devel/aCSXjRCTYKbDf9le@gallifrey/

So yeh, mbps is in unit of bit, but all the rest needs fixing.  How about
below fixup to be squashed (if I won't need to repost for v3):

PS: in the fixup I also did s/psize/pagesize/ to be clear

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 13e93d3c54..ea76f72fa4 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -111,9 +111,9 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 
     if (info->ram) {
         monitor_printf(mon, "RAM info:\n");
-        monitor_printf(mon, "  Throughput (mbps): %0.2f\n",
+        monitor_printf(mon, "  Throughput (Mbps): %0.2f\n",
                        info->ram->mbps);
-        monitor_printf(mon, "  Sizes (KB): psize=%" PRIu64
+        monitor_printf(mon, "  Sizes (KiB): pagesize=%" PRIu64
                        ", total=%" PRIu64 "\n",
                        info->ram->page_size >> 10,
                        info->ram->total >> 10);

-- 
Peter Xu

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

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> On Wed, May 14, 2025 at 08:29:53PM +0000, Dr. David Alan Gilbert wrote:
> > * Peter Xu (peterx@redhat.com) wrote:
> > > A new parameter "-a" is added to "info migrate" to dump all info, while
> > > when not specified it only dumps the important ones.  When at it, reorg
> > > everything to make it easier to read for human.
> > > 
> > > 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.
> > 
> > Thanks for the update,
> > 
> > Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> 
> Thanks for the quick comments!
> 
> > 
> > Note that you did miss the change (which would be fine as a follow up)
> > where I point out that I think your unit abbreviations are slightly wrong
> 
> Ouch, it's in the spam filter... :-( I would have missed that if you didn't
> mention it. I would think any decent AI models would do better than this..
> I have no idea how this could ever happen in 2025.

Ah...

> > (although I think I was wrong as well...)
> > I think your throughput is in Mbps (capital M or Mb/s or Mbit/s) - ie.
> > 10^6 bits/second.
> > 
> > While I think all your KB are KiB not KB (i.e. 2^10 bytes).
> 
> True..
> 
> Now I've read the missing reply:
> 
> https://lore.kernel.org/qemu-devel/aCSXjRCTYKbDf9le@gallifrey/
> 
> So yeh, mbps is in unit of bit, but all the rest needs fixing.  How about
> below fixup to be squashed (if I won't need to repost for v3):
> 
> PS: in the fixup I also did s/psize/pagesize/ to be clear

That's fine.

> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 13e93d3c54..ea76f72fa4 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -111,9 +111,9 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>  
>      if (info->ram) {
>          monitor_printf(mon, "RAM info:\n");
> -        monitor_printf(mon, "  Throughput (mbps): %0.2f\n",
> +        monitor_printf(mon, "  Throughput (Mbps): %0.2f\n",
>                         info->ram->mbps);

Right.

> -        monitor_printf(mon, "  Sizes (KB): psize=%" PRIu64
> +        monitor_printf(mon, "  Sizes (KiB): pagesize=%" PRIu64

Right.

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

>                         ", total=%" PRIu64 "\n",
>                         info->ram->page_size >> 10,
>                         info->ram->total >> 10);
> 
> -- 
> Peter Xu
> 
-- 
 -----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 v2 0/2] migration: Some fix and enhancements to HMP "info migrate"

[Mario Casquero]

This series has been successfully tested. Now the HMP info migrate
command is more compact and readable. With the -a option the values of
the globals are displayed as well.

(qemu) info migrate
Status: active
Time (ms): total=4029, setup=36, exp_down=300
RAM info:
  Throughput (mbps): 938.03
  Sizes (KB): psize=4, total=16798280
    transferred=426629, remain=6121884,
    precopy=426752, multifd=0, postcopy=0
  Pages: normal=105457, zero=466489, rate_per_sec=28728
  Others: dirty_syncs=1

Tested-by: Mario Casquero <mcasquer@redhat.com>


On Wed, May 14, 2025 at 10:03 PM Peter Xu <peterx@redhat.com> wrote:
>
> v2:
> - Dropped patch 2, dump globals only if "-a" in the last patch [Dave]
> - Keep using "Throughput" for dumping ->mbps [Juraj]
> - Rearranged some more, e.g. put xbzrle/globals under -a too, added indents
>   for the global dump, etc.
>
> Patch 1 was a bug I found set capabilities, so it's pretty separate issue.
>
> Patch 2 was an attempt that I made the HMP "info migrate" looks slightly
> easier to read.  A new option "-a" is added to request a full dump,
> otherwise only the important info will be dumped.
>
> Comments welcomed, thanks.
>
> Peter Xu (2):
>   migration: Allow caps to be set when preempt or multifd cap enabled
>   migration/hmp: Add "info migrate -a", reorg the dump
>
>  migration/migration-hmp-cmds.c | 186 +++++++++++++++++----------------
>  migration/options.c            |   4 +-
>  hmp-commands-info.hx           |   6 +-
>  3 files changed, 101 insertions(+), 95 deletions(-)
>
> --
> 2.49.0
>
>

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

[Peter Xu]

On Mon, May 19, 2025 at 01:27:35PM +0200, Mario Casquero wrote:
> This series has been successfully tested. Now the HMP info migrate
> command is more compact and readable. With the -a option the values of
> the globals are displayed as well.
> 
> (qemu) info migrate
> Status: active
> Time (ms): total=4029, setup=36, exp_down=300
> RAM info:
>   Throughput (mbps): 938.03
>   Sizes (KB): psize=4, total=16798280

I just noticed there's the comma missing at the end of this line.. I'll add
it.

>     transferred=426629, remain=6121884,
>     precopy=426752, multifd=0, postcopy=0
>   Pages: normal=105457, zero=466489, rate_per_sec=28728
>   Others: dirty_syncs=1
> 
> Tested-by: Mario Casquero <mcasquer@redhat.com>

Thanks Mario.  I'll queue this series with the touch up.

-- 
Peter Xu

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

[Zhijian Li (Fujitsu) via]

On 15/05/2025 04:01, Peter Xu wrote:
> A new parameter "-a" is added to "info migrate" to dump all info, while
> when not specified it only dumps the important ones.  When at it, reorg
> everything to make it easier to read for human.
> 
> 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):
> 
>    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, sample output (default, no "-a" specified):
> 
>    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

(Feel free to ignore my comment if you have reached a consensus.)

Putting multiple fields in one line is a true need for human reading?
I don't have confident to reorg them so I feed this request to the AI,
he suggested something like this:

Migration Status:
   Status: completed

Time Statistics:
   Setup: 15 ms
   Downtime: 76 ms
   Total: 122952 ms
RAM Transfer:
   Throughput: 8717.68 mbps
   Page Size: 4 KB
   Transferred:
     Total: 130825923 KB
     Precopied: 15 KB
     Postcopied: 13691151 KB
     Multifd: 117134260 KB
   Remaining: 0 KB
   Total RAM: 16777992 KB
Page Statistics:
   Normal Pages: 32622225
   Zero Pages: 0
   Duplicate Pages: 997263
   Transfer Page Rate: 169431
   Dirty Page Rate: 1234
   Dirty Syncs: 10

Thanks
Zhijian

> 
> Sample output when "-a" specified:
> 
>    Status: active
>    Time (ms): total=3040, setup=4, exp_down=300
>    RAM info:
>      Throughput (mbps): 10.51
>      Sizes (KB): psize=4, total=4211528
>        transferred=3979, remain=4206452,
>        precopy=3978, multifd=0, postcopy=0
>      Pages: normal=992, zero=277, rate_per_sec=320
>      Others: dirty_syncs=1
>    Globals:
>      store-global-state: on
>      only-migratable: off
>      send-configuration: on
>      send-section-footer: on
>      send-switchover-start: on
>      clear-bitmap-shift: 18
>    XBZRLE: size=67108864, transferred=0, pages=0, miss=188451
>      miss_rate=0.00, encode_rate=0.00, overflow=0
>    CPU Throttle (%): 0
>    Dirty-limit Throttle (us): 0
>    Dirty-limit Ring Full (us): 0
>    Postcopy Blocktime (ms): 0
>    Postcopy vCPU Blocktime: ...
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   migration/migration-hmp-cmds.c | 186 +++++++++++++++++----------------
>   hmp-commands-info.hx           |   6 +-
>   2 files changed, 99 insertions(+), 93 deletions(-)
> 
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 49c26daed3..13e93d3c54 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -37,29 +37,28 @@ static void migration_global_dump(Monitor *mon)
>   {
>       MigrationState *ms = migrate_get_current();
>   
> -    monitor_printf(mon, "globals:\n");
> -    monitor_printf(mon, "store-global-state: %s\n",
> +    monitor_printf(mon, "Globals:\n");
> +    monitor_printf(mon, "  store-global-state: %s\n",
>                      ms->store_global_state ? "on" : "off");
> -    monitor_printf(mon, "only-migratable: %s\n",
> +    monitor_printf(mon, "  only-migratable: %s\n",
>                      only_migratable ? "on" : "off");
> -    monitor_printf(mon, "send-configuration: %s\n",
> +    monitor_printf(mon, "  send-configuration: %s\n",
>                      ms->send_configuration ? "on" : "off");
> -    monitor_printf(mon, "send-section-footer: %s\n",
> +    monitor_printf(mon, "  send-section-footer: %s\n",
>                      ms->send_section_footer ? "on" : "off");
> -    monitor_printf(mon, "send-switchover-start: %s\n",
> +    monitor_printf(mon, "  send-switchover-start: %s\n",
>                      ms->send_switchover_start ? "on" : "off");
> -    monitor_printf(mon, "clear-bitmap-shift: %u\n",
> +    monitor_printf(mon, "  clear-bitmap-shift: %u\n",
>                      ms->clear_bitmap_shift);
>   }
>   
>   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);
>   
> -    migration_global_dump(mon);
> -
>       if (info->blocked_reasons) {
>           strList *reasons = info->blocked_reasons;
>           monitor_printf(mon, "Outgoing migration blocked:\n");
> @@ -70,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);
> @@ -78,107 +77,130 @@ 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, "  Throughput (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 (!show_all) {
> +        goto out;
>       }
>   
> +    migration_global_dump(mon);
> +
>       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 (info->has_cpu_throttle_percentage) {
> -        monitor_printf(mon, "cpu throttle percentage: %" PRIu64 "\n",
> +        monitor_printf(mon, "CPU Throttle (%%): %" PRIu64 "\n",
>                          info->cpu_throttle_percentage);
>       }
>   
>       if (info->has_dirty_limit_throttle_time_per_round) {
> -        monitor_printf(mon, "dirty-limit throttle time: %" PRIu64 " us\n",
> +        monitor_printf(mon, "Dirty-limit Throttle (us): %" PRIu64 "\n",
>                          info->dirty_limit_throttle_time_per_round);
>       }
>   
>       if (info->has_dirty_limit_ring_full_time) {
> -        monitor_printf(mon, "dirty-limit ring full time: %" PRIu64 " us\n",
> +        monitor_printf(mon, "Dirty-limit Ring Full (us): %" PRIu64 "\n",
>                          info->dirty_limit_ring_full_time);
>       }
>   
>       if (info->has_postcopy_blocktime) {
> -        monitor_printf(mon, "postcopy blocktime: %u\n",
> +        monitor_printf(mon, "Postcopy Blocktime (ms): %" PRIu32 "\n",
>                          info->postcopy_blocktime);
>       }
>   
> @@ -189,28 +211,12 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>           visit_type_uint32List(v, NULL, &info->postcopy_vcpu_blocktime,
>                                 &error_abort);
>           visit_complete(v, &str);
> -        monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
> +        monitor_printf(mon, "Postcopy vCPU Blocktime: %s\n", str);
>           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,
>       },
>   

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

[Peter Xu]

On Wed, May 21, 2025 at 08:43:37AM +0000, Zhijian Li (Fujitsu) wrote:
[...]
> > After this change, sample output (default, no "-a" specified):
> > 
> >    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
> 
> (Feel free to ignore my comment if you have reached a consensus.)
> 
> Putting multiple fields in one line is a true need for human reading?

It definitely helps me but I agree that can be subjective.  So I'm happy to
collect opinions.

So my above layout was trying to leverage more on screens where width is
bigger than the height (which is pretty much the default).

> I don't have confident to reorg them so I feed this request to the AI,
> he suggested something like this:
> 
> Migration Status:
>    Status: completed
> 
> Time Statistics:
>    Setup: 15 ms
>    Downtime: 76 ms
>    Total: 122952 ms
> RAM Transfer:
>    Throughput: 8717.68 mbps
>    Page Size: 4 KB
>    Transferred:
>      Total: 130825923 KB
>      Precopied: 15 KB
>      Postcopied: 13691151 KB
>      Multifd: 117134260 KB
>    Remaining: 0 KB
>    Total RAM: 16777992 KB
> Page Statistics:
>    Normal Pages: 32622225
>    Zero Pages: 0
>    Duplicate Pages: 997263
>    Transfer Page Rate: 169431
>    Dirty Page Rate: 1234
>    Dirty Syncs: 10

I would trust you more than the AI, so feel free to share your opinion next
time (which won't hurt if it was a result of AI discussions, but only which
you agreed on :).

It's already in a pull, let's revisit whenever necessary.  Thanks for the
input!

-- 
Peter Xu

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

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> On Wed, May 21, 2025 at 08:43:37AM +0000, Zhijian Li (Fujitsu) wrote:
> [...]
> > > After this change, sample output (default, no "-a" specified):
> > > 
> > >    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
> > 
> > (Feel free to ignore my comment if you have reached a consensus.)
> > 
> > Putting multiple fields in one line is a true need for human reading?
> 
> It definitely helps me but I agree that can be subjective.  So I'm happy to
> collect opinions.
> 
> So my above layout was trying to leverage more on screens where width is
> bigger than the height (which is pretty much the default).

I think perhaps the problem with the on-one-line layout is that the grouping
is wrong;  grouping by unit probably doesn't make sense.

So it makes sense to me to have:
   Sizes: psize=4/KB
   Transfer: total=16777992 kB transferred=37673019 kB remain=11563855 kB
   Pages: normal=9394288 zero=600672
   Page rates: transferred=185875/s dirtied=278378/s
   Other: dirty_sync=3 postcopy_req=4078

so you have things on one line when you're comparing them - so it's easy
to see the transferred page/s is way less than the dirtied in this example
(really??)  because they're next to each other.
Or the 'Transfer' line is showing total, how much so far and how much remains

> > I don't have confident to reorg them so I feed this request to the AI,
> > he suggested something like this:
> > 
> > Migration Status:
> >    Status: completed
> > 
> > Time Statistics:
> >    Setup: 15 ms
> >    Downtime: 76 ms
> >    Total: 122952 ms
> > RAM Transfer:
> >    Throughput: 8717.68 mbps
> >    Page Size: 4 KB
> >    Transferred:
> >      Total: 130825923 KB
> >      Precopied: 15 KB
> >      Postcopied: 13691151 KB
> >      Multifd: 117134260 KB
> >    Remaining: 0 KB
> >    Total RAM: 16777992 KB
> > Page Statistics:
> >    Normal Pages: 32622225
> >    Zero Pages: 0
> >    Duplicate Pages: 997263
> >    Transfer Page Rate: 169431
> >    Dirty Page Rate: 1234
> >    Dirty Syncs: 10
> 
> I would trust you more than the AI, so feel free to share your opinion next
> time (which won't hurt if it was a result of AI discussions, but only which
> you agreed on :).

Haha, yes; whatever it is for those humans find easiest - that's the H in HMP!
(Those AIs can parse Json way easier than I can read Json!)

Of course you don't need to reorg it all at once again, if someone finds
one section hard, then reorg the way that people find it easy.

> It's already in a pull, let's revisit whenever necessary.  Thanks for the
> input!
> 

Nod.

Dave


> -- 
> Peter Xu
> 
-- 
 -----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 v2 2/2] migration/hmp: Add "info migrate -a", reorg the dump

[Zhijian Li (Fujitsu) via]

On 22/05/2025 05:04, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
>> On Wed, May 21, 2025 at 08:43:37AM +0000, Zhijian Li (Fujitsu) wrote:
>> [...]
>>>> After this change, sample output (default, no "-a" specified):
>>>>
>>>>     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
>>>
>>> (Feel free to ignore my comment if you have reached a consensus.)
>>>
>>> Putting multiple fields in one line is a true need for human reading?
>>
>> It definitely helps me but I agree that can be subjective.  So I'm happy to
>> collect opinions.
>>
>> So my above layout was trying to leverage more on screens where width is
>> bigger than the height (which is pretty much the default).
> 
> I think perhaps the problem with the on-one-line layout is that the grouping
> is wrong;  grouping by unit probably doesn't make sense.
> 
> So it makes sense to me to have:
>     Sizes: psize=4/KB
>     Transfer: total=16777992 kB transferred=37673019 kB remain=11563855 kB
>     Pages: normal=9394288 zero=600672
>     Page rates: transferred=185875/s dirtied=278378/s
>     Other: dirty_sync=3 postcopy_req=4078


Oh, I vote this !!, more clear to me.

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

[Zhijian Li (Fujitsu) via]

On 21/05/2025 22:03, Peter Xu wrote:
> On Wed, May 21, 2025 at 08:43:37AM +0000, Zhijian Li (Fujitsu) wrote:
> [...]
>>> After this change, sample output (default, no "-a" specified):
>>>
>>>     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
>>
>> (Feel free to ignore my comment if you have reached a consensus.)
>>
>> Putting multiple fields in one line is a true need for human reading?
> 
> It definitely helps me but I agree that can be subjective.  So I'm happy to
> collect opinions.
> 
> So my above layout was trying to leverage more on screens where width is
> bigger than the height (which is pretty much the default).
> 
>> I don't have confident to reorg them so I feed this request to the AI,
>> he suggested something like this:
>>
>> Migration Status:
>>     Status: completed
>>
>> Time Statistics:
>>     Setup: 15 ms
>>     Downtime: 76 ms
>>     Total: 122952 ms
>> RAM Transfer:
>>     Throughput: 8717.68 mbps
>>     Page Size: 4 KB
>>     Transferred:
>>       Total: 130825923 KB
>>       Precopied: 15 KB
>>       Postcopied: 13691151 KB
>>       Multifd: 117134260 KB
>>     Remaining: 0 KB
>>     Total RAM: 16777992 KB
>> Page Statistics:
>>     Normal Pages: 32622225
>>     Zero Pages: 0
>>     Duplicate Pages: 997263
>>     Transfer Page Rate: 169431
>>     Dirty Page Rate: 1234
>>     Dirty Syncs: 10
> 
> I would trust you more than the AI, so feel free to share your opinion next
> time (which won't hurt if it was a result of AI discussions, but only which
> you agreed on :).

Thank you.

Actually the AI does much better than me in any aspects.
I have decided to surrender to AI:) https://syaro.io/1ksu/

> 
> It's already in a pull, let's revisit whenever necessary.  Thanks for the
> input!

Sounds good. Let’s proceed with the this and observe feedback from the end-uers..

Thanks
Zhijian

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

[Peter Xu]

On Thu, May 22, 2025 at 12:55:05AM +0000, Zhijian Li (Fujitsu) wrote:
> 
> 
> On 22/05/2025 05:04, Dr. David Alan Gilbert wrote:
> > * Peter Xu (peterx@redhat.com) wrote:
> >> On Wed, May 21, 2025 at 08:43:37AM +0000, Zhijian Li (Fujitsu) wrote:
> >> [...]
> >>>> After this change, sample output (default, no "-a" specified):
> >>>>
> >>>>     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
> >>>
> >>> (Feel free to ignore my comment if you have reached a consensus.)
> >>>
> >>> Putting multiple fields in one line is a true need for human reading?
> >>
> >> It definitely helps me but I agree that can be subjective.  So I'm happy to
> >> collect opinions.
> >>
> >> So my above layout was trying to leverage more on screens where width is
> >> bigger than the height (which is pretty much the default).
> > 
> > I think perhaps the problem with the on-one-line layout is that the grouping
> > is wrong;  grouping by unit probably doesn't make sense.
> > 
> > So it makes sense to me to have:
> >     Sizes: psize=4/KB
> >     Transfer: total=16777992 kB transferred=37673019 kB remain=11563855 kB
> >     Pages: normal=9394288 zero=600672
> >     Page rates: transferred=185875/s dirtied=278378/s
> >     Other: dirty_sync=3 postcopy_req=4078
> 
> 
> Oh, I vote this !!, more clear to me.

I followed up with Dave's idea, but then added all entries into it, below.

  Status: postcopy-active
  Time (ms): total=40504, setup=14, down=145
  RAM info:
    Throughput (Mbps): 6102.65
    Sizes (KiB):        pagesize=4, total=16777992
    Transfers (KiB):    transferred=37673019, remain=2136404
      Channels (KiB):   precopy=3, multifd=26108780, postcopy=11563855
      Page Types:       normal=9394288, zero=600672
    Page Rates (pps):   transfer_rate=185875, dirty_rate=278378
    Others:             dirty_syncs=3, postcopy_req=4078

Logically I should have moved "Throughput" out, because that should also
include all other things (non-ram iterators, device states).  But currently
it's an entry under info->ram.. so I kept it there.

It also has the "total" in "Sizes" to make the next line shorter
(meanwhile, "total" is also a constant size like "psize"), the hope is it's
still close enough to read when reading "Transfers" on the next line.

I also provided further indents to "Channels" and "Page Types" because they
should be taken as sub-class of "Transfer".

How is this?  Since we're at it, I can send a follow up patch after we
reach a consensus (I may also include that in another series where I'll
further add things into HMP; I'm looking at making blocktime to report page
latencies too).

-- 
Peter Xu

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

[Michael Tokarev]

On 14.05.2025 23:01, 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")
> Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Hi!

Is this a qemu-stable material (for 9.2 and 10.0 branches)?
Please let me know if it is not.

Thanks,

/mjt

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

[Zhijian Li (Fujitsu) via]

On 22/05/2025 21:16, Peter Xu wrote:
> I followed up with Dave's idea, but then added all entries into it, below.
> 
>    Status: postcopy-active
>    Time (ms): total=40504, setup=14, down=145
>    RAM info:
>      Throughput (Mbps): 6102.65
>      Sizes (KiB):        pagesize=4, total=16777992
>      Transfers (KiB):    transferred=37673019, remain=2136404
>        Channels (KiB):   precopy=3, multifd=26108780, postcopy=11563855
>        Page Types:       normal=9394288, zero=600672
>      Page Rates (pps):   transfer_rate=185875, dirty_rate=278378


Regarding the "transfer_rate" and "dirty_rate" fields:
Would it be clearer to drop the "_rate" suffix since the category header
"Page Rates (pps)" already implies they are rate metrics?


>      Others:             dirty_syncs=3, postcopy_req=4078
> 
> Logically I should have moved "Throughput" out, because that should also
> include all other things (non-ram iterators, device states).  But currently
> it's an entry under info->ram.. so I kept it there.
> 
> It also has the "total" in "Sizes" to make the next line shorter
> (meanwhile, "total" is also a constant size like "psize"), the hope is it's
> still close enough to read when reading "Transfers" on the next line.
> 
> I also provided further indents to "Channels" and "Page Types" because they
> should be taken as sub-class of "Transfer".
> 
> How is this?  Since we're at it, I can send a follow up patch after we
> reach a consensus (I may also include that in another series where I'll
> further add things into HMP; I'm looking at making blocktime to report page
> latencies too).


Your revised layout aligns well with my preferences. Acked.


Thanks

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

[Peter Xu]

On Fri, May 23, 2025 at 02:06:42AM +0000, Zhijian Li (Fujitsu) wrote:
> 
> 
> On 22/05/2025 21:16, Peter Xu wrote:
> > I followed up with Dave's idea, but then added all entries into it, below.
> > 
> >    Status: postcopy-active
> >    Time (ms): total=40504, setup=14, down=145
> >    RAM info:
> >      Throughput (Mbps): 6102.65
> >      Sizes (KiB):        pagesize=4, total=16777992
> >      Transfers (KiB):    transferred=37673019, remain=2136404
> >        Channels (KiB):   precopy=3, multifd=26108780, postcopy=11563855
> >        Page Types:       normal=9394288, zero=600672
> >      Page Rates (pps):   transfer_rate=185875, dirty_rate=278378
> 
> 
> Regarding the "transfer_rate" and "dirty_rate" fields:
> Would it be clearer to drop the "_rate" suffix since the category header
> "Page Rates (pps)" already implies they are rate metrics?

I made it verbose to be clear while still suitable in a line.  I don't feel
strongly - I can drop them when proposing the patch, thanks.

-- 
Peter Xu