[pve-devel] [PATCH qemu-server] audio dev: specify qemu audio backend

[Aaron Lauterer]

On 7/22/19 10:00 AM, Thomas Lamprecht wrote:
> On 7/19/19 3:15 PM, Aaron Lauterer wrote:
>> QEMU 4 adds the `-audiodev` parameter to explicitly specify the audio
>> backend. Setting it avoids occasional error messages when starting a
>> virtual machine with an audio device and qemu wants to connect it to the
>> physical audio device.
> 
> Occasional? The error is always there if no audio card/alsa is there.
> The real issue is that this would have allowed access to host HW with
> only having the VM.Config.Option permission on any VM..
> (no big issue, I just like to have the commit messages tell the real reasons)
> 

Managed to find a reproducible setup to cause the errors. Will rewrite 
the commit msg.

>>
>> For now only SPICE is supported as it's also the biggest use case.
> 
> 
> FYI, testing this in a VM I get the following errors/warnings:
> 
>> audio: Could not init `spice' audio driver
>> audio: warning: Using timer based audio emulation
> 
> but host seems to work...
> 

These errors are thrown if the "-audiodev spice" parameter is set but no 
spice (qxl) display is selected. If no spice display is selected the 
"-spice" parameter is not set.

The manpage for qemu states the following regarding "-audiodev spice":

 > Creates a backend that sends audio through SPICE.  This backend 
requires "-spice" and automatically selected in that case, so usually 
you can ignore this option.  This backend has no backend specific 
properties.

If spice(qxl) is selected as display there are no errors even if the 
"-audiodev spice" is not set.

To limit access to the host hardware I would still set the "-audiodev 
spice" for the cases that a user defines an audio device but does not 
set the display to spice(qxl).

What do you think? @Thomas

>>
>> Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
>> ---
>>   PVE/QemuServer.pm | 24 ++++++++++++++++++++++--
>>   1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index c852e04..e2c96e8 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -267,6 +267,21 @@ my $ivshmem_fmt = {
>>       },
>>   };
>>   
>> +my $audio_fmt = {
>> +    device => {
>> +	type => 'string',
>> +	enum => [qw(ich9-intel-hda intel-hda AC97)],
>> +	description =>  "Configure an audio device."
>> +    },
>> +    driver =>  {
>> +	type => 'string',
>> +	enum => ['spice'],
>> +	default => 'spice',
>> +	optional => 1,
>> +	description => "Driver backend for the audio device."
>> +    },
>> +};
>> +
>>   my $confdesc = {
>>       onboot => {
>>   	optional => 1,
>> @@ -640,7 +655,7 @@ EODESCR
>>       },
>>       audio0 => {
>>   	type => 'string',
>> -	enum => [qw(ich9-intel-hda intel-hda AC97)],
>> +	format => $audio_fmt,
>>   	description => "Configure a audio device, useful in combination with QXL/Spice.",
>>   	optional => 1
>>       },
>> @@ -3786,7 +3801,10 @@ sub config_to_command {
>>   	}
>>       }
>>   
>> -    if (my $audiodevice = $conf->{audio0}) {
>> +    if ($conf->{audio0}) {
> 
> why above change, maybe just change it to
> 
> if (my $audio0 = $conf->{audio0}) {
> 
>> +	my $audioproperties = PVE::JSONSchema::parse_property_string($audio_fmt, $conf->{audio0});
>> +	my $audiodevice = $audioproperties->{device};
>> +	my $audiodriver = $audioproperties->{driver} // 'spice';
> 
> above not needed? just use the hash directly?
> 
> my $audiodev = PVE::JSONSchema::parse_property_string($audio_fmt, $audio0);
> $audiodev->{driver} //= 'spice';
> 

Okay, I'll change that in the next version
> 
> 
>>   	my $audiopciaddr = print_pci_addr("audio0", $bridges, $arch, $machine_type);
>>   
>>   	if ($audiodevice eq 'AC97') {
>> @@ -3798,6 +3816,8 @@ sub config_to_command {
>>   	} else {
>>   	    die "unkown audio device '$audiodevice', implement me!";
>>   	}
>> +
>> +	push @$devices, '-audiodev', "${audiodriver},id=${audiodriver}-driver";
> 
> doesn't the ID of the main -device above? Or is this independent?
> 

I could not find any doc regarding the need for a matching id or a 
different behavior. If someone can shed more light onto this I would 
appreciate it.

>>       }
>>   
>>       my $sockets = 1;
>>
>