qemu: sound: Support intel 'ich6' model

In QEMU, the card itself is a PCI device, but it requires a codec
(either -device hda-output or -device hda-duplex) to actually output
sound. Specifying <sound model='ich6'/> gives us -device intel-hda
-device hda-duplex I think it's important that a simple <sound model='ich6'/>
sets up a useful codec, to have consistent behavior with all other sound cards.

This is basically Dan's proposal of

    <sound model='ich6'>
        <codec type='output' slot='0'/>
        <codec type='duplex' slot='3'/>
    </sound>

without the codec bits implemented.

The important thing is to keep a consistent API here, we don't want some
<sound> devs require tweaking codecs but not others. Steps I see to
accomplishing this:

    - every <sound> device has a <codec type='default'/> (unless codecs are
        manually specified)
    - <codec type='none'/> is required to specify 'no codecs'
    - new audio settings like mic=on|off could then be exposed in
        <sound> or <codec> in a consistent manner for all sound models

v2:
    Use model='ich6'

v3:
    Use feature detection, from eblake
    Set codec id, bus, and cad values

v4:
    intel-hda isn't supported if -device isn't available

v5:
    Comment spelling fixes

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 859f3fdf50a3fbba67e4dc94236bd6bd279cb012..e7f65ade537c72e5ed56df75e78a6770b798e43a 100644 (file)
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1880,8 +1880,9 @@ qemu-kvm -net nic,model=? /dev/null
         The <code>sound</code> element has one mandatory attribute,
         <code>model</code>, which specifies what real sound device is emulated.
         Valid values are specific to the underlying hypervisor, though typical
-        choices are 'es1370', 'sb16', and 'ac97'
-        (<span class="since">'ac97' only since 0.6.0</span>)
+        choices are 'es1370', 'sb16', 'ac97', and 'ich6'
+        (<span class="since">
+         'ac97' only since 0.6.0, 'ich6' only since 0.8.8</span>)
       </dd>
     </dl>
 

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index d4756e6e923a0d1de9752ac294df60e8f244e101..b2f8e0069a24006f0be91441e577ab60c6919227 100644 (file)
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -1530,6 +1530,7 @@
           <value>es1370</value>
           <value>pcspk</value>
           <value>ac97</value>
+          <value>ich6</value>
         </choice>
       </attribute>
       <optional>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 699fee750ae53e5030c1c914061370fa46f3dca9..d5445a40a0e0e5f269f9adb7e5f6d474a84f4f8f 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -231,7 +231,8 @@ VIR_ENUM_IMPL(virDomainSoundModel, VIR_DOMAIN_SOUND_MODEL_LAST,
               "sb16",
               "es1370",
               "pcspk",
-              "ac97")
+              "ac97",
+              "ich6")
 
 VIR_ENUM_IMPL(virDomainMemballoonModel, VIR_DOMAIN_MEMBALLOON_MODEL_LAST,
               "virtio",

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3b00ba0cd8bb6b585561ee9eaddc13f8d654f128..fc69627cfd092b2dfb0b61ccf775495521d6c9b3 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -480,6 +480,7 @@ enum virDomainSoundModel {
     VIR_DOMAIN_SOUND_MODEL_ES1370,
     VIR_DOMAIN_SOUND_MODEL_PCSPK,
     VIR_DOMAIN_SOUND_MODEL_AC97,
+    VIR_DOMAIN_SOUND_MODEL_ICH6,
 
     VIR_DOMAIN_SOUND_MODEL_LAST
 };

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index faf7d441f37f6d12079c2ac4b9e0eb88ad8f4011..02865e41fe36b470a1f504066d91ce4ff474176a 100644 (file)
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1079,6 +1079,11 @@ cleanup:
 int
 qemuCapsParseDeviceStr(const char *str, unsigned long long *flags)
 {
+    /* Which devices exist. */
+    if (strstr(str, "name \"hda-duplex\""))
+        *flags |= QEMUD_CMD_FLAG_HDA_DUPLEX;
+
+    /* Features of given devices. */
     if (strstr(str, "pci-assign.configfd"))
         *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD;
     if (strstr(str, "virtio-blk-pci.bootindex"))

diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e9e2da0d42058d2f03443daf2011067607fa04e6..9148d9085167d6ef16c8826b6a9787b65ee643dc 100644 (file)
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -84,6 +84,7 @@ enum qemuCapsFlags {
     QEMUD_CMD_FLAG_VGA_NONE      = (1LL << 47), /* The 'none' arg for '-vga' */
     QEMUD_CMD_FLAG_MIGRATE_QEMU_FD = (1LL << 48), /* -incoming fd:n */
     QEMUD_CMD_FLAG_BOOTINDEX     = (1LL << 49), /* -device bootindex property */
+    QEMUD_CMD_FLAG_HDA_DUPLEX    = (1LL << 50), /* -device hda-duplex */
 };
 
 virCapsPtr qemuCapsInit(virCapsPtr old_caps);

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c20f03120ede7af25a87fdf030f0133572e2714b..6dc586f9168594b57b99b924ff8647e63466c5ab 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1774,11 +1774,13 @@ qemuBuildSoundDevStr(virDomainSoundDefPtr sound)
         goto error;
     }
 
-    /* Hack for 2 wierdly unusal devices name in QEMU */
+    /* Hack for weirdly unusual devices name in QEMU */
     if (STREQ(model, "es1370"))
         model = "ES1370";
     else if (STREQ(model, "ac97"))
         model = "AC97";
+    else if (STREQ(model, "ich6"))
+        model = "intel-hda";
 
     virBufferVSprintf(&buf, "%s", model);
     virBufferVSprintf(&buf, ",id=%s", sound->info.alias);
@@ -1797,6 +1799,29 @@ error:
     return NULL;
 }
 
+static char *
+qemuBuildSoundCodecStr(virDomainSoundDefPtr sound,
+                       const char *codec)
+{
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    int cad = 0;
+
+    virBufferVSprintf(&buf, "%s", codec);
+    virBufferVSprintf(&buf, ",id=%s-codec%d", sound->info.alias, cad);
+    virBufferVSprintf(&buf, ",bus=%s.0", sound->info.alias);
+    virBufferVSprintf(&buf, ",cad=%d", cad);
+
+    if (virBufferError(&buf)) {
+        virReportOOMError();
+        goto error;
+    }
+
+    return virBufferContentAndReset(&buf);
+
+error:
+    virBufferFreeAndReset(&buf);
+    return NULL;
+}
 
 static char *
 qemuBuildVideoDevStr(virDomainVideoDefPtr video)
@@ -3817,11 +3842,29 @@ qemuBuildCommandLine(virConnectPtr conn,
                     virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL);
                 } else {
                     virCommandAddArg(cmd, "-device");
-
                     if (!(str = qemuBuildSoundDevStr(sound)))
                         goto error;
 
                     virCommandAddArg(cmd, str);
+
+                    if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6) {
+                        char *codecstr = NULL;
+                        if (!(qemuCmdFlags & QEMUD_CMD_FLAG_HDA_DUPLEX)) {
+                            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                                    _("this QEMU binary lacks hda support"));
+                            goto error;
+                        }
+
+                        virCommandAddArg(cmd, "-device");
+                        if (!(codecstr = qemuBuildSoundCodecStr(sound,
+                                                            "hda-duplex"))) {
+                            goto error;
+                        }
+
+                        virCommandAddArg(cmd, codecstr);
+                        VIR_FREE(codecstr);
+                    }
+
                     VIR_FREE(str);
                 }
             }
@@ -3840,6 +3883,13 @@ qemuBuildCommandLine(virConnectPtr conn,
                                     "%s", _("invalid sound model"));
                     goto error;
                 }
+
+                if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6) {
+                    qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                                    _("this QEMU binary lacks hda support"));
+                    goto error;
+                }
+
                 strncat(modstr, model, size);
                 size -= strlen(model);
                 if (i < (def->nsounds - 1))
@@ -5675,6 +5725,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
                     type = VIR_DOMAIN_SOUND_MODEL_ES1370;
                 } else if (STRPREFIX(start, "ac97")) {
                     type = VIR_DOMAIN_SOUND_MODEL_AC97;
+                } else if (STRPREFIX(start, "hda")) {
+                    type = VIR_DOMAIN_SOUND_MODEL_ICH6;
                 }
 
                 if (type != -1) {

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
index 6b2e6972369e8df268650254333e00c4eb5abc4a..4c5172fcf298a1c844f6b00325646bc93cb2e2aa 100644 (file)
--- a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
@@ -1 +1 @@
-LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -usb -soundhw pcspk -device ES1370,id=sound1,bus=pci.0,addr=0x2 -device sb16,id=sound2 -device AC97,id=sound3,bus=pci.0,addr=0x3 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -usb -soundhw pcspk -device ES1370,id=sound1,bus=pci.0,addr=0x2 -device sb16,id=sound2 -device AC97,id=sound3,bus=pci.0,addr=0x3 -device intel-hda,id=sound4,bus=pci.0,addr=0x4 -device hda-duplex,id=sound4-codec0,bus=sound4.0,cad=0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml
index c7253462b2c6337a09677556b374c33b6fce07f1..fbca4fe8902178cb86285412f624711be141e4bc 100644 (file)
--- a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml
@@ -22,6 +22,7 @@
     <sound model='es1370'/>
     <sound model='sb16'/>
     <sound model='ac97'/>
+    <sound model='ich6'/>
     <memballoon model='virtio'/>
   </devices>
 </domain>

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a7270ecc4273e2491b822eb14da2ad412a64bd94..0c0693b7be63e56c0e3703bc9f5d6c895d99fa03 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -418,7 +418,7 @@ mymain(int argc, char **argv)
             QEMUD_CMD_FLAG_NODEFCONFIG, false);
     DO_TEST("sound", 0, false);
     DO_TEST("sound-device", QEMUD_CMD_FLAG_DEVICE |
-            QEMUD_CMD_FLAG_NODEFCONFIG, false);
+            QEMUD_CMD_FLAG_NODEFCONFIG | QEMUD_CMD_FLAG_HDA_DUPLEX, false);
     DO_TEST("fs9p", QEMUD_CMD_FLAG_DEVICE |
             QEMUD_CMD_FLAG_NODEFCONFIG | QEMUD_CMD_FLAG_FSDEV, false);