Re: [PATCH] monitor/hmp-cmds: add units for mirate_parameters.

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Stefano Garzarella (sgarzare@redhat.com) wrote:
> Hi Mao,
> 
> On Fri, Mar 27, 2020 at 03:32:10PM +0800, Mao Zhongyi wrote:
> > When running:
> > (qemu) info migrate_parameters
> > announce-initial: 50 ms
> > announce-max: 550 ms
> > announce-step: 100 ms
> > compress-wait-thread: on
> > ...
> > max-bandwidth: 33554432 bytes/second
> > downtime-limit: 300 milliseconds
> > x-checkpoint-delay: 20000
> > ...
> > xbzrle-cache-size: 67108864
> > 
> > add units for the parameters 'x-checkpoint-delay' and
> > 'xbzrle-cache-size', it's easier to read.
> > 
> > Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> > ---
> >  monitor/hmp-cmds.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> > index 2a900a528a..8d22f96e57 100644
> > --- a/monitor/hmp-cmds.c
> > +++ b/monitor/hmp-cmds.c
> > @@ -440,7 +440,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
> >              MigrationParameter_str(MIGRATION_PARAMETER_DOWNTIME_LIMIT),
> >              params->downtime_limit);
> >          assert(params->has_x_checkpoint_delay);
> > -        monitor_printf(mon, "%s: %u\n",
> > +        monitor_printf(mon, "%s: %u" " milliseconds\n",
>                                        ^
> here we can remove the space and use a single string "%s: %u milliseconds\n"

Yes.

> I've noticed that we use both ms or milliseconds, if you want to clean up in a
> separate patch, maybe we could use one of these everywhere. (I vote for 'ms')

I do prefer 'ms', however we do seem to just use milliseconds in
info migrate

so we should probably stick to it.

Dave

> Thanks,
> Stefano
> 
> >              MigrationParameter_str(MIGRATION_PARAMETER_X_CHECKPOINT_DELAY),
> >              params->x_checkpoint_delay);
> >          assert(params->has_block_incremental);
> > @@ -453,7 +453,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
> >          monitor_printf(mon, "%s: %s\n",
> >              MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION),
> >              MultiFDCompression_str(params->multifd_compression));
> > -        monitor_printf(mon, "%s: %" PRIu64 "\n",
> > +        monitor_printf(mon, "%s: %" PRIu64 " bytes\n",
> >              MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
> >              params->xbzrle_cache_size);
> >          monitor_printf(mon, "%s: %" PRIu64 "\n",
> > -- 
> > 2.17.1
> > 
> > 
> > 
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK