Re: [PATCH 2/3] qapi, audio: respect build time conditions in audio schema

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Mar 05, 2021 at 11:56:13AM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Wed, Mar 03, 2021 at 08:00:59AM +0100, Gerd Hoffmann wrote:
> >> On Tue, Mar 02, 2021 at 05:55:23PM +0000, Daniel P. Berrangé wrote:
> >> > Currently the -audiodev accepts any audiodev type regardless of what is
> >> > built in to QEMU. An error only occurs later at runtime when a sound
> >> > device tries to use the audio backend.
> >> > 
> >> > With this change QEMU will immediately reject -audiodev args that are
> >> > not compiled into the binary. The QMP schema will also be introspectable
> >> > to identify what is compiled in.
> >> 
> >> Note that audio backends are modularized, so "compiled with
> >> CONFIG_AUDIO_ALSA" doesn't imply "alsa support is available".
> >
> > AFAIK, there's no way to handle this with QAPI schema reporting. We
> > can only conditionalize based on what's available at compile time,
> > not what's installed at runtime.
> 
> Correct.  The schema is fixed at compile-time.  query-qmp-schema
> reflects what we compiled into the binary or modules we built along with
> the binary.  It cannot tell which of the modules the binary can load.
> 
> I'd like the commit message to discuss how the patch interacts with
> modules, because my own memory of the details is rather uncertain :)
> 
> I guess we can configure which audio backends to build, and whether to
> build them as modules.
> 
> When not building them as modules, the patch compiles out some useless
> code.  Correct?

Yes.

> When building them as modules, the patch compiles out code for modules
> we don't build.  Correct?

Yes.

> Such code is useless, unless you somehow manage to supply the resulting
> binary with working modules from another build.  Which is probably a bad
> idea.  Compiling out the code stops this (questionable) usage cold.  No
> objection, but the commit message should at least hint at it.
> 
> > To get runtime info, we would have to introduce an explicit
> > "query-audiodev-types" command where just report the backends
> > that have been installed.
> 
> TTOCTOU.  Harmless one.  Still, the robust check for "can module M be
> loaded?" is to try loading it.

Ultimately from libvirt's POV, the introspection is merely used to
let libvirt report errors about unsupported configurations earlier.
So if we don't deal with compiled-but-not-installed modules, then
the effect is that libvirt won't report errors if user requests
'alsa', but QEMU will eventually report such an error when it
starts. So I'm not too worried about optimizing to cope with
modules.

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