On Mon, May 22, 2017 at 09:17:58AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabk...@redhat.com> writes: > > > On Thu, May 18, 2017 at 01:59:53PM +0200, Markus Armbruster wrote: > >> Eduardo Habkost <ehabk...@redhat.com> 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 think it would, in this specific case. I can't even see how to document an enum in this case without being too verbose and confusing. If one day we add a different method to plug devices (e.g. by building a composite QOM object first, and then plugging it), how exactly would we extend the enum to indicate the new method is supported by a device? > I don't like > entangled booleans much... I tend to prefer booleans because each one describe a single fact and nothing else. > > >> 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 ... Indirectly, yes: device-list-properties takes a device type as argument, but its purpose is to list the names of QOM properties we can set/get on device instances. > > > 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." True. > > > (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. I still don't think it means a nest of unions, but I prefer option 2 to option 3+union anyway. Having two separate commands returning different structures sounds simpler and better than a single command returning an union. > > > 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. I already submitted a RFC to extend qom-list-types. I will probably submit another RFC to add a query-device-type command. > > >> 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. Also, support for -object is represented using the "user-creatable" interface name. So user-creatable devices do not implement the "user-creatable" interface. We need better names for all that. I suggest we call objects supported by -object "user-creatable backend objects" instead of just "user-creatable objects". Devices are objects, but they are not backend objects. > > >> > 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. -- Eduardo