Re: QMP introspecting device props common to a bus type

From: Markus Armbruster <armbru@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: "Igor Mammedov" <imammedo@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	qemu-devel@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: QMP introspecting device props common to a bus type
Date: Fri, 09 Apr 2021 11:18:42 +0200	[thread overview]
Message-ID: <87czv34xzh.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20210409064642.ah2tz5vjz2ngfiyo@sirius.home.kraxel.org> (Gerd Hoffmann's message of "Fri, 9 Apr 2021 08:46:42 +0200")

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.