public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH pve-container 0/1] warn that nesting may be required
@ 2025-10-23 11:22 Robert Obkircher
  2025-10-23 11:22 ` [pve-devel] [PATCH pve-container 1/1] fix 6897: warn that nesting may be required for systemd Robert Obkircher
  0 siblings, 1 reply; 3+ messages in thread
From: Robert Obkircher @ 2025-10-23 11:22 UTC (permalink / raw)
  To: pve-devel

This patch adds a task-log warning on CT start if systemd is detected.

I'm not sure if this is the right approach as I'm not familiar with
perl.

It is also unclear what the minimum systemd version should be.


Robert Obkircher (1):
  fix 6897: warn that nesting may be required for systemd

 src/PVE/LXC/Setup.pm      |  7 +++++++
 src/PVE/LXC/Setup/Base.pm | 20 ++++++++++++++++++++
 src/lxc-pve-prestart-hook |  5 +++++
 3 files changed, 32 insertions(+)

-- 
2.47.3



_______________________________________________
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

* [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 0/1] warn that nesting may be required Robert Obkircher
@ 2025-10-23 11:22 ` Robert Obkircher
  2025-10-23 13:19   ` Fabian Grünbichler
  0 siblings, 1 reply; 3+ messages in thread
From: Robert Obkircher @ 2025-10-23 11:22 UTC (permalink / raw)
  To: pve-devel

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 {
+    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;
+    }
+
+    if (!defined($init) || $init !~ m@/systemd$@) {
+        return;
+    }
+
+    my $sdver = $self->get_systemd_version($init);
+    if (!defined($sdver) || $sdver < 232) {
+        return;
+    }
+
+    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);
+        }
+
         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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* 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

end of thread, other threads:[~2025-10-23 13:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-23 11:22 [pve-devel] [PATCH pve-container 0/1] warn that nesting may be required Robert Obkircher
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal