Re: [pve-devel] [PATCH pve-container 1/1] fix 6897: warn that nesting may be required for systemd

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

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