On Wed, May 28, 2025 at 03:08:38PM +0000, Dr. David Alan Gilbert wrote:
> * Markus Armbruster ([email protected]) wrote:
> > Peter Xu <[email protected]> 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 <[email protected]>
> > > Cc: Alexey Perevalov <[email protected]>
> > > Cc: Markus Armbruster <[email protected]>
> > > Signed-off-by: Peter Xu <[email protected]>
> > > ---
> > > 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!
Agreed on.. using backspace? :)
>
> Dave
>
> > I usually do something like
> >
> > sep = "";
> > for (...) {
> > printf("%s...", sep, ...);
> > sep = ", "
> > }
> >
> > To add line breaks, I'd use something like
> >
> > sep = ... ? ", " : ",\n";
Thanks for the suggestion! Definitely better..
I'll squash below into this patch when repost, comments on top always
welcomed.
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 3cf890b887..6c36e202a0 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -209,19 +209,18 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
if (info->has_postcopy_vcpu_blocktime) {
uint32List *item = info->postcopy_vcpu_blocktime;
+ const char *sep = "";
int count = 0;
monitor_printf(mon, "Postcopy vCPU Blocktime (ms): \n [");
while (item) {
- monitor_printf(mon, "%"PRIu32", ", item->value);
+ monitor_printf(mon, "%s%"PRIu32, sep, item->value);
item = item->next;
/* Each line 10 vcpu results, newline if there's more */
- if ((++count % 10 == 0) && item) {
- monitor_printf(mon, "\n ");
- }
+ sep = ((++count % 10 == 0) && item) ? ",\n " : ", ";
}
- monitor_printf(mon, "\b\b]\n");
+ monitor_printf(mon, "]\n");
}
--
Peter Xu
