[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 5/6] qemu: implement <output> element for <sound> devices



So far we were configuring the sound output based on what graphic device
was configured in domain XML.  This had a several disadvantages, it's
not transparent, in case of multiple graphic devices it was overwritten
by the last one and there was no simple way how to configure this per
domain.

The new <output> element for <sound> devices allows you to configure
which output will be used for each domain, however QEMU has a limitation
that all sound devices will always use the same output because it is
configured by environment variable QEMU_AUDIO_DRV per domain.

For backward compatibility we need to preserve the defaults if no output
is specified:

  - for vnc graphic it's by default NONE unless "vnc_allow_host_audio"
    was enabled, in that case we use DEFAULT which means it will pass
    the environment variable visible by libvirtd

  - for sdl graphic by default we pass the environment variable

  - for spice graphic we configure the SPICE output

  - if no graphic is configured we use by default NONE unless
    "nogfx_allow_host_audio" was enabled, in that case we pass
    the environment variable

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375433

Signed-off-by: Pavel Hrdina <phrdina redhat com>
---
 docs/formatdomain.html.in |  4 ++-
 src/qemu/qemu_command.c   | 84 +++++++++++++++++++++--------------------------
 src/qemu/qemu_domain.c    | 54 ++++++++++++++++++++++++++++++
 src/qemu/qemu_process.c   | 41 +++++++++++++++++++++++
 4 files changed, 135 insertions(+), 48 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3b7c367fc7..ae0d8b86be 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7064,7 +7064,9 @@ qemu-kvm -net nic,model=? /dev/null
       the audio output is connected within host. There is mandatory
       <code>type</code> attribute where valid values are 'none' to
       disable the audio output, 'spice', 'pa', 'sdl', 'alsa', 'oss'.
-      This might not be supported by all hypervisors.
+      This might not be supported by all hypervisors. QEMU driver
+      has a limitation that all sound devices have to use the same
+      output.
     </p>
 
     <h4><a id="elementsWatchdog">Watchdog device</a></h4>
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c5c7bd7e54..5cbd1d0d46 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4442,67 +4442,57 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound,
 }
 
 
-static void
+static int
 qemuBuildSoundAudioEnv(virCommandPtr cmd,
-                       const virDomainDef *def,
-                       virQEMUDriverConfigPtr cfg)
+                       const virDomainDef *def)
 {
+    char *envStr = NULL;
+
     if (def->nsounds == 0) {
         virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
-        return;
+        return 0;
     }
 
-    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) {
-        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);
+    /* QEMU doesn't allow setting different audio output per sound device
+     * so it will always be the same for all devices. */
+    switch (def->sounds[0]->output) {
+    case VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT:
+        /* The default output is used only as backward compatible way to
+         * pass-through environment variables configured before starting
+         * libvirtd. */
+        virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
+        if (def->ngraphics > 0 &&
+            def->graphics[def->ngraphics - 1]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
             virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
+        }
+        break;
 
-            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;
+    case VIR_DOMAIN_SOUND_OUTPUT_TYPE_NONE:
+    case VIR_DOMAIN_SOUND_OUTPUT_TYPE_SPICE:
+    case VIR_DOMAIN_SOUND_OUTPUT_TYPE_PA:
+    case VIR_DOMAIN_SOUND_OUTPUT_TYPE_SDL:
+    case VIR_DOMAIN_SOUND_OUTPUT_TYPE_ALSA:
+    case VIR_DOMAIN_SOUND_OUTPUT_TYPE_OSS:
+        if (virAsprintf(&envStr, "QEMU_AUDIO_DRV=%s",
+                        virDomainSoundOutputTypeToString(def->sounds[0]->output)) < 0) {
+            return -1;
         }
+        virCommandAddEnvString(cmd, envStr);
+        VIR_FREE(envStr);
+        break;
+
+    case VIR_DOMAIN_SOUND_OUTPUT_TYPE_LAST:
+        break;
     }
+
+    return 0;
 }
 
 
 static int
 qemuBuildSoundCommandLine(virCommandPtr cmd,
                           const virDomainDef *def,
-                          virQEMUCapsPtr qemuCaps,
-                          virQEMUDriverConfigPtr cfg)
+                          virQEMUCapsPtr qemuCaps)
 {
     size_t i, j;
 
@@ -4556,7 +4546,7 @@ qemuBuildSoundCommandLine(virCommandPtr cmd,
         }
     }
 
-    qemuBuildSoundAudioEnv(cmd, def, cfg);
+    qemuBuildSoundAudioEnv(cmd, def);
 
     return 0;
 }
@@ -10118,7 +10108,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
     if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0)
         goto error;
 
-    if (qemuBuildSoundCommandLine(cmd, def, qemuCaps, cfg) < 0)
+    if (qemuBuildSoundCommandLine(cmd, def, qemuCaps) < 0)
         goto error;
 
     if (qemuBuildWatchdogCommandLine(cmd, def, qemuCaps) < 0)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a36e157529..3b8fa2d79c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3148,6 +3148,31 @@ qemuDomainDefVerifyFeatures(const virDomainDef *def)
 }
 
 
+static void
+qemuDomainDefSoundPostParse(virDomainDefPtr def)
+{
+    size_t i;
+    virDomainSoundOutputType output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT;
+
+    for (i = 0; i < def->nsounds; i++) {
+        if (output != def->sounds[i]->output) {
+            output = def->sounds[i]->output;
+            break;
+        }
+    }
+
+    /* For convenience we will copy the first configured sound output to all
+     * sound devices that doesn't have any output configured because QEMU
+     * will use only one output for all sound devices. */
+    if (output != VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) {
+        for (i = 0; i < def->nsounds; i++) {
+            if (def->sounds[i]->output == VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT)
+                def->sounds[i]->output = output;
+        }
+    }
+}
+
+
 static int
 qemuDomainDefPostParseBasic(virDomainDefPtr def,
                             virCapsPtr caps,
@@ -3221,6 +3246,8 @@ qemuDomainDefPostParse(virDomainDefPtr def,
     if (qemuDomainDefCPUPostParse(def) < 0)
         goto cleanup;
 
+    qemuDomainDefSoundPostParse(def);
+
     ret = 0;
  cleanup:
     virObjectUnref(cfg);
@@ -3301,6 +3328,30 @@ qemuDomainDefValidateVideo(const virDomainDef *def)
 }
 
 
+static int
+qemuDomainDefValidateSound(const virDomainDef *def)
+{
+    size_t i;
+    virDomainSoundOutputType output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT;
+
+    for (i = 0; i < def->nsounds; i++) {
+        if (output == VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT) {
+            output = def->sounds[i]->output;
+            continue;
+        }
+
+        if (output != def->sounds[i]->output) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("all sound devices must be configured to use "
+                             "the same output"));
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+
 #define QEMU_MAX_VCPUS_WITHOUT_EIM 255
 
 
@@ -3403,6 +3454,9 @@ qemuDomainDefValidate(const virDomainDef *def,
     if (qemuDomainDefValidateVideo(def) < 0)
         goto cleanup;
 
+    if (qemuDomainDefValidateSound(def) < 0)
+        goto cleanup;
+
     ret = 0;
 
  cleanup:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6d242b1b51..2957c4a074 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5393,6 +5393,45 @@ qemuProcessPrepareAllowReboot(virDomainObjPtr vm)
 }
 
 
+static void
+qemuProcessPrepareSound(virDomainDefPtr def,
+                        virQEMUDriverConfigPtr cfg)
+{
+    virDomainSoundOutputType output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT;
+    size_t i;
+
+    if (def->nsounds == 0)
+        return;
+
+    if (def->ngraphics == 0) {
+        if (!cfg->nogfxAllowHostAudio)
+            output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_NONE;
+    } else {
+        switch (def->graphics[def->ngraphics - 1]->type) {
+        case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+            output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_SPICE;
+            break;
+
+        case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
+            if (!cfg->vncAllowHostAudio)
+                output = VIR_DOMAIN_SOUND_OUTPUT_TYPE_NONE;
+            break;
+
+        case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+        case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
+        case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
+        case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
+            break;
+        }
+    }
+
+    for (i = 0; i < def->nsounds; i++) {
+        if (def->sounds[i]->output == VIR_DOMAIN_SOUND_OUTPUT_TYPE_DEFAULT)
+            def->sounds[i]->output = output;
+    }
+}
+
+
 /**
  * qemuProcessPrepareDomain:
  * @conn: connection object (for looking up storage volumes)
@@ -5513,6 +5552,8 @@ qemuProcessPrepareDomain(virConnectPtr conn,
             goto cleanup;
     }
 
+    qemuProcessPrepareSound(vm->def, cfg);
+
     ret = 0;
  cleanup:
     virObjectUnref(caps);
-- 
2.13.6


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]