qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] hmp/info_migration: formatting migration capability output
@ 2019-08-02  2:07 Wei Yang
  2019-08-02  5:24 ` Markus Armbruster
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Yang @ 2019-08-02  2:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: dgilbert, Wei Yang

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");
         }
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] hmp/info_migration: formatting migration capability output
  2019-08-02  2:07 [Qemu-devel] [PATCH] hmp/info_migration: formatting migration capability output Wei Yang
@ 2019-08-02  5:24 ` Markus Armbruster
  2019-08-02  5:43   ` Wei Yang
  0 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2019-08-02  5:24 UTC (permalink / raw)
  To: Wei Yang; +Cc: qemu-devel, dgilbert

Subject doesn't quite conform to conventions.  Suggest

    hmp: Improve how "info migrate" formats capabilities

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
[...]


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] hmp/info_migration: formatting migration capability output
  2019-08-02  5:24 ` Markus Armbruster
@ 2019-08-02  5:43   ` Wei Yang
  2019-08-02 10:02     ` Dr. David Alan Gilbert
  2019-08-02 12:12     ` Markus Armbruster
  0 siblings, 2 replies; 6+ messages in thread
From: Wei Yang @ 2019-08-02  5:43 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Wei Yang, dgilbert, qemu-devel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] hmp/info_migration: formatting migration capability output
  2019-08-02  5:43   ` Wei Yang
@ 2019-08-02 10:02     ` Dr. David Alan Gilbert
  2019-08-02 21:33       ` Wei Yang
  2019-08-02 12:12     ` Markus Armbruster
  1 sibling, 1 reply; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-02 10:02 UTC (permalink / raw)
  To: Wei Yang; +Cc: Markus Armbruster, qemu-devel

* Wei Yang (richardw.yang@linux.intel.com) wrote:
> 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?

IMHO the simpler way is just not to display the capabilities on
'info migrate' at all.  If people want the list of capabilities there's
already 'info migrate_capabiltiies' they can use.

It was nice displaying them at the start of 'info migrate'
when we only had a few, but nowe we've got so many it's perhaps
easier just to omit them.

Dave

> -- 
> Wei Yang
> Help you, Help me
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] hmp/info_migration: formatting migration capability output
  2019-08-02  5:43   ` Wei Yang
  2019-08-02 10:02     ` Dr. David Alan Gilbert
@ 2019-08-02 12:12     ` Markus Armbruster
  1 sibling, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2019-08-02 12:12 UTC (permalink / raw)
  To: Wei Yang; +Cc: dgilbert, qemu-devel

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

Perhaps David has a preference.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] hmp/info_migration: formatting migration capability output
  2019-08-02 10:02     ` Dr. David Alan Gilbert
@ 2019-08-02 21:33       ` Wei Yang
  0 siblings, 0 replies; 6+ messages in thread
From: Wei Yang @ 2019-08-02 21:33 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: qemu-devel, Wei Yang, Markus Armbruster

On Fri, Aug 02, 2019 at 11:02:20AM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> 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?
>
>IMHO the simpler way is just not to display the capabilities on
>'info migrate' at all.  If people want the list of capabilities there's
>already 'info migrate_capabiltiies' they can use.
>
>It was nice displaying them at the start of 'info migrate'
>when we only had a few, but nowe we've got so many it's perhaps
>easier just to omit them.
>

Got it.

>Dave
>
>> -- 
>> Wei Yang
>> Help you, Help me
>--
>Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- 
Wei Yang
Help you, Help me


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-08-02 21:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-02  2:07 [Qemu-devel] [PATCH] hmp/info_migration: formatting migration capability output Wei Yang
2019-08-02  5:24 ` Markus Armbruster
2019-08-02  5:43   ` Wei Yang
2019-08-02 10:02     ` Dr. David Alan Gilbert
2019-08-02 21:33       ` Wei Yang
2019-08-02 12:12     ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).