[pve-devel] [PATCH container] create_vm: fix order of config creation/reading/locking

[Fabian Ebner]

The update_pct_config call leads 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.
(Meaning this is necessary for the proposed "update CFS after
obtaining a configuration file lock" changes)

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 at proxmox.com>
---

Note that this should be applied before [0] to avoid temporary
(further ;)) breakage of container creation.

[0]: https://pve.proxmox.com/pipermail/pve-devel/2020-April/043155.html

 src/PVE/API2/LXC.pm | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 148ba6a..46d2fd7 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -334,10 +334,6 @@ __PACKAGE__->register_method({
 	# check/activate default storage
 	&$check_and_activate_storage($storage) if !defined($mp_param->{rootfs});
 
-	PVE::LXC::Config->update_pct_config($vmid, $conf, 0, $no_disk_param);
-
-	$conf->{unprivileged} = 1 if $unprivileged;
-
 	my $emsg = $restore ? "unable to restore CT $vmid -" : "unable to create CT $vmid -";
 
 	eval { PVE::LXC::Config->create_and_lock_config($vmid, $force) };
@@ -346,8 +342,11 @@ __PACKAGE__->register_method({
 	my $code = sub {
 	    my $old_conf = PVE::LXC::Config->load_config($vmid);
 	    my $was_template;
-
 	    my $vollist = [];
+
+	    PVE::LXC::Config->update_pct_config($vmid, $conf, 0, $no_disk_param);
+	    $conf->{unprivileged} = 1 if $unprivileged;
+
 	    eval {
 		my $orig_mp_param; # only used if $restore
 		if ($restore) {
-- 
2.20.1