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

[Thomas Lamprecht]

On 7/18/19 10:23 AM, Fabian Grünbichler wrote:
> On Wed, Jul 17, 2019 at 12:08:24PM +0200, Thomas Lamprecht wrote:
> 
> 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.

yes can make sense, note also that "setup_systemd_console" is only used by
SUSE, and that is a bit broken in general, i.e., the network seup works
not fully correct (their docs and wicked are quite lacking, IMO), I'm not
too sure, but it could make sense to drop suse support completely...
(i.e., note now that it's depreacated and remove with 7.0 if the situation
did not get better and nobody stepped up to do some work needed there)


>> +
>> +	# 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?

No it should have been "tty$i", just a copy paste error of mine, thanks
for noticing.
> 
> maybe a more complete solution would be to remove all getty instances
> first, and then setup $ttycount ones?

Hmm, do you think at a situation where more than 7 (our max tty count)
are present in the CT? Else we already do this, albeit a bit implicitly
(or better said in a single pass).

> 
>> +
>> +	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?
> 

I intentionally copied the hunk we had in setup_systemd_console, yes,
but I also had the feeling that the situation with "masking" was not
really thought out when that got added there.

IMO, it does not make sense, this just produces strange situations,
mask from tty's > tty count get removed and the ones from configured
(and thus wanted ones) not.. will re do for a v2