QMP introspecting device props common to a bus type

QMP introspecting device props common to a bus type

[Daniel P. Berrangé]

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. 

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 ?

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 :|

Re: QMP introspecting device props common to a bus type

[Markus Armbruster]

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.

The following script prints a QOM type forest:

    #!/usr/bin/python3

    true = True
    false = False
    ts = ... output of the qom-list-types above ...
    child={}

    for t in ts:
        n = t['name']
        p = t.get('parent')
        if p not in child:
            child[p] = []
        child[p].append(n)

    def print_type_tree(name, level=-1):
        if name is not None:
            print(" " * level * 4 + name)
        for c in child.get(name, []):
            print_type_tree(c, level + 1)

    print_type_tree(None)

Example 2:

    {"execute": "qom-list-types", "arguments": {"implements": "pci-device"}}

returns all the (concrete) PCI device type names.

Note that "implements" may be an interface, too.

> 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?

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 :|

Re: QMP introspecting device props common to a bus type

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> 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 ?

Good question.

device-list-properties uses module_object_class_by_name(), requires the
result to be a concrete device type, iterates over QOM properties with
object_property_iter_init() / object_property_iter_next(), skipping
properties named "type", "realized", "hotpluggable", "hotplugged",
"parent_bus", and any whose starts with "legacy-".

Paolo, can you remind us why we skip the "legacy-FOO" properties?

qom-list-properties uses object_class_by_name(), requires an object type
(an interface won't do).  If it's abstract, it iterates with
object_class_property_iter_init() / object_property_iter_next(), else
with object_property_iter_init() / object_property_iter_next().  It
doesn't skip properties.

Looks like device-list-properties has become[*] pretty much redundant
*except* for the difference between module_object_class_by_name() and
object_class_by_name().

Gerd, you changed device-list-properties from object_class_by_name() to
module_object_class_by_name() in commit 7ab6e7fcce.  Should
qom-list-properties be changed, too?  If yes, is there any reason to use
object_class_by_name() for looking up user-provided type names in QMP
commands?


[*] "has become" because they used to be more different, if memory
serves.

Re: QMP introspecting device props common to a bus type

[Gerd Hoffmann]

  Hi,

> device-list-properties uses module_object_class_by_name(), requires the
> result to be a concrete device type, iterates over QOM properties with
> object_property_iter_init() / object_property_iter_next(), skipping
> properties named "type", "realized", "hotpluggable", "hotplugged",
> "parent_bus", and any whose starts with "legacy-".
> 
> Paolo, can you remind us why we skip the "legacy-FOO" properties?
> 
> qom-list-properties uses object_class_by_name(), requires an object type
> (an interface won't do).  If it's abstract, it iterates with
> object_class_property_iter_init() / object_property_iter_next(), else
> with object_property_iter_init() / object_property_iter_next().  It
> doesn't skip properties.
> 
> Looks like device-list-properties has become[*] pretty much redundant
> *except* for the difference between module_object_class_by_name() and
> object_class_by_name().
> 
> Gerd, you changed device-list-properties from object_class_by_name() to
> module_object_class_by_name() in commit 7ab6e7fcce.  Should
> qom-list-properties be changed, too?

Makes sense.  We already have non-device modular objects
(some chardevs).

> If yes, is there any reason to use
> object_class_by_name() for looking up user-provided type names in QMP
> commands?

I've tried to be conservative and call module_object_class_by_name()
only in places where it is actually needed.  Reason one being the extra
overhead.  But maybe this isn't too bad given the extra module code runs
only on lookup failures.  Reason two is to avoid modules being loaded by
accident even when not needed.  This needs checking when you try drop
object_class_by_name().  A VM without --for example -- qxl device should
not load the qxl module.

take care,
  Gerd

Re: QMP introspecting device props common to a bus type

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> device-list-properties uses module_object_class_by_name(), requires the
>> result to be a concrete device type, iterates over QOM properties with
>> object_property_iter_init() / object_property_iter_next(), skipping
>> properties named "type", "realized", "hotpluggable", "hotplugged",
>> "parent_bus", and any whose starts with "legacy-".
>> 
>> Paolo, can you remind us why we skip the "legacy-FOO" properties?
>> 
>> qom-list-properties uses object_class_by_name(), requires an object type
>> (an interface won't do).  If it's abstract, it iterates with
>> object_class_property_iter_init() / object_property_iter_next(), else
>> with object_property_iter_init() / object_property_iter_next().  It
>> doesn't skip properties.
>> 
>> Looks like device-list-properties has become[*] pretty much redundant
>> *except* for the difference between module_object_class_by_name() and
>> object_class_by_name().
>> 
>> Gerd, you changed device-list-properties from object_class_by_name() to
>> module_object_class_by_name() in commit 7ab6e7fcce.  Should
>> qom-list-properties be changed, too?
>
> Makes sense.  We already have non-device modular objects
> (some chardevs).
>
>> If yes, is there any reason to use
>> object_class_by_name() for looking up user-provided type names in QMP
>> commands?
>
> I've tried to be conservative and call module_object_class_by_name()
> only in places where it is actually needed.  Reason one being the extra
> overhead.  But maybe this isn't too bad given the extra module code runs
> only on lookup failures.  Reason two is to avoid modules being loaded by
> accident even when not needed.  This needs checking when you try drop
> object_class_by_name().  A VM without --for example -- qxl device should
> not load the qxl module.

Yes, module load should be reasonably explicit, to avoid accidental
loading.

Automatic load on use is explicit enough.

Automatic load on introspection could perhaps be surprising.  I figure
it depends on how the introspection request is phrased.  Loading X on
"tell me more about X" feels okay.  Loading X on "show me all the X that
satisfy Y" feels iffy.

A systematic review of object_class_by_name() and
module_object_class_by_name() use might be advisable.

Re: QMP introspecting device props common to a bus type

[Daniel P. Berrangé]

On Fri, Apr 09, 2021 at 11:18:42AM +0200, Markus Armbruster wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> >   Hi,
> >
> >> device-list-properties uses module_object_class_by_name(), requires the
> >> result to be a concrete device type, iterates over QOM properties with
> >> object_property_iter_init() / object_property_iter_next(), skipping
> >> properties named "type", "realized", "hotpluggable", "hotplugged",
> >> "parent_bus", and any whose starts with "legacy-".
> >> 
> >> Paolo, can you remind us why we skip the "legacy-FOO" properties?
> >> 
> >> qom-list-properties uses object_class_by_name(), requires an object type
> >> (an interface won't do).  If it's abstract, it iterates with
> >> object_class_property_iter_init() / object_property_iter_next(), else
> >> with object_property_iter_init() / object_property_iter_next().  It
> >> doesn't skip properties.
> >> 
> >> Looks like device-list-properties has become[*] pretty much redundant
> >> *except* for the difference between module_object_class_by_name() and
> >> object_class_by_name().
> >> 
> >> Gerd, you changed device-list-properties from object_class_by_name() to
> >> module_object_class_by_name() in commit 7ab6e7fcce.  Should
> >> qom-list-properties be changed, too?
> >
> > Makes sense.  We already have non-device modular objects
> > (some chardevs).
> >
> >> If yes, is there any reason to use
> >> object_class_by_name() for looking up user-provided type names in QMP
> >> commands?
> >
> > I've tried to be conservative and call module_object_class_by_name()
> > only in places where it is actually needed.  Reason one being the extra
> > overhead.  But maybe this isn't too bad given the extra module code runs
> > only on lookup failures.  Reason two is to avoid modules being loaded by
> > accident even when not needed.  This needs checking when you try drop
> > object_class_by_name().  A VM without --for example -- qxl device should
> > not load the qxl module.
> 
> Yes, module load should be reasonably explicit, to avoid accidental
> loading.
> 
> Automatic load on use is explicit enough.
> 
> Automatic load on introspection could perhaps be surprising.  I figure
> it depends on how the introspection request is phrased.  Loading X on
> "tell me more about X" feels okay.  Loading X on "show me all the X that
> satisfy Y" feels iffy.

IIUC, the intention is that as designed today, the existance of modules
is supposed to be transparent to mgmt application.

IOW, to a mgmt app "qemu + installed qxl module" should behaviour
identically to "qemu + statically linked qxl".

Conversely "qemu + uninstalled qxl module" should behaviour identically
to "qemu + qxl disabled at buld time".

This implies that when a mgmt app introspects QEMU for features, then
QEMU must auto-load all modules that are needed to ensure introspection
results match those that would be reported in non-modular build.

If we not going to make introspetion results equivalent, then we may
need to make modules be an explicit concept so mgmt apps can find out
when introspection is incomplete and force loading when they need it.

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 :|

Re: QMP introspecting device props common to a bus type

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Fri, Apr 09, 2021 at 11:18:42AM +0200, Markus Armbruster wrote:
>> Gerd Hoffmann <kraxel@redhat.com> writes:
[...]
>> >> Gerd, you changed device-list-properties from object_class_by_name() to
>> >> module_object_class_by_name() in commit 7ab6e7fcce.  Should
>> >> qom-list-properties be changed, too?
>> >
>> > Makes sense.  We already have non-device modular objects
>> > (some chardevs).
>> >
>> >> If yes, is there any reason to use
>> >> object_class_by_name() for looking up user-provided type names in QMP
>> >> commands?
>> >
>> > I've tried to be conservative and call module_object_class_by_name()
>> > only in places where it is actually needed.  Reason one being the extra
>> > overhead.  But maybe this isn't too bad given the extra module code runs
>> > only on lookup failures.  Reason two is to avoid modules being loaded by
>> > accident even when not needed.  This needs checking when you try drop
>> > object_class_by_name().  A VM without --for example -- qxl device should
>> > not load the qxl module.
>> 
>> Yes, module load should be reasonably explicit, to avoid accidental
>> loading.
>> 
>> Automatic load on use is explicit enough.
>> 
>> Automatic load on introspection could perhaps be surprising.  I figure
>> it depends on how the introspection request is phrased.  Loading X on
>> "tell me more about X" feels okay.  Loading X on "show me all the X that
>> satisfy Y" feels iffy.
>
> IIUC, the intention is that as designed today, the existance of modules
> is supposed to be transparent to mgmt application.
>
> IOW, to a mgmt app "qemu + installed qxl module" should behaviour
> identically to "qemu + statically linked qxl".
>
> Conversely "qemu + uninstalled qxl module" should behaviour identically
> to "qemu + qxl disabled at buld time".
>
> This implies that when a mgmt app introspects QEMU for features, then
> QEMU must auto-load all modules that are needed to ensure introspection
> results match those that would be reported in non-modular build.

Since this is not the only possible design for module behavior, I'd
recomend we spell out the behavior we want in a suitable place, to avoid
misunderstandings.  Maybe we already did; if yes, pointer, please.

> If we not going to make introspetion results equivalent, then we may
> need to make modules be an explicit concept so mgmt apps can find out
> when introspection is incomplete and force loading when they need it.

They are not equivalent now.  Case in point: qom-list-properties does
not load modules.  Thus:

>> A systematic review of object_class_by_name() and
>> module_object_class_by_name() use might be advisable.