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

[Cole Robinson]

On 01/14/2011 02:05 PM, Daniel P. Berrange wrote:
> On Thu, Jan 13, 2011 at 10:45:41AM -0500, 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.
>>
>> 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
>>
>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>> ---
>>  docs/formatdomain.html.in                          |    5 ++-
>>  docs/schemas/domain.rng                            |    1 +
>>  src/conf/domain_conf.c                             |    3 +-
>>  src/conf/domain_conf.h                             |    1 +
>>  src/qemu/qemu_command.c                            |   24 ++++++++++++++++---
>>  .../qemuxml2argv-sound-device.args                 |    2 +-
>>  .../qemuxml2argvdata/qemuxml2argv-sound-device.xml |    1 +
>>  tests/qemuxml2argvdata/qemuxml2argv-sound.args     |    2 +-
>>  tests/qemuxml2argvdata/qemuxml2argv-sound.xml      |    1 +
>>  9 files changed, 31 insertions(+), 9 deletions(-)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index e9fcea1..b9b83c2 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -1663,8 +1663,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 9c1e9bb..ae0defe 100644
>> --- a/docs/schemas/domain.rng
>> +++ b/docs/schemas/domain.rng
>> @@ -1496,6 +1496,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 e5b89a2..3aabdf9 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -225,7 +225,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 81409f8..924ce89 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -458,6 +458,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_command.c b/src/qemu/qemu_command.c
>> index a0075a4..2024221 100644
>> --- a/src/qemu/qemu_command.c
>> +++ 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 */
>>      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);
>> @@ -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);
>> +                    }
> 
> To allow us to later add hotplug, we need to include an 'id'
> on this device. We should also set the codec slot number
> and explicitly reference the intel-hda sound card so that
> you can give multiple ich6 cards in one guest. Something
> like. eg for a device with a codec in slot 3, we'd do
> 
>    -device intel-hda,id=foo -device hda-duplex,id=codecfoo3,bus=foo,cad=3
> 
> Obviously hardcode slot 0 for now.
> 
> Even though we don't have hotplug now, we need all the IDs
> and cad properties wired up, so that if someone upgrades
> they can use hotplug without having to reboot all their
> guests.

Thanks, I'll address this in the next posting.

- Cole