From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id C0E249157B for ; Wed, 14 Feb 2024 15:04:29 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9A11D5440 for ; Wed, 14 Feb 2024 15:03:59 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 14 Feb 2024 15:03:58 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 5E0EE4818D for ; Wed, 14 Feb 2024 15:03:58 +0100 (CET) Message-ID: <03b9f748-7b98-404a-a980-0fdf60678a55@proxmox.com> Date: Wed, 14 Feb 2024 15:03:46 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Christoph Heiss Cc: Proxmox VE development discussion References: <20231121094230.309931-1-c.heiss@proxmox.com> <97b2b802-5e4b-42cc-9623-72fafb7b9495@proxmox.com> From: Fiona Ebner In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.072 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [nixos.pm] Subject: Re: [pve-devel] [PATCH container] setup: nixos: set cmode to console by default X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Feb 2024 14:04:29 -0000 Am 14.02.24 um 14:04 schrieb Christoph Heiss: > 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. > But template_fixup() doesn't change any config values either, right? If we think that most NixOS users can benefit from the different default and that the change is not likely to break existing containers/backups, then I'm not fundamentally against the change. In any case, it would need to be documented in the configuration schema that the default for NixOS is different. Two approaches would be: 1. If done in new(), it needs to be a '//=' assignment, so explicit value wins. 2. Could also be done in PVE::LXC::Config->get_cmode(), might be slightly cleaner (explicit value has to win too of course). > 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? > Is 'shell' also problematic? The check should use PVE::LXC::Config->get_cmode(), to avoid potential comparison against undef.