On 01/13/2011 03:21 PM, Eric Blake wrote:
> 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/
>
Will fix.
>> @@ -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.
>
Ideally we would just rely on qemu to report a useful error in this
case, but instead it gives us:
$ x86_64-softmmu/qemu-system-x86_64 -device foobar
qemu-system-x86_64: -device foobar: Parameter 'driver' expects a driver name
Try with argument '?' for a list.
I consider that a qemu bug though. I've filed a report against RHEL6:
https://bugzilla.redhat.com/show_bug.cgi?id=669524
I'd rather error out than just ignore the unsupported device, so I don't
think a capabilities check buys us much besides working around a
suboptimal error message (which will hopefully be fixed soon anyways)
Thanks,
Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list