Re: QMP introspecting device props common to a bus type

[Daniel P . Berrangé]

On Thu, Apr 08, 2021 at 01:56:28PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > When introspecting properties for devices, libvirt issues a sequence of
> > QMP  'device-list-properties' commands, one for each device type we
> > need info for.  The result of this command tells us about all properties
> > possible on that specific device, which is generally just fine.
> >
> > Every now and then though, there are properties that are inherited from
> > / defined by the parent class, usually props that are common to all
> > devices attached to a given bus type.
> >
> > The current case in point is the "acpi-index" property that was added to
> > the "PCI" bus type, that is a parent for any type that is a PCI dev.
> >
> > Generally when libvirt adds support for a property, it will enable it
> > across all devices that can support the property. So we're enabling use
> > of "acpi-index" across all PCI devices.
> >
> > The question thus becomes how should we probe for existence of the
> > "acpi-index" property. The qemu-system-x86_64 emulator has somewhere
> > around 150 user creatable PCI devices according to "-device help".
> >
> > The existance of a class hierarchy is explicitly not exposed in QMP
> > because we consider that an internal impl detail, so we can't just
> > query "acpi-index" on the "PCI" parent type. 
> 
> Not true.
> 
> qapi/qom.json:
> 
>     ##
>     # @ObjectTypeInfo:
>     #
>     # This structure describes a search result from @qom-list-types
>     #
>     # @name: the type name found in the search
>     #
>     # @abstract: the type is abstract and can't be directly instantiated.
>     #            Omitted if false. (since 2.10)
>     #
>     # @parent: Name of parent type, if any (since 2.10)
>     #
>     # Since: 1.1
>     ##
>     { 'struct': 'ObjectTypeInfo',
>       'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str' } }
> 
>     ##
>     # @qom-list-types:
>     #
>     # This command will return a list of types given search parameters
>     #
>     # @implements: if specified, only return types that implement this type 
> name
>     #
>     # @abstract: if true, include abstract types in the results
>     #
>     # Returns: a list of @ObjectTypeInfo or an empty list if no results are 
> found
>     #
>     # Since: 1.1
>     ##
>     { 'command': 'qom-list-types',
>       'data': { '*implements': 'str', '*abstract': 'bool' },
>       'returns': [ 'ObjectTypeInfo' ],
>       'allow-preconfig': true }
> 
> Example 1:
> 
>     {"execute": "qom-list-types", "arguments": {"abstract": true}}
> 
> returns all type names with their parent type names.

Ah, libvirt isn't setting abstract=true when listing types during its
probing of QEMU capabilities, which is why I didn't see the parents.


> > We certainly don't want to issue 'device-list-properties' over and
> > over for all 147 devices.
> >
> > If we just pick one device type, say virtio-blk-pci, and query that
> > for "acpi-index", then our code is fragile because anyone can make
> > a QEMU build that compiles-out a specific device. This is fairly
> > unlikely for virtio devices, but never say never.
> >
> > For PCI, i'm tending towards probing for the "acpi-index" property on
> > both "pci-bridge" and "pcie-root-port", as it seems unlikely that both
> > of those will be compiled out of QEMU while still retaining PCI support.
> >
> > I'm wondering if QEMU maintainers have a view on "best practice" to
> > probe for device props that are common to specific bus types ?
> 
> The obvious
> 
>     {"execute": "device-list-properties",
>      "arguments": {"typename": "pci-device"}}
> 
> fails with "Parameter 'typename' expects a non-abstract device type".
> But its cousin qom-list-properties works:
> 
>     {"execute": "qom-list-properties",
>      "arguments": {"typename": "pci-device"}}
>     {"return": [
>      {"name": "type", "type": "string"},
>      {"name": "parent_bus", "type": "link<bus>"},
>      {"name": "realized", "type": "bool"},
>      {"name": "hotplugged", "type": "bool"},
>      {"name": "hotpluggable", "type": "bool"},
>      {"name": "failover_pair_id", "type": "str"},
>      {"name": "romfile", "type": "str"},
>      {"name": "addr", "description": "Slot and optional function number, 
> example: 06.0 or 06", "type": "int32"},
>      {"name": "romsize", "type": "uint32"},
>      {"name": "x-pcie-lnksta-dllla", "description": "on/off", "type": "bool"},
>      {"name": "rombar", "type": "uint32"},
>      {"name": "x-pcie-extcap-init", "description": "on/off", "type": "bool"},
>      {"name": "acpi-index", "type": "uint32"},
>      {"name": "multifunction", "description": "on/off", "type": "bool"},
>      {"name": "legacy-addr", "type": "str"}]}
> 
> Does this help?

Yes, its good.

Is there any reason to use 'device-list-properties' at all, given that
'qom-list-properties' exists and works for all types ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|