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@xxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list