Re: [PATCH 02/11] migration/hmp: Fix postcopy-blocktime per-vCPU results

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

* Markus Armbruster (armbru@redhat.com) wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > Unfortunately, it was never correctly shown..
> >
> > This is only found when I started to look into making the blocktime feature
> > more useful (so as to avoid using bpftrace, even though I'm not sure which
> > one will be harder to use..).
> >
> > So the old dump would look like this:
> >
> >   Postcopy vCPU Blocktime: 0-1,4,10,21,33,46,48,59
> >
> > Even though there're actually 40 vcpus, and the string will merge same
> > elements and also sort them.
> >
> > To fix it, simply loop over the uint32List manually.  Now it looks like:
> >
> >   Postcopy vCPU Blocktime (ms):
> >    [15, 0, 0, 43, 29, 34, 36, 29, 37, 41,
> >     33, 37, 45, 52, 50, 38, 40, 37, 40, 49,
> >     40, 35, 35, 35, 81, 19, 18, 19, 18, 30,
> >     22, 3, 0, 0, 0, 0, 0, 0, 0, 0]
> >
> > Cc: Dr. David Alan Gilbert <dave@treblig.org>
> > Cc: Alexey Perevalov <a.perevalov@samsung.com>
> > Cc: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  migration/migration-hmp-cmds.c | 23 ++++++++++++++---------
> >  1 file changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> > index 367ff6037f..3cf890b887 100644
> > --- a/migration/migration-hmp-cmds.c
> > +++ b/migration/migration-hmp-cmds.c
> > @@ -208,15 +208,20 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
> >      }
> >  
> >      if (info->has_postcopy_vcpu_blocktime) {
> > -        Visitor *v;
> > -        char *str;
> > -        v = string_output_visitor_new(false, &str);
> > -        visit_type_uint32List(v, NULL, &info->postcopy_vcpu_blocktime,
> > -                              &error_abort);
> > -        visit_complete(v, &str);
> > -        monitor_printf(mon, "Postcopy vCPU Blocktime: %s\n", str);
> > -        g_free(str);
> > -        visit_free(v);
> > +        uint32List *item = info->postcopy_vcpu_blocktime;
> > +        int count = 0;
> > +
> > +        monitor_printf(mon, "Postcopy vCPU Blocktime (ms): \n [");
> > +
> > +        while (item) {
> > +            monitor_printf(mon, "%"PRIu32", ", item->value);
> > +            item = item->next;
> > +            /* Each line 10 vcpu results, newline if there's more */
> 
> The list can be arbitrarily long?

One per vCPU, so a small arbitrary.

> > +            if ((++count % 10 == 0) && item) {
> > +                monitor_printf(mon, "\n  ");
> > +            }
> > +        }
> > +        monitor_printf(mon, "\b\b]\n");
> 
> Uh, backspace?

Agreed!

Dave

> I usually do something like
> 
>     sep = "";
>     for (...) {
>         printf("%s...", sep, ...);
>         sep = ", "
>     }
> 
> To add line breaks, I'd use something like
> 
>         sep = ... ? ", " : ",\n";
> 
> >      }
> >  
> >  out:
> 
> The less the string visitors are used, the happier I am.
> 
-- 
 -----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   |_______/