| Message ID | 1430766598-8512-1-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/04/2015 01:09 PM, Eduardo Habkost wrote: > This will allow clients to query additional information directly using > qom-get on the CPU objects. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > Reference to previous discussion: > > Date: Mon, 4 May 2015 15:37:40 -0300 > From: Eduardo Habkost <ehabkost@redhat.com> > Message-ID: <20150504183740.GM17796@thinpad.lan.raisama.net> > Subject: Re: [Qemu-devel] [PATCH] cpu: Register QOM links at /machine/cpus/<index> > > The summary is: even if we provide predictable QOM paths for the CPU > objects, the qom-path field will be useful to allow the QOM objects and > query-cpu data to be matched correctly. > --- > +++ b/qapi-schema.json > @@ -602,6 +602,8 @@ > # @halted: true if the virtual CPU is in the halt state. Halt usually refers > # to a processor specific low power mode. > # > +# @qom-path: path to the CPU object in the QOM tree. > +# Needs a '(since 2.4)' designation. But definitely looks useful. > # @pc: #optional If the target is i386 or x86_64, this is the 64-bit instruction > # pointer. > # If the target is Sparc, this is the PC component of the > @@ -622,8 +624,9 @@ > # data is sent to the client, the guest may no longer be halted. > ## > { 'type': 'CpuInfo', Rebase context fun once my series goes in that does s/type/struct/ > - 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', '*pc': 'int', > - '*nip': 'int', '*npc': 'int', '*PC': 'int', 'thread_id': 'int'} } > + 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', 'qom-path': 'str', > + '*pc': 'int', '*nip': 'int', '*npc': 'int', '*PC': 'int', > + 'thread_id': 'int'} } We now have a mix of _ (thread_id) and - (qom-path). It might be nice to someday teach QMP to interchange either spelling. I'm on the fence as to whether this should use qom_path in order to be consistent within the struct itself. > +++ b/qmp-commands.hx > @@ -2569,6 +2569,7 @@ Return a json-array. Each CPU is represented by a json-object, which contains: > - "CPU": CPU index (json-int) > - "current": true if this is the current CPU, false otherwise (json-bool) > - "halted": true if the cpu is halted, false otherwise (json-bool) > +- "qom-path": path to the CPU object in the QOM tree (json-str) > - Current program counter. The key's name depends on the architecture: > "pc": i386/x86_64 (json-int) > "nip": PPC (json-int) The new output is not optional, so the example needs to be updated to show it. For that matter, the example is broken, so you could fix it while touching it: "return":[ { "CPU":0, "current":true, "halted":false, "pc":3227107138 "thread_id":3134 is not valid JSON, since there is a missing comma for "pc".
On Mon, May 04, 2015 at 04:09:58PM -0300, Eduardo Habkost wrote: > This will allow clients to query additional information directly using > qom-get on the CPU objects. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> I'm not sure if it's the only way to accomplish what we need in these new schemes, but it seems like a reasonable thing to have regardless. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > Reference to previous discussion: > > Date: Mon, 4 May 2015 15:37:40 -0300 > From: Eduardo Habkost <ehabkost@redhat.com> > Message-ID: <20150504183740.GM17796@thinpad.lan.raisama.net> > Subject: Re: [Qemu-devel] [PATCH] cpu: Register QOM links at /machine/cpus/<index> > > The summary is: even if we provide predictable QOM paths for the CPU > objects, the qom-path field will be useful to allow the QOM objects and > query-cpu data to be matched correctly. > --- > cpus.c | 1 + > qapi-schema.json | 7 +++++-- > qmp-commands.hx | 1 + > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/cpus.c b/cpus.c > index 62d157a..de6469f 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -1435,6 +1435,7 @@ CpuInfoList *qmp_query_cpus(Error **errp) > info->value->CPU = cpu->cpu_index; > info->value->current = (cpu == first_cpu); > info->value->halted = cpu->halted; > + info->value->qom_path = object_get_canonical_path(OBJECT(cpu)); > info->value->thread_id = cpu->thread_id; > #if defined(TARGET_I386) > info->value->has_pc = true; > diff --git a/qapi-schema.json b/qapi-schema.json > index ac9594d..7a52a78 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -602,6 +602,8 @@ > # @halted: true if the virtual CPU is in the halt state. Halt usually refers > # to a processor specific low power mode. > # > +# @qom-path: path to the CPU object in the QOM tree. > +# > # @pc: #optional If the target is i386 or x86_64, this is the 64-bit instruction > # pointer. > # If the target is Sparc, this is the PC component of the > @@ -622,8 +624,9 @@ > # data is sent to the client, the guest may no longer be halted. > ## > { 'type': 'CpuInfo', > - 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', '*pc': 'int', > - '*nip': 'int', '*npc': 'int', '*PC': 'int', 'thread_id': 'int'} } > + 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', 'qom-path': 'str', > + '*pc': 'int', '*nip': 'int', '*npc': 'int', '*PC': 'int', > + 'thread_id': 'int'} } > > ## > # @query-cpus: > diff --git a/qmp-commands.hx b/qmp-commands.hx > index d4a837c..5c92162 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2569,6 +2569,7 @@ Return a json-array. Each CPU is represented by a json-object, which contains: > - "CPU": CPU index (json-int) > - "current": true if this is the current CPU, false otherwise (json-bool) > - "halted": true if the cpu is halted, false otherwise (json-bool) > +- "qom-path": path to the CPU object in the QOM tree (json-str) > - Current program counter. The key's name depends on the architecture: > "pc": i386/x86_64 (json-int) > "nip": PPC (json-int)
On Mon, 4 May 2015 16:09:58 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > This will allow clients to query additional information directly using > qom-get on the CPU objects. Eduardo, I'm not applying this patch this time because Eric's comments have to be addressed. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > Reference to previous discussion: > > Date: Mon, 4 May 2015 15:37:40 -0300 > From: Eduardo Habkost <ehabkost@redhat.com> > Message-ID: <20150504183740.GM17796@thinpad.lan.raisama.net> > Subject: Re: [Qemu-devel] [PATCH] cpu: Register QOM links at /machine/cpus/<index> > > The summary is: even if we provide predictable QOM paths for the CPU > objects, the qom-path field will be useful to allow the QOM objects and > query-cpu data to be matched correctly. > --- > cpus.c | 1 + > qapi-schema.json | 7 +++++-- > qmp-commands.hx | 1 + > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/cpus.c b/cpus.c > index 62d157a..de6469f 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -1435,6 +1435,7 @@ CpuInfoList *qmp_query_cpus(Error **errp) > info->value->CPU = cpu->cpu_index; > info->value->current = (cpu == first_cpu); > info->value->halted = cpu->halted; > + info->value->qom_path = object_get_canonical_path(OBJECT(cpu)); > info->value->thread_id = cpu->thread_id; > #if defined(TARGET_I386) > info->value->has_pc = true; > diff --git a/qapi-schema.json b/qapi-schema.json > index ac9594d..7a52a78 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -602,6 +602,8 @@ > # @halted: true if the virtual CPU is in the halt state. Halt usually refers > # to a processor specific low power mode. > # > +# @qom-path: path to the CPU object in the QOM tree. > +# > # @pc: #optional If the target is i386 or x86_64, this is the 64-bit instruction > # pointer. > # If the target is Sparc, this is the PC component of the > @@ -622,8 +624,9 @@ > # data is sent to the client, the guest may no longer be halted. > ## > { 'type': 'CpuInfo', > - 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', '*pc': 'int', > - '*nip': 'int', '*npc': 'int', '*PC': 'int', 'thread_id': 'int'} } > + 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', 'qom-path': 'str', > + '*pc': 'int', '*nip': 'int', '*npc': 'int', '*PC': 'int', > + 'thread_id': 'int'} } > > ## > # @query-cpus: > diff --git a/qmp-commands.hx b/qmp-commands.hx > index d4a837c..5c92162 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2569,6 +2569,7 @@ Return a json-array. Each CPU is represented by a json-object, which contains: > - "CPU": CPU index (json-int) > - "current": true if this is the current CPU, false otherwise (json-bool) > - "halted": true if the cpu is halted, false otherwise (json-bool) > +- "qom-path": path to the CPU object in the QOM tree (json-str) > - Current program counter. The key's name depends on the architecture: > "pc": i386/x86_64 (json-int) > "nip": PPC (json-int)
On Fri, May 08, 2015 at 08:51:45AM -0400, Luiz Capitulino wrote: > On Mon, 4 May 2015 16:09:58 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > This will allow clients to query additional information directly using > > qom-get on the CPU objects. > > Eduardo, I'm not applying this patch this time because Eric's comments > have to be addressed. Yes, I will submit a new version later.
Am 08.05.2015 um 16:36 schrieb Eduardo Habkost: > On Fri, May 08, 2015 at 08:51:45AM -0400, Luiz Capitulino wrote: >> On Mon, 4 May 2015 16:09:58 -0300 >> Eduardo Habkost <ehabkost@redhat.com> wrote: >> >>> This will allow clients to query additional information directly using >>> qom-get on the CPU objects. >> >> Eduardo, I'm not applying this patch this time because Eric's comments >> have to be addressed. > > Yes, I will submit a new version later. If you just add the "since 2.4", Reviewed-by: Andreas Färber <afaerber@suse.de> As to qom-path vs. qom_path, I think we're using qom-path elsewhere in QMP or some of the scripts? I mainly care about the QOM properties being consistent, so choose as you see fit here. As for the reference to /machine/cpus/... I still think you're missing the point I made on the call and in my RFC. You shouldn't expect all CPUs to live in the same place like they do for PC or pSeries. Instead, I think exposing the same search-by-type functionality we have in the QOM C API in some QMP command (qom-search? qom-find? or qom-list?) will be much better than trying to "equalize" the composition tree for the benefit of libvirt accessing it by fixed paths. Therefore my abstract socket type, and I would propose to do the same thing for the core. Regards, Andreas
On Fri, May 08, 2015 at 04:46:42PM +0200, Andreas Färber wrote: > Am 08.05.2015 um 16:36 schrieb Eduardo Habkost: > > On Fri, May 08, 2015 at 08:51:45AM -0400, Luiz Capitulino wrote: > >> On Mon, 4 May 2015 16:09:58 -0300 > >> Eduardo Habkost <ehabkost@redhat.com> wrote: > >> > >>> This will allow clients to query additional information directly using > >>> qom-get on the CPU objects. > >> > >> Eduardo, I'm not applying this patch this time because Eric's comments > >> have to be addressed. > > > > Yes, I will submit a new version later. > > If you just add the "since 2.4", > > Reviewed-by: Andreas Färber <afaerber@suse.de> > > As to qom-path vs. qom_path, I think we're using qom-path elsewhere in > QMP or some of the scripts? I mainly care about the QOM properties being > consistent, so choose as you see fit here. qapi-code-gen.txt recommends hyphens, but also notes that consistency is preferred when extending existing commands that already use underscores. So unless there are objections, the next version of this patch will use "qom_path". > > As for the reference to /machine/cpus/... I still think you're missing > the point I made on the call and in my RFC. You shouldn't expect all > CPUs to live in the same place like they do for PC or pSeries. Instead, Which point, on which call? Note that I am not proposing changing where the CPUs live, I was just adding links in a well-known place, to make things like query-cpus unnecessary. Machines are still free to place CPUs wherever they want. > Instead, > I think exposing the same search-by-type functionality we have in the > QOM C API in some QMP command (qom-search? qom-find? or qom-list?) will > be much better than trying to "equalize" the composition tree for the > benefit of libvirt accessing it by fixed paths. Therefore my abstract > socket type, and I would propose to do the same thing for the core. A search-by-type functionality may work, too, but when would it be available? Fortunately the qom-path field will be enough for what libvirt needs today, so it won't need to wait until a new core QOM feature is implemented to get its work done. If you reject /machine/cpus, I won't spend time implementing an alternative mechanism.
diff --git a/cpus.c b/cpus.c index 62d157a..de6469f 100644 --- a/cpus.c +++ b/cpus.c @@ -1435,6 +1435,7 @@ CpuInfoList *qmp_query_cpus(Error **errp) info->value->CPU = cpu->cpu_index; info->value->current = (cpu == first_cpu); info->value->halted = cpu->halted; + info->value->qom_path = object_get_canonical_path(OBJECT(cpu)); info->value->thread_id = cpu->thread_id; #if defined(TARGET_I386) info->value->has_pc = true; diff --git a/qapi-schema.json b/qapi-schema.json index ac9594d..7a52a78 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -602,6 +602,8 @@ # @halted: true if the virtual CPU is in the halt state. Halt usually refers # to a processor specific low power mode. # +# @qom-path: path to the CPU object in the QOM tree. +# # @pc: #optional If the target is i386 or x86_64, this is the 64-bit instruction # pointer. # If the target is Sparc, this is the PC component of the @@ -622,8 +624,9 @@ # data is sent to the client, the guest may no longer be halted. ## { 'type': 'CpuInfo', - 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', '*pc': 'int', - '*nip': 'int', '*npc': 'int', '*PC': 'int', 'thread_id': 'int'} } + 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', 'qom-path': 'str', + '*pc': 'int', '*nip': 'int', '*npc': 'int', '*PC': 'int', + 'thread_id': 'int'} } ## # @query-cpus: diff --git a/qmp-commands.hx b/qmp-commands.hx index d4a837c..5c92162 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2569,6 +2569,7 @@ Return a json-array. Each CPU is represented by a json-object, which contains: - "CPU": CPU index (json-int) - "current": true if this is the current CPU, false otherwise (json-bool) - "halted": true if the cpu is halted, false otherwise (json-bool) +- "qom-path": path to the CPU object in the QOM tree (json-str) - Current program counter. The key's name depends on the architecture: "pc": i386/x86_64 (json-int) "nip": PPC (json-int)