[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 4/6] conf: introduce <output> element for <sound> devices




On 11/21/2017 08:12 AM, Pavel Hrdina wrote:
> On Mon, Nov 20, 2017 at 06:04:27PM -0500, John Ferlan wrote:
>>
>>
>> On 11/14/2017 08:45 AM, Pavel Hrdina wrote:
>>> So far it was not possible to specify how the audio output from guest
>>
>> s/So far it was/It is/
>>
>>> should be presented to host/users.  Now it will be possible to do so
>>> via <output> element for <sound> device where you specify the output
>>> "type".
>>>
>>> Signed-off-by: Pavel Hrdina <phrdina redhat com>
>>> ---
>>>  docs/formatdomain.html.in     |  9 +++++++
>>>  docs/schemas/domaincommon.rng | 14 ++++++++++
>>>  src/conf/domain_conf.c        | 61 +++++++++++++++++++++++++++++++++++++++++++
>>>  src/conf/domain_conf.h        | 14 ++++++++++
>>>  src/libvirt_private.syms      |  2 ++
>>>  5 files changed, 100 insertions(+)
>>>
>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>> index 47c43d0666..3b7c367fc7 100644
>>> --- a/docs/formatdomain.html.in
>>> +++ b/docs/formatdomain.html.in
>>> @@ -7058,6 +7058,15 @@ qemu-kvm -net nic,model=? /dev/null
>>>        slot, <a href="#elementsAddress">documented above</a>.
>>>      </p>
>>>  
>>> +    <p>
>>> +      <span class="since">Since 3.10.0</span> sound device can have
>>
>> s/sound/, a sound/
>>
>>> +      an optional <code>output</code> element which configures where
>>> +      the audio output is connected within host. There is mandatory
>>> +      <code>type</code> attribute where valid values are 'none' to
>>> +      disable the audio output, 'spice', 'pa', 'sdl', 'alsa', 'oss'.
>>
>> I've become preferential to seeing these in a list. I have no idea based
>> on the text here what 'spice', ... 'oss' really means.  IOW: Why would I
>> want to set to something else - what does it do/provide?
> 
> I'm not sure how to describe it more than providing links to all the
> projects.  SPICE is the only unique output since the audio stream is
> sent over SPICE protocol, but the remaining outputs are IMHO
> self-explanatory because the are standard linux/unix audio
> layers/libraries.

I was reading literally vis-a-vis a list of supported values and there
was something missing. I was not thinking in terms that the list is/was
from the standard linux/unix audio layers/libraries. As I've learned
since originally looking at this - the overall design of graphics/sound
is a bit, well, odd/unusual...  Anwyay restated:

There is mandatory <code>type</code> attribute where valid values to
enable the audio output are 'spice', 'pa', 'sdl', 'alsa', or 'oss'. Use
'none' in order to disable the audio output.

With a bit more thought on this now - why would someone add a sound
card, but select an output of 'none'?  Why even add the sound card then?
You have a sound card that doesn't emit sound anywhere? Why have it?!

And in summary, it seems to me now that what the 'output' is designed to
do is supply the driver for the QEMU_AUDIO_DRV environment variable,
true? To override what may be globally set?


> 
> Describing all the differences is out of scope of our documentation.
> 

Hmmm.. maybe I wasn't clear enough - it wasn't the differences that I
cared about it was the what are those shorthand acronyms. Someone
reading the list literally may not know what they are. I certainly don't
think open sound system (I had to look it up)... So pa is perhaps a
known synonym for pulseaudio...

>>
>>> +      This might not be supported by all hypervisors.
>>
>> True, but perhaps also true of many other things, too.
> 
> I can change it to "Currently supported only by QEMU driver."
> 

OK, so this makes sense in light of what appears to be the goal - to
provide something for the env variable.


BTW: Probably should add the xml2xml parsing tests to this patch -
although they are present later - it is something important to ask of
any XML changes.


John

>>> +    </p>
>>> +
>>>      <h4><a id="elementsWatchdog">Watchdog device</a></h4>
>>>  
>>>      <p>
>>
>>
>> Reviewed-by: John Ferlan <jferlan redhat com>
>>
>> John
>>
>> [...]
>>
>> --
>> libvir-list mailing list
>> libvir-list redhat com
>> https://www.redhat.com/mailman/listinfo/libvir-list


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]