Re: QMP introspecting device props common to a bus type

[Markus Armbruster <armbru@redhat.com>]

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.