Re: [libvirt] [PATCH] Allow control over QEMU audio backend

On Tue, Sep 22, 2009 at 11:45:02AM +0100, Daniel P. Berrange wrote: > When using VNC for graphics + keyboard + mouse, we shouldn't > then use the host OS for audio. Audio should go back over > VNC. > > When using SDL for graphics, we should use the host OS for > audio since that's where the display is. We need to allow > certain QEMU env variables to be passed through to guest > too to allow choice of QEMU audio backend. All this makes sense ! > * qemud/libvirtd.sysconf: Mention QEMU/SDL audio env vars > * src/qemu_conf.c: Passthrough QEMU/SDL audio env for SDL display, > disable host audio for VNC display > --- > daemon/libvirtd.sysconf | 8 ++++++++ > src/qemu/qemu_conf.c | 14 ++++++++++++++ > 2 files changed, 22 insertions(+), 0 deletions(-) > > diff --git a/daemon/libvirtd.sysconf b/daemon/libvirtd.sysconf > index fe4596a..28080a0 100644 > --- a/daemon/libvirtd.sysconf > +++ b/daemon/libvirtd.sysconf > @@ -7,3 +7,11 @@ > > # Override Kerberos service keytab for SASL/GSSAPI > #KRB5_KTNAME=/etc/libvirt/krb5.tab > + > +# Override the QEMU/SDL default audio driver probing when > +# starting virtual machines using SDL graphics > +# > +# NB these have no effect for VMs using VNC > +#QEMU_AUDIO_DRV=sdl > +# > +#SDL_AUDIODRIVER=pulse > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c > index 273aeca..74ca52b 100644 > --- a/src/qemu/qemu_conf.c > +++ b/src/qemu/qemu_conf.c > @@ -2109,6 +2109,13 @@ int qemudBuildCommandLine(virConnectPtr conn, > ADD_ARG_LIT("-k"); > ADD_ARG_LIT(def->graphics[0]->data.vnc.keymap); > } > + > + /* QEMU implements a VNC extension for providing audio, so we > + * set the audio backend to none, to prevent it opening the > + * host OS audio devices since that causes security issues > + * and is non-sensical when using VNC. > + */ > + ADD_ENV_LIT("QEMU_AUDIO_DRV=none"); > } else if ((def->ngraphics == 1) && > def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { > char *xauth = NULL; > @@ -2131,6 +2138,13 @@ int qemudBuildCommandLine(virConnectPtr conn, > ADD_ENV(display); > if (def->graphics[0]->data.sdl.fullscreen) > ADD_ARG_LIT("-full-screen"); > + > + /* If using SDL for video, then we should just let it > + * use QEMU's host audio drivers, possibly SDL too > + * User can set these two before starting libvirtd > + */ > + ADD_ENV_COPY("QEMU_AUDIO_DRV"); > + ADD_ENV_COPY("SDL_AUDIODRIVER"); > } > > if (def->nvideos) { ACK, I wonder if that would be sufficient to drop the libvirt-0.6.4-svirt-sound.patch that we carry in Fedora, not sure because we still may use the local audio in some cases. Opinion ?