[pve-devel] [PATCH v4 container 0/2] post_clone hook for containers

[pve-devel] [PATCH v4 container 0/2] post_clone hook for containers

[Oguz Bektas]

fixes #3443 by implementing post_clone_hook for containers (which at the
moment only generates a unique /etc/machine-id for the cloned
container). this can be reused later for other things we want to do post
clone.



Oguz Bektas (2):
  setup: add post_clone_hook for containers
  run post_clone_hook in clone_vm

 src/PVE/API2/LXC.pm       | 37 ++++++++++++++++++++++++++++++-------
 src/PVE/LXC/Setup.pm      | 12 ++++++++++++
 src/PVE/LXC/Setup/Base.pm | 31 +++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 7 deletions(-)

-- 
2.20.1

[pve-devel] [PATCH v4 container 1/2] setup: add post_clone_hook for containers

[Oguz Bektas]

for now it only calls the new clear_machine_id function.

this way when new containers are created they will have a unique
/etc/machine-id.

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 src/PVE/LXC/Setup.pm      | 12 ++++++++++++
 src/PVE/LXC/Setup/Base.pm | 31 +++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm
index 8b8fee9..bcb995d 100644
--- a/src/PVE/LXC/Setup.pm
+++ b/src/PVE/LXC/Setup.pm
@@ -352,6 +352,18 @@ sub pre_start_hook {
     $self->protected_call($code);
 }
 
+sub post_clone_hook {
+    my ($self, $conf) = @_;
+
+    my $clone = 1;
+
+    my $code = sub {
+	$self->{plugin}->post_clone_hook($self->{conf}, $clone);
+    };
+    $self->protected_call($code);
+
+}
+
 sub post_create_hook {
     my ($self, $root_password, $ssh_keys) = @_;
 
diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm
index d73335b..54cb9a8 100644
--- a/src/PVE/LXC/Setup/Base.pm
+++ b/src/PVE/LXC/Setup/Base.pm
@@ -476,6 +476,30 @@ sub set_timezone {
     }
 }
 
+sub clear_machine_id {
+    my ($self, $conf, $clone) = @_;
+
+    my $uses_systemd = $self->ct_is_executable("/lib/systemd/systemd")
+	|| $self->ct_is_executable("/usr/lib/systemd/systemd");
+
+    my $dbus_machine_id_path = "/var/lib/dbus/machine-id";
+    my $machine_id_path = "/etc/machine-id";
+    if (
+	$self->ct_file_exists($dbus_machine_id_path)
+	&& !$self->ct_is_symlink($dbus_machine_id_path)
+	&& $uses_systemd
+    ) {
+        $self->ct_unlink($dbus_machine_id_path);
+    }
+
+    # don't remove file if container is being cloned
+    if ($clone) {
+	$self->ct_file_set_contents($machine_id_path, "\n");
+    } else {
+	$self->ct_unlink($machine_id_path);
+    }
+}
+
 sub pre_start_hook {
     my ($self, $conf) = @_;
 
@@ -488,9 +512,16 @@ sub pre_start_hook {
     # fixme: what else ?
 }
 
+sub post_clone_hook {
+    my ($self, $conf) = @_;
+
+    $self->clear_machine_id($conf, 1);
+}
+
 sub post_create_hook {
     my ($self, $conf, $root_password, $ssh_keys) = @_;
 
+    $self->clear_machine_id($conf);
     $self->template_fixup($conf);
 
     &$randomize_crontab($self, $conf);
-- 
2.20.1

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

[Oguz Bektas]

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

Re: [pve-devel] [PATCH v4 container 0/2] post_clone hook for containers

[Thomas Lamprecht]

On 16.06.21 15:24, Oguz Bektas wrote:
> fixes #3443 by implementing post_clone_hook for containers (which at the
> moment only generates a unique /etc/machine-id for the cloned
> container). this can be reused later for other things we want to do post
> clone.
> 

what are the changes since v3? I did not found any note here nor in the actual
patches.

> 
> 
> Oguz Bektas (2):
>   setup: add post_clone_hook for containers
>   run post_clone_hook in clone_vm
> 
>  src/PVE/API2/LXC.pm       | 37 ++++++++++++++++++++++++++++++-------
>  src/PVE/LXC/Setup.pm      | 12 ++++++++++++
>  src/PVE/LXC/Setup/Base.pm | 31 +++++++++++++++++++++++++++++++
>  3 files changed, 73 insertions(+), 7 deletions(-)
> 

Re: [pve-devel] [PATCH v4 container 0/2] post_clone hook for containers

[Oguz Bektas]

On Wed, Jun 16, 2021 at 03:48:42PM +0200, Thomas Lamprecht wrote:
> On 16.06.21 15:24, Oguz Bektas wrote:
> > fixes #3443 by implementing post_clone_hook for containers (which at the
> > moment only generates a unique /etc/machine-id for the cloned
> > container). this can be reused later for other things we want to do post
> > clone.
> > 
> 
> what are the changes since v3? I did not found any note here nor in the actual
> patches.

oops. sorry!

v3->v4:
* call generic post_clone_hook in setup code instead of directly calling
clear_machine_id in clone_vm API
* improve config locking in clone_vm API (don't write to config file
without checking/obtaining lock on it)
* clone firewall config _inside_ worker instead of _before_ worker
* fixed minor typo in error message for bind mounts

> 
> > 
> > 
> > Oguz Bektas (2):
> >   setup: add post_clone_hook for containers
> >   run post_clone_hook in clone_vm
> > 
> >  src/PVE/API2/LXC.pm       | 37 ++++++++++++++++++++++++++++++-------
> >  src/PVE/LXC/Setup.pm      | 12 ++++++++++++
> >  src/PVE/LXC/Setup/Base.pm | 31 +++++++++++++++++++++++++++++++
> >  3 files changed, 73 insertions(+), 7 deletions(-)
> > 
>