Re: [Qemu-devel] [PATCH] hmp/info_migration: formatting migration capability output

[Markus Armbruster]

Wei Yang <address@hidden> writes:

> On Fri, Aug 02, 2019 at 07:24:34AM +0200, Markus Armbruster wrote:
>>Subject doesn't quite conform to conventions.  Suggest
>>
>>    hmp: Improve how "info migrate" formats capabilities
>
> Thanks
>
>>
>>Wei Yang <address@hidden> writes:
>>
>>> Current we put all migration capability in one line, which make it hard
>>> to read them and someone them are missed due to terminal width.
>>>
>>> This patch formats it to print 4 in one line, which looks like this now:
>>>
>>> capabilities:
>>>               xbzrle: off         rdma-pin-all: off        auto-converge: 
>>> off          zero-blocks: off
>>>             compress:  on               events: off         postcopy-ram: 
>>> off               x-colo: off
>>>          release-ram: off                block: off          return-path: 
>>> off pause-before-switchover: off
>>>              multifd: off        dirty-bitmaps: off   postcopy-blocktime: 
>>> off  late-block-activate: off
>>>      x-ignore-shared: off
>>>
>>> Signed-off-by: Wei Yang <address@hidden>
>>> ---
>>>  monitor/hmp-cmds.c | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>>> index 5ca3ebe942..29ce5b73e4 100644
>>> --- a/monitor/hmp-cmds.c
>>> +++ b/monitor/hmp-cmds.c
>>> @@ -229,9 +229,13 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>>>  
>>>      /* do not display parameters during setup */
>>>      if (info->has_status && caps) {
>>> +        int index = 0;
>>>          monitor_printf(mon, "capabilities: ");
>>> -        for (cap = caps; cap; cap = cap->next) {
>>> -            monitor_printf(mon, "%s: %s ",
>>> +        for (cap = caps; cap; cap = cap->next, index++) {
>>> +            if (!(index % 4)) {
>>> +                monitor_printf(mon, "\n");
>>> +            }
>>> +            monitor_printf(mon, "%20s: %3s ",
>>>                             MigrationCapability_str(cap->value->capability),
>>>                             cap->value->state ? "on" : "off");
>>>          }
>>
>>This assumes migration capability names are at most 20 characters long.
>>late-block-activate is pushing it already: 19 characters.
>>
>>It adds up to 104 characters per line, which is rather wide.
>>
>>What about putting each capability on its own line, just like globals,
>>and just like "info migrate_capabilities"?
>>
>>(qemu) info migrate
>>globals:
>>store-global-state: on
>>only-migratable: off
>>send-configuration: on
>>send-section-footer: on
>>decompress-error-check: on
>>clear-bitmap-shift: 18
>>capabilities:
>>xbzrle: off
>>rdma-pin-all: off
>>auto-converge: off
>>[...]
>
> This looks good, while would reader may feel difficult to clarify from which
> one the capabilities stops. 
>
> How about add 4 space at the beginning of each capability?
>
> Would the output be too long? How about wrap two capability into one line?

Perhaps David has a preference.