[pve-devel] [PATCH qemu-server 4/4] apply_pending: add error handling and remove redundant config calls

[Oguz Bektas]

* add $errors parameter and error handling code to vmconfig_apply_pending.

* replace redundant write/load config calls with a single write_config
at the end

Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
---
 PVE/API2/Qemu.pm  |  6 ++---
 PVE/QemuServer.pm | 60 +++++++++++++++++++++++++----------------------
 2 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index b2c0b0d..a261bf6 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1226,13 +1226,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, $errors);
 	    }
+	    raise_param_exc($errors) if scalar(keys %$errors);
 
 	    return;
 	};
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 3855f6e..bc4a388 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5038,48 +5038,52 @@ sub vmconfig_delete_or_detach_drive {
 
 
 sub vmconfig_apply_pending {
-    my ($vmid, $conf, $storecfg) = @_;
+    my ($vmid, $conf, $storecfg, $errors) = @_;
 
-    # cold plug
+    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;
+    };
 
     my $pending_delete_hash = PVE::QemuConfig->parse_pending_delete($conf->{pending}->{delete});
     foreach my $opt (sort keys %$pending_delete_hash) {
 	die "internal error" if $opt =~ m/^unused/;
 	my $force = $pending_delete_hash->{$opt}->{force};
-	$conf = PVE::QemuConfig->load_config($vmid); # update/reload
-	if (!defined($conf->{$opt})) {
-	    PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
-	    PVE::QemuConfig->write_config($vmid, $conf);
-	} 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};
-	    PVE::QemuConfig->write_config($vmid, $conf);
+	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);
+	    PVE::QemuConfig->cleanup_pending($conf);
 	    delete $conf->{$opt};
-	    PVE::QemuConfig->write_config($vmid, $conf);
+	    PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
 	}
     }
 
-    $conf = PVE::QemuConfig->load_config($vmid); # update/reload
-
-    foreach my $opt (keys %{$conf->{pending}}) { # add/change
-	$conf = PVE::QemuConfig->load_config($vmid); # update/reload
-
-	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};
+    foreach my $opt (sort keys %{$conf->{pending}}) { # add/change
+	next if $opt eq 'delete';
+	eval {
+	    if (is_valid_drivename($opt)) {
+		vmconfig_register_unused_drive($storecfg, $vmid, $conf, parse_drive($opt, $conf->{$opt}))
+		    if defined($conf->{$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);
     }
+
+    PVE::QemuConfig->write_config($vmid, $conf);
 }
 
 my $safe_num_ne = sub {
-- 
2.20.1