| Message ID | 1427725708-52100-13-git-send-email-mimu@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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 <mimu@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.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Mon, 30 Mar 2015 14:28:01 -0600 Eric Blake <eblake@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 <mimu@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-----
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.
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.
On Tue, 31 Mar 2015 16:46:56 -0300 Eduardo Habkost <ehabkost@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'"}} >
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, + Error **errp) { CpuDefinitionInfoList *cpu_list = NULL; GSList *list; diff --git a/target-i386/cpu.c b/target-i386/cpu.c index b2d1c95..c7fa98e 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2023,7 +2023,11 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf) } } -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) { CpuDefinitionInfoList *cpu_list = NULL; X86CPUDefinition *def; diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index d74f4f0..2729b9f 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -9472,7 +9472,11 @@ static void ppc_cpu_defs_entry(gpointer data, gpointer user_data) *first = entry; } -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) { CpuDefinitionInfoList *cpu_list = NULL; GSList *list; diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c index 1698b52..615173f 100644 --- a/target-s390x/cpu.c +++ b/target-s390x/cpu.c @@ -66,7 +66,11 @@ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf) } #ifndef CONFIG_USER_ONLY -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) { CpuDefinitionInfoList *entry; CpuDefinitionInfo *info;
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 <mimu@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(-)