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 <pet...@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,
>       },
>   

Reply via email to