Re: [Qemu-devel] [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions

2015-03-31 Thread Michael Mueller 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Mon, 30 Mar 2015 14:28:01 -0600
Eric Blake ebl...@redhat.com wrote:

 On 03/30/2015 08:28 AM, Michael Mueller wrote:
  The patch adds optional parameters to the QMP command query-cpu-definitions.
  Thus the signature of routine arch_query_cpu_definitions needs to be changed
  for the stub function and all target implementations:
  
  target-arm
  target-i386
  target-ppc
  target-s390
  
  Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
  ---
 
  +++ b/qapi-schema.json
  @@ -2532,21 +2532,31 @@
   #
   # @name: the name of the CPU definition
   #
  +# @default: #optional defines if cpu model is the default (since 2.4)
 
 Reads poorly.  How about:
 
 # @default: #optional true if cpu model is the default, omitted if false
 (since 2.4)

Yep, will change

 
 
  +#
  +# @runnable: #optional defines if cpu model is runnable (since 2.4)
 
 Similarly:
 
 # @runnable: #optional true if cpu model is runnable, omitted if false
 (since 2.4)

here as well

 
  +#
   # Since: 1.2.0
   ##
   { 'type': 'CpuDefinitionInfo',
  -  'data': { 'name': 'str' } }
  +  'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool' } }
   
   ##
   # @query-cpu-definitions:
   #
   # Return a list of supported virtual CPU definitions
   #
  +# @machine: #optional machine type (since 2.4)
  +#
  +# @accel: #optional accelerator id (since 2.4)
 
 Maybe mention that these two fields are for filtering results.

I will add a comment as it is more than filtering, it is execution context 
information that allows
to determine if a cpu model is runnable.

Thanks a lot,
Michael

 

-BEGIN PGP SIGNATURE-
Version: GnuPG v1

iQIcBAEBAgAGBQJVGk/gAAoJELPcPLQSJsKQ9qcQAJiURTXS+NZO/kmKfP1aH18s
RC/bhXyV6gVmfsvZ1X7S0cH5eO4Z7AfpNNT73Mw0lYDIXei+94/Mdby4AplF7S8q
RoPVu9KxWXV6oM1nigEMvExt5n6BxIM3+/xvKt1Rkef4cx8qIRju+rCLNekmBd3E
yJtSs3Oasft8LoG+4ZPEv26jC7uvHa04bp1nZslXhgUmbUJZzRtArRohp0JA0kfl
BIzpFSKJEvGB/xwyj4bvfC4NQJ9nMtel6BhO04oxHgQNXmpaJK4vN5h7wi6PG2ac
I7mKhC/nNFPUXvQUGQ91itWH/ir1fyim4Rjhtd2Pvpq19waEg2M+dHp2YKAqg1xd
YrHpAQA/6MLqlBqrsqYzVS/LHz7juXP3u/sX5azdbZY8LPynAXqnSwqiNinvk2bA
sc3NG/JwZnbtASFrjJEpmrudS29IXuNNycISzGwrL06pwgmrFaJkpyzD6gOkJfnh
UByIMqTYskk3yP8G8K4n6775al0Zx8v39E7En+dQozEnVa/SxA5YdjJMVPOZiEt4
O/szkqCr5kcQHZJ/x42Sz0YFZ5QIidhMkX6jEqeak7q0Ow0awXgreuxXEmPYu6lG
5wHo6WP1h6tdogQnJGnyFXC5kWzp2iBYxVDP86/4aKLGyZViNPS1XDSjhd9NH4X/
9IPbCIOJYwpZF9l5GeG/
=JAwu
-END PGP SIGNATURE-

Re: [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions

2015-03-31 Thread Eduardo Habkost 
On Mon, Mar 30, 2015 at 04:28:25PM +0200, Michael Mueller wrote:
[...]
  ##
  # @query-cpu-definitions:
  #
  # Return a list of supported virtual CPU definitions
  #
 +# @machine: #optional machine type (since 2.4)
 +#
 +# @accel: #optional accelerator id (since 2.4)
 +#
  # Returns: a list of CpuDefInfo
  #
  # Since: 1.2.0
  ##
 -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
 +{ 'command': 'query-cpu-definitions',
 +  'data': { '*machine': 'str', '*accel': 'AccelId' },
 +  'returns': ['CpuDefinitionInfo'] }

What happens if the new parameters are provided to an old QEMU version
that doesn't accept them? It looks like we need an introspection
mechanism or a new command name.

-- 
Eduardo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions

2015-03-31 Thread Eric Blake 
On 03/31/2015 01:46 PM, Eduardo Habkost wrote:
 On Mon, Mar 30, 2015 at 04:28:25PM +0200, Michael Mueller wrote:
 [...]
  ##
  # @query-cpu-definitions:
  #
  # Return a list of supported virtual CPU definitions
  #
 +# @machine: #optional machine type (since 2.4)
 +#
 +# @accel: #optional accelerator id (since 2.4)
 +#
  # Returns: a list of CpuDefInfo
  #
  # Since: 1.2.0
  ##
 -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
 +{ 'command': 'query-cpu-definitions',
 +  'data': { '*machine': 'str', '*accel': 'AccelId' },
 +  'returns': ['CpuDefinitionInfo'] }
 
 What happens if the new parameters are provided to an old QEMU version
 that doesn't accept them? It looks like we need an introspection
 mechanism or a new command name.

Providing an optional parameter that a new qemu understands to an older
qemu gracefully errors out about an unknown parameter.  But it's
annoying to have to probe for whether the parameter is understood by
exploiting that particular error message from older qemu.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions

2015-03-31 Thread Michael Mueller 
On Tue, 31 Mar 2015 16:46:56 -0300
Eduardo Habkost ehabk...@redhat.com wrote:

 On Mon, Mar 30, 2015 at 04:28:25PM +0200, Michael Mueller wrote:
 [...]
   ##
   # @query-cpu-definitions:
   #
   # Return a list of supported virtual CPU definitions
   #
  +# @machine: #optional machine type (since 2.4)
  +#
  +# @accel: #optional accelerator id (since 2.4)
  +#
   # Returns: a list of CpuDefInfo
   #
   # Since: 1.2.0
   ##
  -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
  +{ 'command': 'query-cpu-definitions',
  +  'data': { '*machine': 'str', '*accel': 'AccelId' },
  +  'returns': ['CpuDefinitionInfo'] }
 
 What happens if the new parameters are provided to an old QEMU version
 that doesn't accept them? It looks like we need an introspection
 mechanism or a new command name.

Yep, as Eric mentions:

[mimu@p57lp59 (master) qemu]$ sudo virsh qemu-monitor-command zhyp027 '{ 
execute:
query-cpu-definitions, arguments: { accel: kvm, machine:
s390-ccw-virtio } }'

{id:libvirt-13,error:{class:GenericError,desc:Invalid parameter 
'accel'}}



 

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions

2015-03-30 Thread Michael Mueller 
The patch adds optional parameters to the QMP command query-cpu-definitions.
Thus the signature of routine arch_query_cpu_definitions needs to be changed
for the stub function and all target implementations:

target-arm
target-i386
target-ppc
target-s390

Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
---
 include/sysemu/arch_init.h  |  6 +-
 qapi-schema.json| 14 --
 qmp-commands.hx |  2 +-
 qmp.c   | 11 ---
 stubs/arch-query-cpu-def.c  |  6 +-
 target-arm/helper.c |  6 +-
 target-i386/cpu.c   |  6 +-
 target-ppc/translate_init.c |  6 +-
 target-s390x/cpu.c  |  6 +-
 9 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 86344a2..ab48c35 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -36,7 +36,11 @@ void audio_init(void);
 int kvm_available(void);
 int xen_available(void);
 
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp);
+CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
+  const char *machine,
+  bool has_accel,
+  AccelId accel,
+  Error **errp);
 CpuModelInfo *arch_query_cpu_model(Error **errp);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 14d294f..61a145d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2532,21 +2532,31 @@
 #
 # @name: the name of the CPU definition
 #
+# @default: #optional defines if cpu model is the default (since 2.4)
+#
+# @runnable: #optional defines if cpu model is runnable (since 2.4)
+#
 # Since: 1.2.0
 ##
 { 'type': 'CpuDefinitionInfo',
-  'data': { 'name': 'str' } }
+  'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool' } }
 
 ##
 # @query-cpu-definitions:
 #
 # Return a list of supported virtual CPU definitions
 #
+# @machine: #optional machine type (since 2.4)
+#
+# @accel: #optional accelerator id (since 2.4)
+#
 # Returns: a list of CpuDefInfo
 #
 # Since: 1.2.0
 ##
-{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
+{ 'command': 'query-cpu-definitions',
+  'data': { '*machine': 'str', '*accel': 'AccelId' },
+  'returns': ['CpuDefinitionInfo'] }
 
 ##
 # @CpuModelInfo:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8fe577f..ed86773d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3412,7 +3412,7 @@ EQMP
 
 {
 .name   = query-cpu-definitions,
-.args_type  = ,
+.args_type  = machine:s?,accel:s?,
 .mhandler.cmd_new = qmp_marshal_input_query_cpu_definitions,
 },
 
diff --git a/qmp.c b/qmp.c
index ad63803..ad52fb3 100644
--- a/qmp.c
+++ b/qmp.c
@@ -567,9 +567,14 @@ DevicePropertyInfoList *qmp_device_list_properties(const 
char *typename,
 return prop_list;
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-return arch_query_cpu_definitions(errp);
+CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
+ const char *machine,
+ bool has_accel,
+ AccelId accel,
+ Error **errp)
+{
+return arch_query_cpu_definitions(has_machine, machine,
+  has_accel, accel, errp);
 }
 
 CpuModelInfo *qmp_query_cpu_model(Error **errp)
diff --git a/stubs/arch-query-cpu-def.c b/stubs/arch-query-cpu-def.c
index 22e0b43..6f8904e 100644
--- a/stubs/arch-query-cpu-def.c
+++ b/stubs/arch-query-cpu-def.c
@@ -2,7 +2,11 @@
 #include sysemu/arch_init.h
 #include qapi/qmp/qerror.h
 
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
+  const char *machine,
+  bool has_accel,
+  AccelId accel,
+  Error **errp)
 {
 error_set(errp, QERR_UNSUPPORTED);
 return NULL;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 10886c5..d56d47a 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3535,7 +3535,11 @@ static void arm_cpu_add_definition(gpointer data, 
gpointer user_data)
 *cpu_list = entry;
 }
 
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
+  const char *machine,
+  bool has_accel,
+  AccelId accel,
+

Re: [Qemu-devel] [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions

2015-03-30 Thread Eric Blake 
On 03/30/2015 08:28 AM, Michael Mueller wrote:
 The patch adds optional parameters to the QMP command query-cpu-definitions.
 Thus the signature of routine arch_query_cpu_definitions needs to be changed
 for the stub function and all target implementations:
 
 target-arm
 target-i386
 target-ppc
 target-s390
 
 Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
 ---

 +++ b/qapi-schema.json
 @@ -2532,21 +2532,31 @@
  #
  # @name: the name of the CPU definition
  #
 +# @default: #optional defines if cpu model is the default (since 2.4)

Reads poorly.  How about:

# @default: #optional true if cpu model is the default, omitted if false
(since 2.4)


 +#
 +# @runnable: #optional defines if cpu model is runnable (since 2.4)

Similarly:

# @runnable: #optional true if cpu model is runnable, omitted if false
(since 2.4)

 +#
  # Since: 1.2.0
  ##
  { 'type': 'CpuDefinitionInfo',
 -  'data': { 'name': 'str' } }
 +  'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool' } }
  
  ##
  # @query-cpu-definitions:
  #
  # Return a list of supported virtual CPU definitions
  #
 +# @machine: #optional machine type (since 2.4)
 +#
 +# @accel: #optional accelerator id (since 2.4)

Maybe mention that these two fields are for filtering results.

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



signature.asc
Description: OpenPGP digital signature