Re: [Qemu-devel] [RFC] qmp: Return 'user_creatable' & 'hotpluggable' fields on qom-list-types

[Markus Armbruster]

Eduardo Habkost <address@hidden> writes:

> On Thu, May 18, 2017 at 01:59:53PM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <address@hidden> writes:
>> 
>> > Currently there's no way for QMP clients to get a list of device types
>> > that are really usable with -device.  This information would be useful
>> > for management software and test scripts (like the device-crash-test
>> > script I have submitted recently).  Interestingly, the "info qdm" HMP
>> > command provides this information, but no QMP command does.
>> >
>> > Add two new fields to the return value of qom-list-types:
>> > "user-creatable-device" and "hotpluggable-device".
>> 
>> Does the combination
>> 
>>     "user-creatable-device": false,
>>     "hotpluggable-device": true
>> 
>> make any sense?
>
> It doesn't, and the code ensures this won't happen.

Would a single variable with three states be clearer?  I don't like
entangled booleans much...

>> Exposing information on user-creatable/hot-pluggable via QMP makes
>> sense.  The question is how.  This is a design question, and as so often
>> with design questions, I need more words to make my case than I'd like
>> to.  Please bear with me.
>> 
>> >                                                     Also, add extra
>> > arguments for filtering the list of types based on the new fields.
>> 
>> I consider the case for filtering weak.  Let's ignore this part for now.
>
> I considered sending a version that didn't include filtering.  I
> will be happy to ignore it.  :)
>
>> 
>> We have a number of commands to introspect static information,
>> e.g. query-version, query-commands, query-qmp-schema, query-target,
>> query-machines, query-cpu-definitions, query-chardev-backends,
>> device-list-properties, qom-list-types.
>> 
>> Aside: us abandoning the convention to name such commands query-FOO is
>> regrettable.
>> 
>> In its basic form, i.e. without arguments, qom-list-types does what its
>> name suggests: it lists the QOM types.
>> 
>> It also permits finding out whether a type is abstract, but only in a
>> roundabout way: subtract the result of running it without arguments (or
>> with 'abstract':false) from the result with 'abstract':true.
>> 
>> It also permits finding the "implements" relation, but only in an even
>> more roundabout way: run it with 'implements':T for every abstract T,
>> then solve the resulting puzzle.
>> 
>> Unless there's a direct way to find both (and I'm not aware of any),
>> this is bad design.  The obvious fix is to extend its return type to
>> include the information.
>
> Agreed.
>
>> 
>> With qom-list-types fixed that way, there's precedence for exposing
>> additional information on QOM types there.
>> 
>> Note that both the "abstract" bit and the "implements directly" list
>> apply to any QOM type, not just to certain subtypes.  As proposed,
>> "user-creatable-device' and "hotpluggable-device" apply only to the
>> "device" subtype.  There's no precedence for exposing information
>> specific to certain subtypes.
>> 
>> If we want to do it anyway, then qom-list-type should perhaps return a
>> union.  Taken to the logical conclusion, this becomes a nest of unions
>> mirroring the "direct subtype of" relation.  Hmm.
>
> I don't think that's the logical conclusion.  The differences
> between "object" and "device" are hardcoded in our interfaces: we
> already have -object and -device.  Modelling our data to take
> care of thoes differences doesn't mean we will also have to treat
> the differences between other QOM types (e.g. between
> "pci-device" and "usb-device") the same way.

*If* we make qom-list-type return a union to properly reflect which
results apply to which devices, *then* the logical conclusion is a nest
of unions mirroring the "direct subtype of" relation.

"Hmm" expresses my less than enthusiastic reaction to the thought of
such a nest.

> Now, we could choose to encode that in different ways.  We could
> have a single command for all QOM types (like qom-list-types or a
> new query-qom-type command) that return an union, or have
> separate commands for object types and device types.  I'm not
> sure what's the better option here (see below for additional
> comments on that).
>
>> 
>> However, we already have a command to introspect device types:
>> device-list-properties.  Can we expose the information as read-only
>> property/ies of type "device" and be done?
>
> I don't think we should.  device-list-properties has a very
> specific purpose: listing QOM properties that can be read or
> written using qom-get and qom-set, or set in -device.  If there's
> no use case for qom-get/qom-set on a property like "hotpluggable"
> or "user-creatable", we shouldn't make them QOM properties (hence
> they shouldn't appear on device-list-properties).

I figure your argument boils down to "we should distinguish between
information about a device instance and a device type, and properties
are for the former, but ...

> But that doesn't mean we can't have something like a
> "query-device-type" command that returns other information about
> a given device type, in addition to the property list.

... we still need something for the latter."

> (In my specific use case (the device-crash-test script), I would
> prefer to avoid having to fetch the full list of properties of
> every single device type, just to find out which ones are
> user-creatable.  But this is not really performance-critical
> code, so I can live with that.)
>
> So, I see a few options here:
>
> 1) Including DeviceClass::user_creatable and
>    DeviceClass::hotpluggable in qom-list-types output.  Probably
>    using an union.
>
> 2) Adding a new "query-device-type" command, returning
>    DeviceClass::user_creatable and DeviceClass::hotpluggable, and
>    possibly other DeviceClass fields (like the list of
>    properties)
>
> 3) Adding a new "query-qom-type" command, returning extended
>    information about a QOM type.  This might include the list of
>    properties supported by the type.  This might include
>    device-specific data if the command return an union.

That way's a nest of unions.

> Those options are not mutually exclusive.  e.g.: we might decide
> that a small set of fields is useful if included in
> qom-list-types too, even if we implement a query-device-type or
> query-qom-type command.

Yes.  But let's start with just one.

>> But perhaps they aren't really specific to devices.  There are other QOM
>> types that can be created by users, e.g. with -object, so the
>> "user-creatable" bit applies more widely.  Are any of them only
>> creatable during initial startup?  If yes, then that applies more
>> widely, too.
>
> I prefer to have very specific semantics like "this type can be
> used with -device" than something more generic and prone to
> confusion like "this can be user-creatable, but the method used
> to create it can vary".

Point taken.

We have user-creatable objects and user-creatable devices.  The latter
are objects, but not user-creatable objects.  Aesthetically displeasing.

>> > I'm not sure about the naming of the new command arguments.  Maybe the
>> > names are too long, but I believe that "user-creatable-devices-only=false"
>> > would have more obvious semantics than "user-creatable-devices=false"
>> > (the latter looks ambiguous: it could mean "return only
>> > non-user-creatable devices" or "return all devices").
>> 
>> I consider the filtering feature unnecessary complexity.  The filtering
>> we have got in against my objections.  I won't veto additional filtering
>> outright, but I will insist on test coverage.
>> 
>> The unfiltered output of qom-list-types is less than 10 KiB.  Even if we
>> extend it some, the need to filter it client-side seems dubious.  If a
>> management application really wants to save resources here, it should
>> cache the result and re-get it only when QEMU changes.
>
> I don't think we really need filtering, and I will be happy to
> remove that feature on the next version.

We can always add it later if we find a real need.