Re: [PATCH for-6.0 1/6] qapi: Add query-accel command

[Eduardo Habkost]

On Mon, Nov 16, 2020 at 10:20:04AM -0600, Eric Blake wrote:
> On 11/16/20 7:10 AM, Roman Bolshakov wrote:
> > There's a problem for management applications to determine if certain
> > accelerators available. Generic QMP command should help with that.
> > 
> > Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> > ---
> >  monitor/qmp-cmds.c | 15 +++++++++++++++
> >  qapi/machine.json  | 19 +++++++++++++++++++
> >  2 files changed, 34 insertions(+)
> > 
> 
> > +++ b/qapi/machine.json
> > @@ -591,6 +591,25 @@
> >  ##
> >  { 'command': 'query-kvm', 'returns': 'KvmInfo' }
> >  
> > +##
> > +# @query-accel:
> > +#
> > +# Returns information about an accelerator
> > +#
> > +# Returns: @KvmInfo
> > +#
> > +# Since: 6.0.0
> 
> We're inconsistent on whether we have 'Since: x.y' or 'Since: x.y.z',
> although I prefer the shorter form.  Maybe Markus has an opnion on that.
> 
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "query-accel", "arguments": { "name": "kvm" } }
> > +# <- { "return": { "enabled": true, "present": true } }
> > +#
> > +##
> > +{ 'command': 'query-accel',
> > +  'data': { 'name': 'str' },
> > +  'returns': 'KvmInfo' }
> 
> '@name' is undocumented and an open-coded string.  Better would be
> requiring 'name' to be one of an enum type.  [...]

This seem similar to CPU models, machine types, device types, and
backend object types: the set of valid values is derived from the
list of subtypes of a QOM type.  We don't duplicate lists of QOM
types in the QAPI schema, today.

Do we want to duplicate the list of accelerators in the QAPI
schema, or should we wait for a generic solution that works for
any QOM type?

>                                       [...]  Even better would be
> returning an array of KvmInfo with information on all supported
> accelerators at once, rather than making the user call this command once
> per name.

Maybe.  It would save us the work of answering the question
above, but is this (querying information for all accelerators at
once) going to be a common use case?

-- 
Eduardo