vmconfig_apply_pending: add error handling

wrap around code which can possibly fail in evals to handle them
gracefully, and log errors.

note: this results in a change of behavior in the API. since errors
are handled gracefully instead of "die"ing, when there is a pending
change which cannot be applied for some reason, it will get logged in
the tasklog but the vm will continue booting regardless. the
non-applied change will stay in the pending section of the
configuration.

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 5bae5138177bbb6fa7f8adf6a6d73a558acf2d2b..e853a8355f4456f662cb2b4cc619dffa47c77219 100644 (file)
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1242,13 +1242,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 2b68d81fe1d7b32cb44ccab37ad51dcdb415276a..2de83768d3957ca3473a91b4a5997fa34aef367f 100644 (file)
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4977,23 +4977,39 @@ 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
 
     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 {
+           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);
+           } 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 = $@) {
+           $add_apply_error->($opt, $err);
        } else {
            PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
            delete $conf->{$opt};
@@ -5006,17 +5022,24 @@ sub vmconfig_apply_pending {
     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};
+       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};
+           }
+       };
+       if (my $err = $@) {
+           $add_apply_error->($opt, $err);
        } else {
-           $conf->{$opt} = $conf->{pending}->{$opt};
+           $conf->{$opt} = delete $conf->{pending}->{$opt};
+           PVE::QemuConfig->cleanup_pending($conf);
        }
 
-       delete $conf->{pending}->{$opt};
        PVE::QemuConfig->write_config($vmid, $conf);
     }
 }