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

[Wei Yang <richardw.yang@linux.intel.com>]

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 <richardw.yang@linux.intel.com> 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 <richardw.yang@linux.intel.com>
>> ---
>>  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?

-- 
Wei Yang
Help you, Help me