Re: [PATCH v2 1/4] accel: Introduce 'query-accels' QMP command

[Philippe Mathieu-Daudé]

On 3/16/21 6:29 PM, Eric Blake wrote:
> On 3/16/21 12:24 PM, 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.
>>
> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> Since v1: 'type' -> 'name' in comments
>> ---
>>  qapi/machine.json | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>>  accel/accel-qmp.c | 49 +++++++++++++++++++++++++++++++++++++++++
>>  accel/meson.build |  2 +-
>>  3 files changed, 105 insertions(+), 1 deletion(-)
>>  create mode 100644 accel/accel-qmp.c
>>
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 330189efe3d..610252fc25c 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -1471,3 +1471,58 @@
>>  ##
>>  { 'event': 'MEM_UNPLUG_ERROR',
>>    'data': { 'device': 'str', 'msg': 'str' } }
>> +
>> +##
>> +# @Accelerator:
>> +#
>> +# An enumeration of accelerator names.
>> +#
>> +# Since: 6.0
>> +##
>> +{ 'enum': 'Accelerator',
>> +  'data': [ { 'name': 'qtest' },
>> +            { 'name': 'tcg' },
>> +            { 'name': 'kvm' },
>> +            { 'name': 'hax' },
>> +            { 'name': 'hvf' },
>> +            { 'name': 'whpx' },
>> +            { 'name': 'xen' } ] }
> 
> Shorter, but semantically equivalent:
> { 'enum': 'Accelerator', 'data': [ 'qtest', 'tcg', ... ] }
> 
> I'd mention in the commit message body that we can't make the enum
> values or union branches conditional because of target-specific
> poisoning.

Good idea.

> With that,
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> If we're trying to get it into 6.0, it is a new feature, and so we
> should get it in a pull request before feature freeze today.  Otherwise
> we'll have to s/6.0/6.1/

There is no rush for this, I posted it to scratch it from my today's
TODO list and be able to focus on the next task.

I'll wait for other review and repost with 6.1 (except if someone judge
it is useful to get this for 6.0).

Thanks for your review!

Phil.