create_vm: avoid premature write_config caused by update_pct_config

by moving the write_config calls from vmconfig_*_pending to their
call sites. The single other call site for update_pct_config in
update_vm is also adapted.

The update_pct_config call lead to a write_config call and so the
configuration file was created before it was intended to be created.

When the CFS is updated in between the write_config call and the
PVE::Cluster::check_vmid_unused call in create_and_lock_config,
the container file would already exist and so creation would
fail after writing out a basically empty config.

Even worse, a race was possible for two containers created with the
same ID at the same time:
Assuming the initial PVE::Cluster::check_vmid_unused check in the
parameter verification passes for both create_vm calls, the later one
would potentially overwrite the earlier configuration file with its
update_pct_config call.

Additionally, the file read for $old_config was always the one written
by update_pct_config. Meaning that for a create_vm call with force=1,
already existing old volumes were not removed.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>

diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
index 42e16d17f8e1adb37838130636e0228e7739a573..73fec36605b2bf925ff5671288be6b1fc80041d5 100644 (file)
--- a/src/PVE/API2/LXC/Config.pm
+++ b/src/PVE/API2/LXC/Config.pm
@@ -204,6 +204,7 @@ __PACKAGE__->register_method({
            my $running = PVE::LXC::check_running($vmid);
 
            my $errors = PVE::LXC::Config->update_pct_config($vmid, $conf, $running, $param, \@delete, \@revert);
+           PVE::LXC::Config->write_config($vmid, $conf);
            $conf = PVE::LXC::Config->load_config($vmid);
 
            PVE::LXC::update_lxc_config($vmid, $conf);

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 4eb1a14ac233dbb8c9ff97eec0bd4e425b486c7b..e0792082cdac830e24708dde6a706e6839c0dca2 100644 (file)
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -2173,6 +2173,7 @@ sub vm_start {
     if (scalar(keys %{$conf->{pending}})) {
        my $storecfg = PVE::Storage::config();
        PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storecfg);
+       PVE::LXC::Config->write_config($vmid, $conf);
        $conf = PVE::LXC::Config->load_config($vmid); # update/reload
     }
 

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 1443f8739d10fe165b8bf5004324a5bca3f15bed..dcc8755228cdf080b32a0498e17d911a14d38558 100644 (file)
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -1161,19 +1161,13 @@ sub vmconfig_hotplug_pending {
        $errors->{$opt} = "unable to hotplug $opt: $msg";
     };
 
-    my $changes;
     foreach my $opt (sort keys %{$conf->{pending}}) { # add/change
        next if $selection && !$selection->{$opt};
        if ($LXC_FASTPLUG_OPTIONS->{$opt}) {
            $conf->{$opt} = delete $conf->{pending}->{$opt};
-           $changes = 1;
        }
     }
 
-    if ($changes) {
-       $class->write_config($vmid, $conf);
-    }
-
     my $cgroup = PVE::LXC::CGroup->new($vmid);
 
     # There's no separate swap size to configure, there's memory and "total"
@@ -1258,8 +1252,6 @@ sub vmconfig_hotplug_pending {
            delete $conf->{pending}->{$opt};
        }
     }
-
-    $class->write_config($vmid, $conf);
 }
 
 sub vmconfig_apply_pending {
@@ -1316,8 +1308,6 @@ sub vmconfig_apply_pending {
            $conf->{$opt} = delete $conf->{pending}->{$opt};
        }
     }
-
-    $class->write_config($vmid, $conf);
 }
 
 my $rescan_volume = sub {

diff --git a/src/lxc-pve-poststop-hook b/src/lxc-pve-poststop-hook
index 1dba48c02f83ad48bb86c0e9b4deff47a1905ce3..2683ae2d1361fc011038a58d064f57e14c7a1176 100755 (executable)
--- a/src/lxc-pve-poststop-hook
+++ b/src/lxc-pve-poststop-hook
@@ -51,7 +51,10 @@ PVE::LXC::Tools::lxc_hook('post-stop', 'lxc', sub {
 
     my $config_updated = 0;
     if ($conf->{pending}) {
-       eval { PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storage_cfg) };
+       eval {
+           PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storage_cfg);
+           PVE::LXC::Config->write_config($vmid, $conf);
+       };
        warn "$@" if $@;
        PVE::LXC::update_lxc_config($vmid, $conf);
        $config_updated = 1;