From: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
This commit 137b5cb6ab565cb3781d5337591e155932b4230e
refactors info cpus output and changes output format from
'thread_id' to 'thread-id', this would break parsing
of output in above layers like libvirt, test framework etc.
This patch just reverts back output format to 'thread_id'.
CC: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
---
hmp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hmp.c b/hmp.c
index 679467d85a..a25c7bd9a8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -381,7 +381,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "%c CPU #%" PRId64 ":", active,
cpu->value->cpu_index);
- monitor_printf(mon, " thread-id=%" PRId64 "\n", cpu->value->thread_id);
+ monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
}
qapi_free_CpuInfoFastList(cpu_list);
--
2.14.3
On Tue, Mar 27, 2018 at 06:08:00PM +0530, sathnaga@linux.vnet.ibm.com wrote: > From: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > > This commit 137b5cb6ab565cb3781d5337591e155932b4230e > refactors info cpus output and changes output format from > 'thread_id' to 'thread-id', this would break parsing > of output in above layers like libvirt, test framework etc. Libvirt doesn't use HMP, only QMP, so this shouldn't have broken libvirt afaik. HMP is not intended to be a stable API so these kind of changes should be expected. > > This patch just reverts back output format to 'thread_id'. > > CC: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> > Signed-off-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > --- > hmp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hmp.c b/hmp.c > index 679467d85a..a25c7bd9a8 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -381,7 +381,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict) > > monitor_printf(mon, "%c CPU #%" PRId64 ":", active, > cpu->value->cpu_index); > - monitor_printf(mon, " thread-id=%" PRId64 "\n", cpu->value->thread_id); > + monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id); > } > > qapi_free_CpuInfoFastList(cpu_list); > -- > 2.14.3 > > Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Mar 27, 2018 at 01:40:00PM +0100, Daniel P. Berrangé wrote:
> On Tue, Mar 27, 2018 at 06:08:00PM +0530, sathnaga@linux.vnet.ibm.com wrote:
> > From: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
> >
> > This commit 137b5cb6ab565cb3781d5337591e155932b4230e
> > refactors info cpus output and changes output format from
> > 'thread_id' to 'thread-id', this would break parsing
> > of output in above layers like libvirt, test framework etc.
>
> Libvirt doesn't use HMP, only QMP, so this shouldn't have broken libvirt
> afaik. HMP is not intended to be a stable API so these kind of changes
> should be expected.
Am sure avocado test framework uses this output for parsing to extract threadid
and in libvirt atleast I checked below code uses it, I hope keeping the
format same as previous won't affect anything?
in src/qemu/qemu_monitor_text.c
int
qemuMonitorTextQueryCPUs(qemuMonitorPtr mon,
struct qemuMonitorQueryCpusEntry **entries,
size_t *nentries)
...
if (qemuMonitorHMPCommand(mon, "info cpus", &qemucpus) < 0)
return -1;
/*
* This is the gross format we're about to parse :-{
*
* (qemu) info cpus
* * CPU #0: pc=0x00000000000f0c4a thread_id=30019
* CPU #1: pc=0x00000000fffffff0 thread_id=30020
* CPU #2: pc=0x00000000fffffff0 (halted) thread_id=30021
*
*/
....
/* Extract host Thread ID */
if ((offset = strstr(line, "thread_id=")) == NULL)
Regards,
-Satheesh
>
> >
> > This patch just reverts back output format to 'thread_id'.
> >
> > CC: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> > Signed-off-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
> > ---
> > hmp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hmp.c b/hmp.c
> > index 679467d85a..a25c7bd9a8 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -381,7 +381,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
> >
> > monitor_printf(mon, "%c CPU #%" PRId64 ":", active,
> > cpu->value->cpu_index);
> > - monitor_printf(mon, " thread-id=%" PRId64 "\n", cpu->value->thread_id);
> > + monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
> > }
> >
> > qapi_free_CpuInfoFastList(cpu_list);
> > --
> > 2.14.3
> >
> >
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
On 03/27/2018 07:38 AM, sathnaga@linux.vnet.ibm.com wrote: > From: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > > This commit 137b5cb6ab565cb3781d5337591e155932b4230e > refactors info cpus output and changes output format from > 'thread_id' to 'thread-id', this would break parsing > of output in above layers like libvirt, test framework etc. > > This patch just reverts back output format to 'thread_id'. > > CC: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> > Signed-off-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > --- > hmp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I'm not opposed to this patch (because it is a trivial way to keep older stuff working), but agree with Daniel that HMP output can change at any time, so other stuff relying on HMP should be fixed. Libvirt is not affected, so if we DO take this into 2.12, we should update the commit message to drop mention of libvirt being impacted, maybe along the lines of: Commit 137b5cb6 refactored 'info cpus' output, changing 'thread_id' to 'thread-id'. While HMP is not a stable interface, it is trivial to keep the spelling consistent for test frameworks that have not yet updated to using QMP. With the improved commit message, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Tue, 27 Mar 2018 08:59:04 -0500 Eric Blake <eblake@redhat.com> wrote: > On 03/27/2018 07:38 AM, sathnaga@linux.vnet.ibm.com wrote: > > From: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > > > > This commit 137b5cb6ab565cb3781d5337591e155932b4230e > > refactors info cpus output and changes output format from > > 'thread_id' to 'thread-id', this would break parsing > > of output in above layers like libvirt, test framework etc. > > > > This patch just reverts back output format to 'thread_id'. > > > > CC: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> > > Signed-off-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > > --- > > hmp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > I'm not opposed to this patch (because it is a trivial way to keep older > stuff working), but agree with Daniel that HMP output can change at any > time, so other stuff relying on HMP should be fixed. Libvirt is not > affected, so if we DO take this into 2.12, we should update the commit > message to drop mention of libvirt being impacted, maybe along the lines of: > > Commit 137b5cb6 refactored 'info cpus' output, changing 'thread_id' to > 'thread-id'. While HMP is not a stable interface, it is trivial to keep > the spelling consistent for test frameworks that have not yet updated to > using QMP. > > With the improved commit message, > Reviewed-by: Eric Blake <eblake@redhat.com> > Yes, that certainly makes sense. Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On 27.03.2018 16:25, Cornelia Huck wrote: > On Tue, 27 Mar 2018 08:59:04 -0500 > Eric Blake <eblake@redhat.com> wrote: > >> On 03/27/2018 07:38 AM, sathnaga@linux.vnet.ibm.com wrote: >>> From: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> >>> >>> This commit 137b5cb6ab565cb3781d5337591e155932b4230e >>> refactors info cpus output and changes output format from >>> 'thread_id' to 'thread-id', this would break parsing >>> of output in above layers like libvirt, test framework etc. >>> >>> This patch just reverts back output format to 'thread_id'. >>> >>> CC: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> >>> Signed-off-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> >>> --- >>> hmp.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> I'm not opposed to this patch (because it is a trivial way to keep older >> stuff working), but agree with Daniel that HMP output can change at any >> time, so other stuff relying on HMP should be fixed. Libvirt is not >> affected, so if we DO take this into 2.12, we should update the commit >> message to drop mention of libvirt being impacted, maybe along the lines of: >> >> Commit 137b5cb6 refactored 'info cpus' output, changing 'thread_id' to >> 'thread-id'. While HMP is not a stable interface, it is trivial to keep >> the spelling consistent for test frameworks that have not yet updated to >> using QMP. >> >> With the improved commit message, >> Reviewed-by: Eric Blake <eblake@redhat.com> >> > > Yes, that certainly makes sense. > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > Right. Seems I got carried away during the thread_id to thread-id conversion. -- Regards, Viktor Mihajlovski
[adding the maintainer in cc] On 03/27/2018 07:38 AM, sathnaga@linux.vnet.ibm.com wrote: > From: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > > This commit 137b5cb6ab565cb3781d5337591e155932b4230e > refactors info cpus output and changes output format from > 'thread_id' to 'thread-id', this would break parsing > of output in above layers like libvirt, test framework etc. > > This patch just reverts back output format to 'thread_id'. > > CC: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> > Signed-off-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > --- > hmp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I can take this through my qapi tree if David is happy with it > > diff --git a/hmp.c b/hmp.c > index 679467d85a..a25c7bd9a8 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -381,7 +381,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict) > > monitor_printf(mon, "%c CPU #%" PRId64 ":", active, > cpu->value->cpu_index); > - monitor_printf(mon, " thread-id=%" PRId64 "\n", cpu->value->thread_id); > + monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id); > } > > qapi_free_CpuInfoFastList(cpu_list); > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
* Eric Blake (eblake@redhat.com) wrote: > [adding the maintainer in cc] > > On 03/27/2018 07:38 AM, sathnaga@linux.vnet.ibm.com wrote: > > From: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > > > > This commit 137b5cb6ab565cb3781d5337591e155932b4230e > > refactors info cpus output and changes output format from > > 'thread_id' to 'thread-id', this would break parsing > > of output in above layers like libvirt, test framework etc. > > > > This patch just reverts back output format to 'thread_id'. > > > > CC: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> > > Signed-off-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > > --- > > hmp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > I can take this through my qapi tree if David is happy with it Yes, with the commit message change you suggested. Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > > > diff --git a/hmp.c b/hmp.c > > index 679467d85a..a25c7bd9a8 100644 > > --- a/hmp.c > > +++ b/hmp.c > > @@ -381,7 +381,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict) > > monitor_printf(mon, "%c CPU #%" PRId64 ":", active, > > cpu->value->cpu_index); > > - monitor_printf(mon, " thread-id=%" PRId64 "\n", cpu->value->thread_id); > > + monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id); > > } > > qapi_free_CpuInfoFastList(cpu_list); > > > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
© 2016 - 2025 Red Hat, Inc.