[pve-devel] [PATCH v4 container 2/2] run post_clone_hook in clone_vm

[Oguz Bektas <o.bektas@proxmox.com>]

also cleaned up the locking situation with this, as Fabian G. suggested.
now we check if the 'create' lock is held before writing out the config
file.

use the 'create_and_lock_config' helper in the beginning to ensure that
the target CTID is available, and that the target config is locked from the
beginning. in case any error happens during the initial checks, we
unlink this config in error handling.

firewall config is also now cloned inside the worker instead of before the
worker, in case the clone fails.

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 src/PVE/API2/LXC.pm | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index a9ea3a6..d6b7bd2 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -1427,9 +1427,8 @@ __PACKAGE__->register_method({
 
 		my $src_conf = $snapname ? $src_conf->{snapshots}->{$snapname} : $src_conf;
 
+		PVE::LXC::Config->create_and_lock_config($newid, 0);
 		$conffile = PVE::LXC::Config->config_file($newid);
-		die "unable to create CT $newid: config file already exists\n"
-		    if -f $conffile;
 
 		my $sharedvm = 1;
 		foreach my $opt (keys %$src_conf) {
@@ -1468,7 +1467,7 @@ __PACKAGE__->register_method({
 
 			} else {
 			    # TODO: allow bind mounts?
-			    die "unable to clone mountpint '$opt' (type $mp->{type})\n";
+			    die "unable to clone mountpoint '$opt' (type $mp->{type})\n";
 			}
 		    } elsif ($opt =~ m/^net(\d+)$/) {
 			# always change MAC! address
@@ -1498,16 +1497,29 @@ __PACKAGE__->register_method({
 		    $newconf->{description} = $param->{description};
 		}
 
-		# create empty/temp config - this fails if CT already exists on other node
-		PVE::LXC::Config->write_config($newid, $newconf);
+		PVE::LXC::Config->lock_config($newid, sub {
+		    # read empty config, lock needs to be still here
+		    my $conf = PVE::LXC::Config->load_config($newid);
+		    die "Lost 'create' config lock, aborting.\n"
+			if !PVE::LXC::Config->has_lock($conf, 'create');
+		    # write the actual new config now to disk
+		    PVE::LXC::Config->write_config($newid, $newconf);
+		});
 	    };
 	    if (my $err = $@) {
 		eval { PVE::LXC::Config->remove_lock($vmid, 'disk') };
+		PVE::LXC::Config->lock_config($newid, sub {
+		    my $conf = PVE::LXC::Config->load_config($newid);
+		    die "Lost 'create' config lock, aborting.\n"
+			if !PVE::LXC::Config->has_lock($conf, 'create');
+		    unlink($conffile);
+		});
 		warn $@ if $@;
 		die $err;
 	    }
 	});
 
+
 	my $update_conf = sub {
 	    my ($key, $value) = @_;
 	    return PVE::LXC::Config->lock_config($newid, sub {
@@ -1519,6 +1531,8 @@ __PACKAGE__->register_method({
 	    });
 	};
 
+
+
 	my $realcmd = sub {
 	    my ($upid) = @_;
 
@@ -1559,6 +1573,16 @@ __PACKAGE__->register_method({
 		}
 
 		PVE::AccessControl::add_vm_to_pool($newid, $pool) if $pool;
+
+		$newconf = PVE::LXC::Config->load_config($newid);
+		die "Lost 'create' config lock, aborting.\n"
+		    if !PVE::LXC::Config->has_lock($newconf, 'create');
+		my $rootdir = PVE::LXC::mount_all($newid, $storecfg, $newconf, 1);
+		my $lxc_setup = PVE::LXC::Setup->new($newconf, $rootdir);
+		$lxc_setup->post_clone_hook($newconf);
+		PVE::LXC::umount_all($newid, $storecfg, $newconf, 1);
+
+
 		PVE::LXC::Config->remove_lock($newid, 'create');
 
 		if ($target) {
@@ -1572,7 +1596,6 @@ __PACKAGE__->register_method({
 		}
 	    };
 	    my $err = $@;
-
 	    # Unlock the source config in any case:
 	    eval { PVE::LXC::Config->remove_lock($vmid, 'disk') };
 	    warn $@ if $@;
@@ -1590,10 +1613,10 @@ __PACKAGE__->register_method({
 		die "clone failed: $err";
 	    }
 
+	    PVE::Firewall::clone_vmfw_conf($vmid, $newid);
 	    return;
 	};
 
-	PVE::Firewall::clone_vmfw_conf($vmid, $newid);
 	return $rpcenv->fork_worker('vzclone', $vmid, $authuser, $realcmd);
     }});
 
-- 
2.20.1