Re: [PATCH v8 02/12] accel: Introduce 'query-accels' QMP command

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 6/8/21 3:27 PM, Thomas Huth wrote:
>> On 26/05/2021 19.04, Philippe Mathieu-Daudé wrote:
>>> Introduce the 'query-accels' QMP command which returns a list
>>> of built-in accelerator names.
>>>
>>> - Accelerator is a QAPI enum of all existing accelerators,
>>>
>>> - AcceleratorInfo is a QAPI structure providing accelerator
>>>    specific information. Currently the common structure base
>>>    provides the name of the accelerator, while the specific
>>>    part is empty, but each accelerator can expand it.
>>>
>>> - 'query-accels' QMP command returns a list of @AcceleratorInfo
>>>
>>> For example on a KVM-only build we get:
>>>
>>>      { "execute": "query-accels" }
>>>      {
>>>          "return": [
>>>              {
>>>                  "name": "qtest"
>>>              },
>>>              {
>>>                  "name": "kvm"
>>>              }
>>>          ]
>>>      }
>>>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>> Tested-by: Alex Bennée <alex.bennee@linaro.org>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> v8:
>>> - Include code snippet from Markus adding to machine-target.json
>>>    to be able to use enum values or union branches conditional.
>>> - Use accel_find() on enum to be sure the accelerator is enabled
>>>    at runtime (chat with jsnow / eblake).
>>>
>>> Cc: Eric Blake <eblake@redhat.com>
>>> Cc: John Snow <jsnow@redhat.com>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>   qapi/machine-target.json | 54 ++++++++++++++++++++++++++++++++++++++++
>>>   accel/accel-qmp.c        | 32 ++++++++++++++++++++++++
>>>   accel/meson.build        |  2 +-
>>>   3 files changed, 87 insertions(+), 1 deletion(-)
>>>   create mode 100644 accel/accel-qmp.c
>>>
>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>>> index e7811654b72..586a61b5d99 100644
>>> --- a/qapi/machine-target.json
>>> +++ b/qapi/machine-target.json
>>> @@ -329,3 +329,57 @@
>>>   ##
>>>   { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
>>>     'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) ||
>>> defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
>>> +
>>> +##
>>> +# @Accelerator:
>>> +#
>>> +# An enumeration of accelerator names.
>>> +#
>>> +# Since: 6.1
>>> +##
>>> +{ 'enum': 'Accelerator',
>>> +  'data': [
>>> +      { 'name': 'hax', 'if': 'defined(CONFIG_HAX)' },
>>> +      { 'name': 'hvf', 'if': 'defined(CONFIG_HVF)' },
>>> +      { 'name': 'kvm', 'if': 'defined(CONFIG_KVM)' },
>>> +      { 'name': 'qtest' },
>>> +      { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
>>> +      { 'name': 'whpx', 'if': 'defined(CONFIG_WHPX)' },
>>> +      { 'name': 'xen', 'if': 'defined(CONFIG_XEN_BACKEND)' } ] }
>>> +
>>> +##
>>> +# @AcceleratorInfo:
>>> +#
>>> +# Accelerator information.
>>> +#
>>> +# @name: The accelerator name.
>>> +#
>>> +# Since: 6.1
>>> +##
>>> +{ 'struct': 'AcceleratorInfo',
>>> +  'data': { 'name': 'Accelerator' } }
>>> +
>>> +##
>>> +# @query-accels:
>>> +#
>>> +# Get a list of AcceleratorInfo for all built-in accelerators.
>>> +#
>>> +# Returns: a list of @AcceleratorInfo describing each accelerator.
>>> +#
>>> +# Since: 6.1
>>> +#
>>> +# Example:
>>> +#
>>> +# -> { "execute": "query-accels" }
>>> +# <- { "return": [
>>> +#        {
>>> +#            "name": "qtest"
>>> +#        },
>>> +#        {
>>> +#            "name": "kvm"
>>> +#        }
>>> +#    ] }
>>> +#
>>> +##
>>> +{ 'command': 'query-accels',
>>> +  'returns': ['AcceleratorInfo'] }
>> 
>> What about Markus' comment here:
>> 
>>  https://lore.kernel.org/qemu-devel/87mtsoieyz.fsf@dusky.pond.sub.org/
>> 
>> ?
>> 
>> If I've got him right, you don't need the command at all, the Accelerator
>> enum should be sufficient?
>
> Yes, this is the part jsnow said "we are waiting for Markus to comment"
> on the other thread ;) We'd like to only have enums, but QAPI doesn't
> seem to allow a "leaf without branch", we need a command or struct to
> use the enum else it is elided.

Correct; query-qmp-schema shows only the stuff used by commands and
events.

>                                 Or maybe we didn't understood Markus
> idea.