* QMP introspecting device props common to a bus type @ 2021-04-07 13:44 Daniel P. Berrangé 2021-04-08 11:56 ` Markus Armbruster 0 siblings, 1 reply; 8+ messages in thread From: Daniel P. Berrangé @ 2021-04-07 13:44 UTC (permalink / raw) To: qemu-devel; +Cc: Igor Mammedov, Markus Armbruster 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 :| ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: QMP introspecting device props common to a bus type 2021-04-07 13:44 QMP introspecting device props common to a bus type Daniel P. Berrangé @ 2021-04-08 11:56 ` Markus Armbruster 2021-04-08 12:46 ` Daniel P. Berrangé 0 siblings, 1 reply; 8+ messages in thread From: Markus Armbruster @ 2021-04-08 11:56 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: Igor Mammedov, qemu-devel 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? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: QMP introspecting device props common to a bus type 2021-04-08 11:56 ` Markus Armbruster @ 2021-04-08 12:46 ` Daniel P. Berrangé 2021-04-08 14:59 ` Markus Armbruster 0 siblings, 1 reply; 8+ messages in thread From: Daniel P. Berrangé @ 2021-04-08 12:46 UTC (permalink / raw) To: Markus Armbruster; +Cc: Igor Mammedov, qemu-devel 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 :| ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: QMP introspecting device props common to a bus type 2021-04-08 12:46 ` Daniel P. Berrangé @ 2021-04-08 14:59 ` Markus Armbruster 2021-04-09 6:46 ` Gerd Hoffmann 0 siblings, 1 reply; 8+ messages in thread From: Markus Armbruster @ 2021-04-08 14:59 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Igor Mammedov, Paolo Bonzini, qemu-devel, Gerd Hoffmann 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: QMP introspecting device props common to a bus type 2021-04-08 14:59 ` Markus Armbruster @ 2021-04-09 6:46 ` Gerd Hoffmann 2021-04-09 9:18 ` Markus Armbruster 0 siblings, 1 reply; 8+ messages in thread From: Gerd Hoffmann @ 2021-04-09 6:46 UTC (permalink / raw) To: Markus Armbruster Cc: Igor Mammedov, Daniel P. Berrangé, qemu-devel, Paolo Bonzini 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: QMP introspecting device props common to a bus type 2021-04-09 6:46 ` Gerd Hoffmann @ 2021-04-09 9:18 ` Markus Armbruster 2021-04-09 9:41 ` Daniel P. Berrangé 0 siblings, 1 reply; 8+ messages in thread From: Markus Armbruster @ 2021-04-09 9:18 UTC (permalink / raw) To: Gerd Hoffmann Cc: Igor Mammedov, Daniel P. Berrangé, qemu-devel, Paolo Bonzini 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: QMP introspecting device props common to a bus type 2021-04-09 9:18 ` Markus Armbruster @ 2021-04-09 9:41 ` Daniel P. Berrangé 2021-04-09 14:04 ` Markus Armbruster 0 siblings, 1 reply; 8+ messages in thread From: Daniel P. Berrangé @ 2021-04-09 9:41 UTC (permalink / raw) To: Markus Armbruster; +Cc: Igor Mammedov, Paolo Bonzini, Gerd Hoffmann, qemu-devel 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 :| ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: QMP introspecting device props common to a bus type 2021-04-09 9:41 ` Daniel P. Berrangé @ 2021-04-09 14:04 ` Markus Armbruster 0 siblings, 0 replies; 8+ messages in thread From: Markus Armbruster @ 2021-04-09 14:04 UTC (permalink / raw) To: Daniel P. Berrangé Cc: qemu-devel, Igor Mammedov, Gerd Hoffmann, Paolo Bonzini 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-04-09 14:06 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-07 13:44 QMP introspecting device props common to a bus type Daniel P. Berrangé 2021-04-08 11:56 ` Markus Armbruster 2021-04-08 12:46 ` Daniel P. Berrangé 2021-04-08 14:59 ` Markus Armbruster 2021-04-09 6:46 ` Gerd Hoffmann 2021-04-09 9:18 ` Markus Armbruster 2021-04-09 9:41 ` Daniel P. Berrangé 2021-04-09 14:04 ` Markus Armbruster
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).