[pve-devel] [PATCH qemu-server 3/3] add error handling to vmconfig_apply_pending

[Thomas Lamprecht]

On 11/26/19 6:59 PM, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
>  PVE/API2/Qemu.pm  |  6 +++---
>  PVE/QemuServer.pm | 46 ++++++++++++++++++++++++++++++----------------
>  2 files changed, 33 insertions(+), 19 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 1811795..f291f52 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1222,13 +1222,13 @@ my $update_vm_api  = sub {
>  
>  	    $conf = PVE::QemuConfig->load_config($vmid); # update/reload
>  
> +	    my $errors = {};
>  	    if ($running) {
> -		my $errors = {};
>  		PVE::QemuServer::vmconfig_hotplug_pending($vmid, $conf, $storecfg, $modified, $errors);
> -		raise_param_exc($errors) if scalar(keys %$errors);
>  	    } else {
> -		PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running);
> +		PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running, $errors);
>  	    }
> +	    raise_param_exc($errors) if scalar(keys %$errors);
>  
>  	    return;
>  	};
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 7d2959a..70e7017 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4971,7 +4971,14 @@ sub vmconfig_delete_or_detach_drive {
>  
>  
>  sub vmconfig_apply_pending {
> -    my ($vmid, $conf, $storecfg) = @_;
> +    my ($vmid, $conf, $storecfg, $errors) = @_;
> +
> +    my $add_apply_error = sub {
> +	my ($opt, $msg) = @_;
> +	my $err_msg = "unable to apply pending change $opt : $msg";
> +	$errors->{$opt} = $err_msg;
> +	warn $err_msg;
> +    };
>  
>      # cold plug
>  
> @@ -4979,30 +4986,37 @@ sub vmconfig_apply_pending {
>      foreach my $opt (sort keys %$pending_delete_hash) {
>  	die "internal error" if $opt =~ m/^unused/;

this die needs to move in the eval, else the "only write config out once"
logic is flawed.

>  	my $force = $pending_delete_hash->{$opt}->{force};
> -	if (!defined($conf->{$opt})) {
> -	    PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
> -	} elsif (is_valid_drivename($opt)) {
> -	    vmconfig_delete_or_detach_drive($vmid, $storecfg, $conf, $opt, $force);
> -	    PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
> -	    delete $conf->{$opt};
> +	eval {
> +	    if (!defined($conf->{$opt})) {
> +		PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
> +	    } elsif (is_valid_drivename($opt)) {
> +		vmconfig_delete_or_detach_drive($vmid, $storecfg, $conf, $opt, $force);
> +	    }
> +	};
> +	if (my $err = $@) {
> +	    $add_apply_error->($opt, $err);
>  	} else {
>  	    PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
>  	    delete $conf->{$opt};
>  	}
>      }
>  
> -    foreach my $opt (keys %{$conf->{pending}}) { # add/change
> -	if (defined($conf->{$opt}) && ($conf->{$opt} eq $conf->{pending}->{$opt})) {
> -	    # skip if nothing changed
> -	} elsif (is_valid_drivename($opt)) {
> -	    vmconfig_register_unused_drive($storecfg, $vmid, $conf, parse_drive($opt, $conf->{$opt}))
> +    foreach my $opt (sort keys %{$conf->{pending}}) { # add/change
> +	next if $opt eq 'delete'; # just to be sure
> +	eval {
> +	    if (defined($conf->{$opt}) && ($conf->{$opt} eq $conf->{pending}->{$opt})) {
> +		# skip if nothing changed
> +	    } elsif (is_valid_drivename($opt)) {
> +		vmconfig_register_unused_drive($storecfg, $vmid, $conf, parse_drive($opt, $conf->{$opt}))
>  		if defined($conf->{$opt});
> -	    $conf->{$opt} = $conf->{pending}->{$opt};
> +	    }
> +	};
> +	if (my $err = $@) {
> +	    $add_apply_error->($opt, $err);
>  	} else {
> -	    $conf->{$opt} = $conf->{pending}->{$opt};
> +	    PVE::QemuConfig->cleanup_pending($conf);
> +	    $conf->{$opt} = delete $conf->{pending}->{$opt};
>  	}
> -
> -	delete $conf->{pending}->{$opt};
>      }
>  
>      PVE::QemuConfig->write_config($vmid, $conf);
>