Re: [pve-devel] [PATCH container] setup: nixos: set cmode to console by default

[Christoph Heiss <c.heiss@proxmox.com>]

Thanks for the review!

On Fri, Jan 26, 2024 at 03:47:34PM +0100, Fiona Ebner wrote:
> Am 21.11.23 um 10:40 schrieb Christoph Heiss:
[..]
> > diff --git a/src/PVE/LXC/Setup/NixOS.pm b/src/PVE/LXC/Setup/NixOS.pm
> > index c702f3d..7f23111 100644
> > --- a/src/PVE/LXC/Setup/NixOS.pm
> > +++ b/src/PVE/LXC/Setup/NixOS.pm
> > @@ -17,6 +17,11 @@ sub new {
> >
> >      $conf->{ostype} = "nixos";
> >
> > +    # Set `cmode` to `console` for NixOS containers, as getty is only configured for /dev/console by
> > +    # default, but not any TTY ports. This way, users still get a login/shell instead of just a
> > +    # blank screen when openinng the console in the web UI.
> > +    $conf->{cmode} = 'console';
> > +
> >      return bless $self, $class;
> >  }
> >
>
> Won't this override any pre-existing setting (from user or backup)?
> Even if checking for that, it could still be considered a breaking
> change, i.e. other people might have configured their NixOS container
> differently, so maybe best to wait for the next major release?

Yep, just tried that out - so that's definitely a no-go.

>
> For now, we could log a warning if no explicit 'cmode' was specified for
> NixOS.
>
> I don't see any other implementation of new() overriding any defaults.
> Should we even start doing that?

Well, looking at the code again, there is the ->template_fixup() sub,
which gets called on restores after ->new(). This does seem to override
some things (e.g. ->setup_securetty()) for most of the distros.

So I guess this would be correct place after all for this, if we
override something.

Anyway, so just a

  warn "..." if `$conf->{cmode} ne 'console'`;

if ->new() would be acceptable?