[pve-devel] [PATCH container] fix: avoid invalid config creation on hotplug failure

* [pve-devel] [PATCH container] fix: avoid invalid config creation on hotplug failure
@ 2024-11-19 11:29 Gabriel Goller
  2024-11-19 11:53 ` Maximiliano Sandoval
  2024-11-19 13:07 ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 2 replies; 3+ messages in thread
From: Gabriel Goller @ 2024-11-19 11:29 UTC (permalink / raw)
  To: pve-devel

If the hotplug of an interface on a lxc container fails for whatever
reason, the configuration will be broken and needs to manually fixed.

For example when adding a network interface with a bridge to a evpn vnet
(which doesn't support vlans) and we add a vlan tag, the interface will
be created even though we get an error. This will result in a broken
config (a interface without a bridge), which will cause the container to
not start anymore. Furthermore the veth interface will remain in a
`nomaster` state, which means the interface isn't connected to anything.
To solve this you would need to remove the interface manually from the
config.

To fix this we remove the logic that writes the intermediary config as
the config is wrong. This obviously reduces the consistency of the
config in some way, although that shouldn't be a problem (as it's
illegal anyway). We also need to revert to the old config in case the
new config can't be applied.

We also abort the api handler if we get an error updating the pending
config – this is not really necessary in this case, as we refrain from
writing the bad config completely. But it is nevertheless a good
practice because we won't write any other potentially bad config which
was produced during an erroneous pending config update.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 src/PVE/API2/LXC/Config.pm |  3 +++
 src/PVE/LXC.pm             | 12 ++++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
index e6c09801d67f..5cbc014b7c67 100644
--- a/src/PVE/API2/LXC/Config.pm
+++ b/src/PVE/API2/LXC/Config.pm
@@ -209,6 +209,9 @@ __PACKAGE__->register_method({
 	    my $running = PVE::LXC::check_running($vmid);
 
 	    my $errors = PVE::LXC::Config->update_pct_config($vmid, $conf, $running, $param, \@delete, \@revert);
+	    # don't write to config if we get any errors – this can result in a broken config
+	    raise_param_exc($errors) if scalar(keys %$errors);
+
 	    PVE::LXC::Config->write_config($vmid, $conf);
 	    $conf = PVE::LXC::Config->load_config($vmid);
 
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index dda41b26ee03..7874a0c56d30 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1051,11 +1051,6 @@ sub update_net {
 		    my $oldbridge = $oldnet->{bridge};
 
 		    PVE::Network::tap_unplug($veth);
-		    foreach (qw(bridge tag firewall)) {
-			delete $oldnet->{$_};
-		    }
-		    $conf->{$opt} = PVE::LXC::Config->print_lxc_network($oldnet);
-		    PVE::LXC::Config->write_config($vmid, $conf);
 
 		    if ($have_sdn && $bridge_changed) {
 			eval { PVE::Network::SDN::Vnets::del_ips_from_mac($oldbridge, $oldnet->{hwaddr}, $conf->{hostname}) };
@@ -1066,7 +1061,12 @@ sub update_net {
 		if ($have_sdn && $bridge_changed) {
 		    PVE::Network::SDN::Vnets::add_next_free_cidr($newnet->{bridge}, $conf->{hostname}, $newnet->{hwaddr}, $vmid, undef, 1);
 		}
-		PVE::LXC::net_tap_plug($veth, $newnet);
+		eval { PVE::LXC::net_tap_plug($veth, $newnet) };
+		if ($@) {
+		    my $err = $@;
+		    PVE::LXC::net_tap_plug($veth, $oldnet);
+		    die "$err\n";
+		}
 
 		# This includes the rate:
 		foreach (qw(bridge tag firewall rate link_down)) {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread