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

["Dr. David Alan Gilbert" <dave@treblig.org>]

* 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   |_______/