[pve-devel] [PATCH container 1/2] setup getty: ensure the correct services are enabled

[Fabian Grünbichler]

On Wed, Jul 17, 2019 at 12:08:24PM +0200, Thomas Lamprecht wrote:
> I.e., some distro templates do not have anything enabled, thus also
> ensure that the respective container-getty@ services are enabled.
> 
> But, as to getty on the same TTY makes for a strange experience also
> ensure that the getty@ are all removed (and vice versa in the other
> case)

s/as to/since two/

kind of meta, but these two subs (setup_systemd_console and
setup_container_getty) are very very similar now, maybe it would make
sense to just move the console unit content setting into its own sub,
and unify the rest where only the unit names differ (e.g., by passing
that in as parameter, or deciding it based on ostype)? alternatively,
just the iteration and some of the duplicate constants could be pulled
out into private helpers/vars.

> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
>  src/PVE/LXC/Setup/Base.pm | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm
> index f5f8b0d..5ec78a0 100644
> --- a/src/PVE/LXC/Setup/Base.pm
> +++ b/src/PVE/LXC/Setup/Base.pm
> @@ -185,7 +185,12 @@ sub setup_systemd_console {
>      my $ttycount = PVE::LXC::Config->get_tty_count($conf);
>  
>      for (my $i = 1; $i < 7; $i++) {
> -	my $tty_service_lnk = "/etc/systemd/system/getty.target.wants/getty\@tty$i.service";
> +	my $getty_target_fn = "/etc/systemd/system/getty.target.wants/";
> +	my $tty_service_lnk = "$getty_target_fn/getty\@tty$i.service";
> +
> +	# ensure that not two gettys are using the same tty!
> +	$self->ct_unlink("$getty_target_fn/container-getty\@$i.service");
> +
>  	if ($i > $ttycount) {
>  	    $self->ct_unlink($tty_service_lnk);
>  	} else {
> @@ -210,11 +215,30 @@ sub setup_container_getty_service {
>      my $systemd_dir_rel = $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);
>      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);
>      }
> +
> +    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\@$i.service");

here you unlink a different service instance (tty$i vs $i) than gets
linked in setup_systemd_console - is this intentional? doesn't this then
mean we actually have three different combos we have to take a look at?

maybe a more complete solution would be to remove all getty instances
first, and then setup $ttycount ones?

> +
> +	if ($i > $ttycount) {
> +	    $self->ct_unlink($tty_service_lnk);
> +	} else {
> +	    if (!$self->ct_is_symlink($tty_service_lnk)) {

this means masked units don't get enabled - intentionally?

> +		$self->ct_unlink($tty_service_lnk);
> +		$self->ct_symlink($servicefile, $tty_service_lnk);
> +	    }
> +	}
> +    }
>  }
>  
>  sub setup_systemd_networkd {
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel