On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
> Setting the default audio output depends on specific graphic device
> but requires having sound device configured as well and it's the sound
> device that handles the audio.
>
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
> src/qemu/qemu_command.c | 84 +++++++++++++++-------
> .../qemuxml2argv-clock-france.args | 2 +-
> 2 files changed, 58 insertions(+), 28 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index eb72db33ba..e1ef1b05fa 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4442,10 +4442,62 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound,
> }
> >
> +static void
> +qemuBuildSoundAudioEnv(virCommandPtr cmd,
> + const virDomainDef *def,
> + virQEMUDriverConfigPtr cfg)
> +{
> + if (def->ngraphics == 0) {
> + if (cfg->nogfxAllowHostAudio)
> + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
> + else
> + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
> + } else {
> + switch (def->graphics[def->ngraphics - 1]->type) {
So it's the "last" graphics device that then defines "how" this all
works? Makes sense for QEMU_AUDIO_DRV since whichever is last would be
the winner and get the chicken dinner, but...
> + case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
> + /* 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
> + */
> + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
> + virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
... if there was more than one graphics device defined, where one was
SDL and it wasn't the last one, then theoretically at least this would
not be defined.
I think it's easily fixable...
> +
> + break;
> +
> + case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
> + /* Unless user requested it, set the audio backend to none, to
> + * prevent it opening the host OS audio devices, since that causes
> + * security issues and might not work when using VNC.
> + */
> + if (cfg->vncAllowHostAudio)
> + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
> + else
> + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
> +
> + break;
> +
> + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
> + /* SPICE includes native support for tunnelling audio, so we
> + * set the audio backend to point at SPICE's own driver
> + */
> + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice");
> +
> + break;
> +
> + case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
> + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
> + case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
> + break;
> + }
> + }
> +}
> +
> +
> static int
> qemuBuildSoundCommandLine(virCommandPtr cmd,
> const virDomainDef *def,
> - virQEMUCapsPtr qemuCaps)
> + virQEMUCapsPtr qemuCaps,
> + virQEMUDriverConfigPtr cfg)
> {
> size_t i, j;
>
> @@ -4498,6 +4550,9 @@ qemuBuildSoundCommandLine(virCommandPtr cmd,
> }
> }
> }
> +
> + qemuBuildSoundAudioEnv(cmd, def, cfg);
> +
> return 0;
> }
>
> @@ -7951,15 +8006,6 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
> if (graphics->data.vnc.keymap)
> virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap, NULL);
>
> - /* Unless user requested it, set the audio backend to none, to
> - * prevent it opening the host OS audio devices, since that causes
> - * security issues and might not work when using VNC.
> - */
> - if (cfg->vncAllowHostAudio)
> - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
> - else
> - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
> -
> return 0;
>
> error:
> @@ -8201,10 +8247,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
> if (graphics->data.spice.keymap)
> virCommandAddArgList(cmd, "-k",
> graphics->data.spice.keymap, NULL);
> - /* SPICE includes native support for tunnelling audio, so we
> - * set the audio backend to point at SPICE's own driver
> - */
> - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice");
>
> return 0;
>
> @@ -8235,13 +8277,6 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
> if (graphics->data.sdl.fullscreen)
> virCommandAddArg(cmd, "-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
> - */
> - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
> - virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
... perhaps the fix to above is to just keep SDL_AUDIODRIVER here, just
in case there's more than one graphics device and SDL isn't last.
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
John
> -
> /* New QEMU has this flag to let us explicitly ask for
> * SDL graphics. This is better than relying on the
> * default, since the default changes :-( */
> @@ -9995,11 +10030,6 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
> } else {
> virCommandAddArg(cmd, "-nographic");
> }
> -
> - if (cfg->nogfxAllowHostAudio)
> - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
> - else
> - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
> }
>
> /* Disable global config files and default devices */
> @@ -10083,7 +10113,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
> if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0)
> goto error;
>
> - if (qemuBuildSoundCommandLine(cmd, def, qemuCaps) < 0)
> + if (qemuBuildSoundCommandLine(cmd, def, qemuCaps, cfg) < 0)
> goto error;
>
> if (qemuBuildWatchdogCommandLine(cmd, def, qemuCaps) < 0)
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args
> index 9bde6d967b..2701179273 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args
> @@ -3,8 +3,8 @@ PATH=/bin \
> HOME=/home/test \
> USER=test \
> LOGNAME=test \
> -QEMU_AUDIO_DRV=none \
> TZ=Europe/Paris \
> +QEMU_AUDIO_DRV=none \
> /usr/bin/qemu-system-i686 \
> -name QEMUGuest1 \
> -S \
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
