[pve-devel] [PATCH container v2 3/5] setup getty: generalize setup_container_getty_service

[Fabian Grünbichler]

On Thu, Jul 18, 2019 at 06:37:00PM +0200, Thomas Lamprecht wrote:
> to allow switching the two remaining users and then finally dropping
> the setup_systemd_console method
> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
> 
> new in v2
> 
>  src/PVE/LXC/Setup/Base.pm | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm
> index e0b8244..4880a2b 100644
> --- a/src/PVE/LXC/Setup/Base.pm
> +++ b/src/PVE/LXC/Setup/Base.pm
> @@ -216,26 +216,41 @@ sub fixup_old_getty {
>  sub setup_container_getty_service {
>      my ($self, $conf) = @_;
>  
> -    my $systemd_dir_rel = $self->ct_is_executable("/lib/systemd/systemd") ?
> +    my $sd_dir = $self->ct_is_executable("/lib/systemd/systemd") ?
>  	"/lib/systemd/system" : "/usr/lib/systemd/system";
> -    my $servicefile = "$systemd_dir_rel/container-getty\@.service";
> -    my $raw = $self->ct_file_get_contents($servicefile);
> +
> +    # prefer container-getty.service shipped by newer systemd versions
> +    # fallback to getty.service and just return if that doesn't exists either..
> +    my $service_base = "container-getty\@";
> +    my $service = "${service_base}.service";
> +    my $template_base = '';

purely cosmetic, but shouldn't these actually be

$template_base = "container-getty\@"
$template_unit = "${template_base}.service"
$instance_prefix = '';

also,  $service is only used in combination with $sd_dir, so maybe we
could even do

$template_base = "container-getty\@"
$template_path = "${sd_dir}/${template_base}.service"
$instance_prefix = '';

similarly, for $instance_prefix we could do the full $instance_base:

$instance_base = "${template_base}"
$instance_base = "${template_base}tty"

instead, i.e., the following diff on top of this commit (feel free to
just pull in as follow-up/squash):

diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm
index 4880a2b..e811043 100644
--- a/src/PVE/LXC/Setup/Base.pm
+++ b/src/PVE/LXC/Setup/Base.pm
@@ -221,21 +221,21 @@ sub setup_container_getty_service {
 
     # prefer container-getty.service shipped by newer systemd versions
     # fallback to getty.service and just return if that doesn't exists either..
-    my $service_base = "container-getty\@";
-    my $service = "${service_base}.service";
-    my $template_base = '';
-
-    if (!$self->ct_file_exists("$sd_dir/$service")) {
-	$service_base = "getty\@";
-	$service = "${service_base}.service";
-	$template_base = 'tty';
-	return if !$self->ct_file_exists("$sd_dir/$service");
+    my $template_base = "container-getty\@";
+    my $template_path = "${sd_dir}/${template_base}.service";
+    my $instance_base = $template_base;
+
+    if (!$self->ct_file_exists($template_path)) {
+	$template_base = "getty\@";
+	$template_path = "${template_base}.service";
+	$instance_base = "{$template_base}tty";
+	return if !$self->ct_file_exists($template_path);
     }
 
-    my $raw = $self->ct_file_get_contents("$sd_dir/$service");
+    my $raw = $self->ct_file_get_contents($template_path);
     my $ttyname = $self->devttydir($conf) . 'tty%I';
     if ($raw =~ s at pts/%I|lxc/tty%I@$ttyname at g) {
-	$self->ct_file_set_contents("$sd_dir/$service", $raw);
+	$self->ct_file_set_contents($template_path, $raw);
     }
 
     my $getty_target_fn = "/etc/systemd/system/getty.target.wants/";
@@ -248,9 +248,9 @@ sub setup_container_getty_service {
 
 	# re-enable only those requested
 	if ($i <= $ttycount) {
-	    my $tty_service = "${service_base}${template_base}${i}.service";
+	    my $tty_service = "${instance_base}${i}.service";
 
-	    $self->ct_symlink("$sd_dir/$service", "$getty_target_fn/$tty_service");
+	    $self->ct_symlink($template_path, "$getty_target_fn/$tty_service");
 	}
     }
 }

> +
> +    if (!$self->ct_file_exists("$sd_dir/$service")) {
> +	$service_base = "getty\@";
> +	$service = "${service_base}.service";
> +	$template_base = 'tty';
> +	return if !$self->ct_file_exists("$sd_dir/$service");
> +    }
> +
> +    my $raw = $self->ct_file_get_contents("$sd_dir/$service");
>      my $ttyname = $self->devttydir($conf) . 'tty%I';
>      if ($raw =~ s at pts/%I|lxc/tty%I@$ttyname at g) {
> -	$self->ct_file_set_contents($servicefile, $raw);
> +	$self->ct_file_set_contents("$sd_dir/$service", $raw);
>      }
>  
> +    my $getty_target_fn = "/etc/systemd/system/getty.target.wants/";
>      my $ttycount = PVE::LXC::Config->get_tty_count($conf);
> +
>      for (my $i = 1; $i < 7; $i++) {
> -	my $getty_target_fn = "/etc/systemd/system/getty.target.wants/";
> -	my $tty_service_lnk = "$getty_target_fn/container-getty\@$i.service";
> -
>  	# ensure that not two gettys are using the same tty!
>  	$self->ct_unlink("$getty_target_fn/getty\@tty$i.service");
> +	$self->ct_unlink("$getty_target_fn/container-getty\@$i.service");
>  
> -	$self->ct_unlink($tty_service_lnk);
> +	# re-enable only those requested
>  	if ($i <= $ttycount) {
> -	    $self->ct_symlink($servicefile, $tty_service_lnk);
> +	    my $tty_service = "${service_base}${template_base}${i}.service";
> +
> +	    $self->ct_symlink("$sd_dir/$service", "$getty_target_fn/$tty_service");
>  	}
>      }
>  }
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel