[pve-devel] [PATCH v5 qemu-server] vmconfig_apply_pending: remove redundant write/load config calls

[Oguz Bektas]

since we handle errors gracefully now, we don't need to write & save
config every time we change a setting.

Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
---
v4 -> v5:

changed some stuff according to the feedback from fabian and thomas,
thanks a lot!

* remove forgotten load_config call
* some style changes (if block vs. postfix if) for consistency
* do not run cleanup_pending in every iteration, but instead between the
delete and add/change loops (for optimization)
* remove unnecessary remove_from_pending_delete in the eval, since
that's already handled afterwards in the else block

 PVE/QemuServer.pm | 41 ++++++++++++++---------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index f7d99e3..eb4ef85 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5108,20 +5108,13 @@ sub vmconfig_apply_pending {
     foreach my $opt (sort keys %$pending_delete_hash) {
 	my $force = $pending_delete_hash->{$opt}->{force};
 	eval {
-	    die "internal error" if $opt =~ m/^unused/;
-	    $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);
+	    if ($opt =~ m/^unused/) {
+		die "internal error";
+	    }
+	    elsif (!defined($conf->{$opt})) {
+		# pass
 	    } 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);
-	    } else {
-		PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
-		delete $conf->{$opt};
-		PVE::QemuConfig->write_config($vmid, $conf);
 	    }
 	};
 	if (my $err = $@) {
@@ -5129,35 +5122,29 @@ sub vmconfig_apply_pending {
 	} else {
 	    PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
 	    delete $conf->{$opt};
-	    PVE::QemuConfig->write_config($vmid, $conf);
 	}
     }
 
-    $conf = PVE::QemuConfig->load_config($vmid); # update/reload
+    PVE::QemuConfig->cleanup_pending($conf);
 
     foreach my $opt (keys %{$conf->{pending}}) { # add/change
-	$conf = PVE::QemuConfig->load_config($vmid); # update/reload
-
+	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};
-	    } else {
-		$conf->{$opt} = $conf->{pending}->{$opt};
+	    my $old_val = $conf->{$opt};
+	    my $new_val = $conf->{pending}->{$opt};
+	    if (defined($old_val) && is_valid_drivename($opt) && $old_val ne $new_val) {
+		vmconfig_register_unused_drive($storecfg, $vmid, $conf, parse_drive($opt, $old_val))
 	    }
 	};
 	if (my $err = $@) {
 	    $add_apply_error->($opt, $err);
 	} else {
 	    $conf->{$opt} = delete $conf->{pending}->{$opt};
-	    PVE::QemuConfig->cleanup_pending($conf);
 	}
-
-	PVE::QemuConfig->write_config($vmid, $conf);
     }
+
+    # write all changes at once to avoid unnecessary i/o
+    PVE::QemuConfig->write_config($vmid, $conf);
 }
 
 my $safe_num_ne = sub {
-- 
2.20.1