* Re: [pve-devel] [PATCH pve-container 1/1] fix 6897: warn that nesting may be required for systemd
2025-10-23 11:22 ` [pve-devel] [PATCH pve-container 1/1] fix 6897: warn that nesting may be required for systemd Robert Obkircher
@ 2025-10-23 13:19 ` Fabian Grünbichler
0 siblings, 0 replies; 3+ messages in thread
From: Fabian Grünbichler @ 2025-10-23 13:19 UTC (permalink / raw)
To: Proxmox VE development discussion
a little bit of commit message would be nice - e.g. some background why
systemd wants nesting, and how the version was picked, why we need a
separate call and cannot just fold this into the pre-start hook of the
base plugin.
On October 23, 2025 1:22 pm, Robert Obkircher wrote:
> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
> ---
> src/PVE/LXC/Setup.pm | 7 +++++++
> src/PVE/LXC/Setup/Base.pm | 20 ++++++++++++++++++++
> src/lxc-pve-prestart-hook | 5 +++++
> 3 files changed, 32 insertions(+)
>
> diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm
> index 87330c4..197b3ad 100644
> --- a/src/PVE/LXC/Setup.pm
> +++ b/src/PVE/LXC/Setup.pm
> @@ -320,6 +320,13 @@ sub unified_cgroupv2_support {
> return $self->{plugin}->unified_cgroupv2_support($self->get_ct_init_path());
> }
>
> +sub get_may_require_nesting_warning {
this could just be called check_systemd_nesting or something like that,
see below
in particular because at some point this might no longer just emit a
warning, but become a hard requirement..
> + my ($self) = @_;
> +
> + my $init = $self->get_ct_init_path();
> + return $self->{plugin}->get_may_require_nesting_warning($self->{conf}, $init);
> +}
> +
> # os-release(5):
> # (...) a newline-separated list of environment-like shell-compatible
> # variable assignments. (...) beyond mere variable assignments, no shell
> diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm
> index a2c88ed..bcb26ba 100644
> --- a/src/PVE/LXC/Setup/Base.pm
> +++ b/src/PVE/LXC/Setup/Base.pm
> @@ -647,6 +647,26 @@ sub get_ct_init_path {
> return $init_path;
> }
>
> +sub get_may_require_nesting_warning {
> + my ($self, $conf, $init) = @_;
> +
> + my $features = PVE::LXC::Config->parse_features($conf->{features});
> + if ($features->{nesting}) {
> + return;
> + }
this can be written more concisely:
return if $features->{nesting};
in really simple cases like this this is usually easier to read because
of the reduced boiler plate
> +
> + if (!defined($init) || $init !~ m@/systemd$@) {
> + return;
> + }
same here
> +
> + my $sdver = $self->get_systemd_version($init);
> + if (!defined($sdver) || $sdver < 232) {
> + return;
> + }
and here. this one might benefit from a comment how that version was
picked, so that one doesn't have to go history digging in the future ;)
> +
> + return "Systemd $sdver detected. You might need to enable nesting.";
> +}
> +
> sub ssh_host_key_types_to_generate {
> my ($self) = @_;
>
> diff --git a/src/lxc-pve-prestart-hook b/src/lxc-pve-prestart-hook
> index 73125e1..09e8e44 100755
> --- a/src/lxc-pve-prestart-hook
> +++ b/src/lxc-pve-prestart-hook
> @@ -172,6 +172,11 @@ PVE::LXC::Tools::lxc_hook(
> }
> }
>
> + my $nesting_warning = $lxc_setup->get_may_require_nesting_warning();
> + if ($nesting_warning) {
> + log_warn($vmid, $nesting_warning);
> + }
> +
this is not needed, you can call your new helper in
PVE::LXC::Setup::pre_start_hook
but first we probably need to extend the plugin/setup code to have a
warning helper like the one used in the prestart-hook, because right now
any warnings emitted by the setup code (either via `warn` or via the
RestEnvironment's `log_warn`) will go to journal, instead of ending up
in the task log.. this also affects two existing warnings emitted by
Setup->new for Debian and Ubuntu containers with too recent versions..
> if (@$devices) {
> my $devlist = '';
> foreach my $dev (@$devices) {
> --
> 2.47.3
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 3+ messages in thread