[pve-devel] applied: [PATCH qemu-server] api: deletion: check also pending values for serial/usb

[Thomas Lamprecht]

On 9/23/19 10:50 AM, Dominik Csapak wrote:
> otherwise a user with only VM.Config.HWType cannot
> delete a 'pending' usbX: spice or serial: socket option
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/API2/Qemu.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 3355c8b..5814f94 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1202,7 +1202,7 @@ my $update_vm_api  = sub {
>  		    PVE::QemuServer::vmconfig_delete_pending_option($conf, $opt, $force);
>  		    PVE::QemuConfig->write_config($vmid, $conf);
>  		} elsif ($opt =~ m/^serial\d+$/) {
> -		    if ($conf->{$opt} eq 'socket') {
> +		    if ($conf->{$opt} eq 'socket' || (!$conf->{$opt} && $conf->{pending}->{$opt} eq 'socket')) {
>  			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
>  		    } elsif ($authuser ne 'root at pam') {
>  			die "only root can delete '$opt' config for real devices\n";
> @@ -1210,7 +1210,7 @@ my $update_vm_api  = sub {
>  		    PVE::QemuServer::vmconfig_delete_pending_option($conf, $opt, $force);
>  		    PVE::QemuConfig->write_config($vmid, $conf);
>  		} elsif ($opt =~ m/^usb\d+$/) {
> -		    if ($conf->{$opt} =~ m/spice/) {
> +		    if ($conf->{$opt} =~ m/spice/ || (!$conf->{$opt} && $conf->{pending}->{$opt} =~ m/spice/)) {
>  			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
>  		    } elsif ($authuser ne 'root at pam') {
>  			die "only root can delete '$opt' config for real devices\n";
> 

I'll get:
> Use of uninitialized value in pattern match (m//) at /usr/share/perl5/PVE/API2/Qemu.pm line 1213.

This was an issue before your patch (but from an older patch of yours), as
the if branch was hit the same way.

I followed up with a patch introducing a $val variable which is the config
option, or the pending option as fallback. So then we can check only that
single variable and make the if/else conditions a bit easier to read..
and it solves this issue as we've the invariant that either (config or pending)
option is defined here.