[pve-devel] applied: [PATCH qemu-server 1/2] move check for serialX: socket

[Thomas Lamprecht]

On 4/12/19 4:08 PM, Dominik Csapak wrote:
> since we do not want the user to change a device which has a real
> devices already set, we have to check it later when we have the config
> 
> so we do not have to give the params to vm_check_modify_config_perm anymore

argh, makes sense, should have seen that on review, anyway: applied, thanks!

> 
> also improve the regex to \d+
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/API2/Qemu.pm | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 7195ec2..0139de1 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -302,15 +302,16 @@ my $cloudinitoptions = {
>  };
>  
>  my $check_vm_modify_config_perm = sub {
> -    my ($rpcenv, $authuser, $vmid, $pool, $key_list, $values) = @_;
> +    my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_;
>  
>      return 1 if $authuser eq 'root at pam';
>  
>      foreach my $opt (@$key_list) {
> -	# disk checks need to be done somewhere else
> +	# some checks need to be done somewhere else
>  	next if PVE::QemuServer::is_valid_drivename($opt);
>  	next if $opt eq 'cdrom';
>  	next if $opt =~ m/^unused\d+$/;
> +	next if $opt =~ m/^serial\d+$/;
>  
>  	if ($cpuoptions->{$opt} || $opt =~ m/^numa\d+$/) {
>  	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.CPU']);
> @@ -330,14 +331,6 @@ my $check_vm_modify_config_perm = sub {
>  	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk']);
>  	} elsif ($cloudinitoptions->{$opt} || ($opt =~ m/^(?:net|ipconfig)\d+$/)) {
>  	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Network']);
> -	} elsif ($opt =~ m/^serial\d+$/) {
> -	    if ($values && $values->{$opt} eq 'socket') {
> -		$rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
> -	    } elsif (!$values) {
> -		next; # deletion will be checked later since we do not have the config here
> -	    } else {
> -		die "only root can set '$opt' config to real devices\n";
> -	    }
>  	} else {
>  	    # catches usb\d+, hostpci\d+, args, lock, etc.
>  	    # new options will be checked here
> @@ -525,7 +518,7 @@ __PACKAGE__->register_method({
>  
>  	    &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param, $storage);
>  
> -	    &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, $pool, [ keys %$param], $param);
> +	    &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, $pool, [ keys %$param]);
>  
>  	    foreach my $opt (keys %$param) {
>  		if (PVE::QemuServer::is_valid_drivename($opt)) {
> @@ -1135,7 +1128,7 @@ my $update_vm_api  = sub {
>  
>      &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, undef, [@delete]);
>  
> -    &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, undef, [keys %$param], $param);
> +    &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, undef, [keys %$param]);
>  
>      &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param);
>  
> @@ -1198,7 +1191,7 @@ my $update_vm_api  = sub {
>  			if defined($conf->{pending}->{$opt});
>  		    PVE::QemuServer::vmconfig_delete_pending_option($conf, $opt, $force);
>  		    PVE::QemuConfig->write_config($vmid, $conf);
> -		} elsif ($opt =~ m/^serial\d$/) {
> +		} elsif ($opt =~ m/^serial\d+$/) {
>  		    if ($conf->{$opt} eq 'socket') {
>  			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
>  		    } elsif ($authuser ne 'root at pam') {
> @@ -1231,6 +1224,13 @@ my $update_vm_api  = sub {
>  			if defined($conf->{pending}->{$opt});
>  
>  		    &$create_disks($rpcenv, $authuser, $conf->{pending}, $arch, $storecfg, $vmid, undef, {$opt => $param->{$opt}});
> +		} elsif ($opt =~ m/^serial\d+/) {
> +		    if ((!defined($conf->{$opt}) || $conf->{$opt} eq 'socket') && $param->{$opt} eq 'socket') {
> +			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +		    } elsif ($authuser ne 'root at pam') {
> +			die "only root can modify '$opt' config for real devices\n";
> +		    }
> +		    $conf->{pending}->{$opt} = $param->{$opt};
>  		} else {
>  		    $conf->{pending}->{$opt} = $param->{$opt};
>  		}
>