[libvirt] [PATCH v2] qemu: sound: Support intel 'ich6' model

[Eric Blake]

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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110113/9b806bf2/attachment-0001.sig>