[Qemu-devel] [PATCH] qmp: Add qom-path field to query-cpus command

[Qemu-devel] [PATCH] qmp: Add qom-path field to query-cpus command

[Eduardo Habkost]

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.
---
 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)
-- 
2.1.0

Re: [Qemu-devel] [PATCH] qmp: Add qom-path field to query-cpus command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3059 bytes --]

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".

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] qmp: Add qom-path field to query-cpus command

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3489 bytes --]

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)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] qmp: Add qom-path field to query-cpus command

[Luiz Capitulino]

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)

Re: [Qemu-devel] [PATCH] qmp: Add qom-path field to query-cpus command

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

-- 
Eduardo

Re: [Qemu-devel] [PATCH] qmp: Add qom-path field to query-cpus command

[Andreas Färber]

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

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH] qmp: Add qom-path field to query-cpus command

[Eduardo Habkost]

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.

-- 
Eduardo