qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, Igor Mammedov <imammedo@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: QMP introspecting device props common to a bus type
Date: Fri, 09 Apr 2021 16:04:03 +0200	[thread overview]
Message-ID: <87r1jjy2p8.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <YHAhQWdX15V54U8G@redhat.com> ("Daniel P. =?utf-8?Q?Berrang?= =?utf-8?Q?=C3=A9=22's?= message of "Fri, 9 Apr 2021 10:41:21 +0100")

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.



      reply	other threads:[~2021-04-09 14:06 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-07 13:44 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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r1jjy2p8.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --subject='Re: QMP introspecting device props common to a bus type' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).