On 01/13/2011 08:45 AM, Cole Robinson wrote:
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. We set up an hda-duplex codec by default: 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.
Reasonable for a first round of patches.
The important thing is to keep a consistent API here, we don't want some
<sound> models to require tweaking codecs but not others. Steps I see to
accomplishing this when someone gets around to it:
- 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
Agree that those are good steps forward, and that they do not hold up
this patch.
+++ b/src/qemu/qemu_command.c
@@ -1727,11 +1727,13 @@ qemuBuildSoundDevStr(virDomainSoundDefPtr sound)
goto error;
}
- /* Hack for 2 wierdly unusal devices name in QEMU */
+ /* Hack for wierdly unusal devices name in QEMU */
As long as you're touching the comment: s/wierdly/weirdly/
@@ -3751,6 +3753,12 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
virCommandAddArg(cmd, str);
+
+ if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6) {
+ virCommandAddArgList(cmd,
+ "-device",
"hda-duplex", NULL);
+ }
+
Should this come with a qemu_capabilities.[ch] check that device
hda-duplex is available? Or are we relying on the fact that qemu will
exit with a sane error message if an unsupported device is requested?
The patch looks fine to me once you fix the spelling nit, but I'd rather
get an answer to whether qemu_capabilities should be changed before
giving ack.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org