public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH-SERIES container 0/3] add missing no-op methods for unmanaged CTs
@ 2026-02-04  9:17 Daniel Kral
  2026-02-04  9:17 ` [PATCH container 1/3] setup: add no-op check_systemd_nesting " Daniel Kral
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Daniel Kral @ 2026-02-04  9:17 UTC (permalink / raw)
  To: pve-devel

Small fixes, which add missing no-op methods for unmanaged containers,
where the first fixes unmanaged containers to start again and be able to
create/clone unmanaged containers, while the latter is only removing an
error message.

Tested with an unmanaged voidlinux container.

Daniel Kral (3):
  setup: add no-op check_systemd_nesting for unmanaged CTs
  setup: add no-op detect_architecture for unmanaged CTs
  setup: make the architecture fall back to amd64 for empty strings

 src/PVE/LXC/Setup.pm           |  2 +-
 src/PVE/LXC/Setup/Unmanaged.pm | 13 ++++++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.47.3





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

* [PATCH container 1/3] setup: add no-op check_systemd_nesting for unmanaged CTs
  2026-02-04  9:17 [PATCH-SERIES container 0/3] add missing no-op methods for unmanaged CTs Daniel Kral
@ 2026-02-04  9:17 ` Daniel Kral
  2026-02-04  9:35   ` Daniel Kral
  2026-02-05 10:12   ` Robert Obkircher
  2026-02-04  9:17 ` [PATCH container 2/3] setup: add no-op detect_architecture " Daniel Kral
  2026-02-04  9:17 ` [PATCH container 3/3] setup: make the architecture fall back to amd64 for empty strings Daniel Kral
  2 siblings, 2 replies; 10+ messages in thread
From: Daniel Kral @ 2026-02-04  9:17 UTC (permalink / raw)
  To: pve-devel

Otherwise the container will fail to start, create and clone, because
the plugin's check_systemd_nesting is only defined in the Base module
but not the Unmanaged module and is called in the pre_start_hook,
post_clone_hook, and post_create_hook.

Reported in the Proxmox Forum [0].

[0] https://forum.proxmox.com/threads/180258/

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 src/PVE/LXC/Setup/Unmanaged.pm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/PVE/LXC/Setup/Unmanaged.pm b/src/PVE/LXC/Setup/Unmanaged.pm
index d76921e..aa26c1c 100644
--- a/src/PVE/LXC/Setup/Unmanaged.pm
+++ b/src/PVE/LXC/Setup/Unmanaged.pm
@@ -51,7 +51,13 @@ sub unified_cgroupv2_support {
 
 sub get_ct_init_path {
     my ($self) = @_;
-    return '/sbin/init'; # only passed to unified_cgroupv2_support for now
+    # only passed to check_systemd_nesting and unified_cgroupv2_support for now
+    return '/sbin/init';
+}
+
+sub check_systemd_nesting {
+    my ($self, $conf, $init) = @_;
+    return;
 }
 
 sub ssh_host_key_types_to_generate {
-- 
2.47.3





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

* [PATCH container 2/3] setup: add no-op detect_architecture for unmanaged CTs
  2026-02-04  9:17 [PATCH-SERIES container 0/3] add missing no-op methods for unmanaged CTs Daniel Kral
  2026-02-04  9:17 ` [PATCH container 1/3] setup: add no-op check_systemd_nesting " Daniel Kral
@ 2026-02-04  9:17 ` Daniel Kral
  2026-02-04  9:17 ` [PATCH container 3/3] setup: make the architecture fall back to amd64 for empty strings Daniel Kral
  2 siblings, 0 replies; 10+ messages in thread
From: Daniel Kral @ 2026-02-04  9:17 UTC (permalink / raw)
  To: pve-devel

This plugin method is only called in PVE::LXC::Setup::new() and is
wrapped in an eval block, so it won't fail to create the container, but
report an error that the plugin method is not implemented for unmanaged
containers.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 src/PVE/LXC/Setup/Unmanaged.pm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/PVE/LXC/Setup/Unmanaged.pm b/src/PVE/LXC/Setup/Unmanaged.pm
index aa26c1c..b51be55 100644
--- a/src/PVE/LXC/Setup/Unmanaged.pm
+++ b/src/PVE/LXC/Setup/Unmanaged.pm
@@ -65,6 +65,11 @@ sub ssh_host_key_types_to_generate {
     return;
 }
 
+sub detect_architecture {
+    my ($self) = @_;
+    return;
+}
+
 # hooks
 
 sub pre_start_hook {
-- 
2.47.3





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

* [PATCH container 3/3] setup: make the architecture fall back to amd64 for empty strings
  2026-02-04  9:17 [PATCH-SERIES container 0/3] add missing no-op methods for unmanaged CTs Daniel Kral
  2026-02-04  9:17 ` [PATCH container 1/3] setup: add no-op check_systemd_nesting " Daniel Kral
  2026-02-04  9:17 ` [PATCH container 2/3] setup: add no-op detect_architecture " Daniel Kral
@ 2026-02-04  9:17 ` Daniel Kral
  2026-02-06 10:24   ` Fiona Ebner
  2 siblings, 1 reply; 10+ messages in thread
From: Daniel Kral @ 2026-02-04  9:17 UTC (permalink / raw)
  To: pve-devel

Otherwise, if the underlying detect_architecture(...) method returns any
false value, the return value of the call to protected_call(...) will
return an empty string.

This sets the architecture to an empty string and will make the
container fail to start.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 src/PVE/LXC/Setup.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm
index 113093d..fb0207e 100644
--- a/src/PVE/LXC/Setup.pm
+++ b/src/PVE/LXC/Setup.pm
@@ -153,7 +153,7 @@ sub new {
             warn "Architecture detection failed: $err" if $err;
         }
 
-        if (!defined($arch)) {
+        if (!$arch) {
             $arch = 'amd64';
             print "Falling back to $arch.\nUse `pct set VMID --arch ARCH` to change.\n";
         } else {
-- 
2.47.3





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

* Re: [PATCH container 1/3] setup: add no-op check_systemd_nesting for unmanaged CTs
  2026-02-04  9:17 ` [PATCH container 1/3] setup: add no-op check_systemd_nesting " Daniel Kral
@ 2026-02-04  9:35   ` Daniel Kral
  2026-02-05 10:12   ` Robert Obkircher
  1 sibling, 0 replies; 10+ messages in thread
From: Daniel Kral @ 2026-02-04  9:35 UTC (permalink / raw)
  To: Daniel Kral, pve-devel

On Wed Feb 4, 2026 at 10:17 AM CET, Daniel Kral wrote:
> Otherwise the container will fail to start, create and clone, because
> the plugin's check_systemd_nesting is only defined in the Base module
> but not the Unmanaged module and is called in the pre_start_hook,
> post_clone_hook, and post_create_hook.
>
> Reported in the Proxmox Forum [0].
>
> [0] https://forum.proxmox.com/threads/180258/

I noticed there was already a Bugzilla entry for this here [0].

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=7270




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

* Re: [PATCH container 1/3] setup: add no-op check_systemd_nesting for unmanaged CTs
  2026-02-04  9:17 ` [PATCH container 1/3] setup: add no-op check_systemd_nesting " Daniel Kral
  2026-02-04  9:35   ` Daniel Kral
@ 2026-02-05 10:12   ` Robert Obkircher
  2026-02-05 10:46     ` Daniel Kral
  1 sibling, 1 reply; 10+ messages in thread
From: Robert Obkircher @ 2026-02-05 10:12 UTC (permalink / raw)
  To: pve-devel


On 2/4/26 10:16, Daniel Kral wrote:
> Otherwise the container will fail to start, create and clone, because
> the plugin's check_systemd_nesting is only defined in the Base module
> but not the Unmanaged module and is called in the pre_start_hook,
> post_clone_hook, and post_create_hook.
>
> Reported in the Proxmox Forum [0].
>
> [0] https://forum.proxmox.com/threads/180258/
>
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
>  src/PVE/LXC/Setup/Unmanaged.pm | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/LXC/Setup/Unmanaged.pm b/src/PVE/LXC/Setup/Unmanaged.pm
> index d76921e..aa26c1c 100644
> --- a/src/PVE/LXC/Setup/Unmanaged.pm
> +++ b/src/PVE/LXC/Setup/Unmanaged.pm
> @@ -51,7 +51,13 @@ sub unified_cgroupv2_support {
>  
>  sub get_ct_init_path {
>      my ($self) = @_;
> -    return '/sbin/init'; # only passed to unified_cgroupv2_support for now
> +    # only passed to check_systemd_nesting and unified_cgroupv2_support for now
> +    return '/sbin/init';
> +}
> +
> +sub check_systemd_nesting {
> +    my ($self, $conf, $init) = @_;
> +    return;
>  }

Applying this patch fixed the startup problem. I tested this by
changing the ostype of a Debian container to unmanaged.

The only thing I've noticed is that the method is also missing from
Plugin.pm.

>  
>  sub ssh_host_key_types_to_generate {




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

* Re: [PATCH container 1/3] setup: add no-op check_systemd_nesting for unmanaged CTs
  2026-02-05 10:12   ` Robert Obkircher
@ 2026-02-05 10:46     ` Daniel Kral
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Kral @ 2026-02-05 10:46 UTC (permalink / raw)
  To: Robert Obkircher, pve-devel

On Thu Feb 5, 2026 at 11:12 AM CET, Robert Obkircher wrote:
> Applying this patch fixed the startup problem. I tested this by
> changing the ostype of a Debian container to unmanaged.
>
> The only thing I've noticed is that the method is also missing from
> Plugin.pm.

Good catch, thanks! I'll add that in a v2 + prepend the summary with fix
#7270.




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

* Re: [PATCH container 3/3] setup: make the architecture fall back to amd64 for empty strings
  2026-02-04  9:17 ` [PATCH container 3/3] setup: make the architecture fall back to amd64 for empty strings Daniel Kral
@ 2026-02-06 10:24   ` Fiona Ebner
  2026-02-06 12:12     ` Daniel Kral
  0 siblings, 1 reply; 10+ messages in thread
From: Fiona Ebner @ 2026-02-06 10:24 UTC (permalink / raw)
  To: Daniel Kral, pve-devel

Am 04.02.26 um 10:17 AM schrieb Daniel Kral:
> Otherwise, if the underlying detect_architecture(...) method returns any
> false value, the return value of the call to protected_call(...) will

Do you mean undef value here? If I return 0 inside a protected call I get 0

> return an empty string.

not an empty string.

There seems to be a difference in behavior between being in a nested
protected call, which will return the result from the $sub directly, and
a non-nested protected call, which reads the result from the pipe, which
also results in an empty string when the result from $sub is undef.

I think the change here is fine, but we might want to document the
behavior for protected call. Or fix up the non-nested case to properly
return undef if $sub returns undef. Can be it's own series and will
require checking that use sites are happy with the change, but it seems
like there's only a small bunch that look at the return value anyways.

> This sets the architecture to an empty string and will make the
> container fail to start.
> 
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
>  src/PVE/LXC/Setup.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm
> index 113093d..fb0207e 100644
> --- a/src/PVE/LXC/Setup.pm
> +++ b/src/PVE/LXC/Setup.pm
> @@ -153,7 +153,7 @@ sub new {
>              warn "Architecture detection failed: $err" if $err;
>          }
>  
> -        if (!defined($arch)) {
> +        if (!$arch) {
>              $arch = 'amd64';
>              print "Falling back to $arch.\nUse `pct set VMID --arch ARCH` to change.\n";
>          } else {





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

* Re: [PATCH container 3/3] setup: make the architecture fall back to amd64 for empty strings
  2026-02-06 10:24   ` Fiona Ebner
@ 2026-02-06 12:12     ` Daniel Kral
  2026-02-06 12:31       ` Fiona Ebner
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Kral @ 2026-02-06 12:12 UTC (permalink / raw)
  To: Fiona Ebner, pve-devel

On Fri Feb 6, 2026 at 11:24 AM CET, Fiona Ebner wrote:
> Am 04.02.26 um 10:17 AM schrieb Daniel Kral:
>> Otherwise, if the underlying detect_architecture(...) method returns any
>> false value, the return value of the call to protected_call(...) will
>
> Do you mean undef value here? If I return 0 inside a protected call I get 0
>
>> return an empty string.
>
> not an empty string.
>
> There seems to be a difference in behavior between being in a nested
> protected call, which will return the result from the $sub directly, and
> a non-nested protected call, which reads the result from the pipe, which
> also results in an empty string when the result from $sub is undef.

Good catch, thanks! I only tried it with non-nested protected calls
which use the output from the pipe and also assumed that the empty
string comes from the conversion from a falsy value to a string in perl,
which is the empty string.

>
> I think the change here is fine, but we might want to document the
> behavior for protected call. Or fix up the non-nested case to properly
> return undef if $sub returns undef. Can be it's own series and will
> require checking that use sites are happy with the change, but it seems
> like there's only a small bunch that look at the return value anyways.

Right, as of writing this only get_os_release(), get_ct_init_path(), and
new() use the return value and all of these expect a string value.

For this series, I'll adapt the commit message in a v2.

>
>> This sets the architecture to an empty string and will make the
>> container fail to start.
>> 
>> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
>> ---
>>  src/PVE/LXC/Setup.pm | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm
>> index 113093d..fb0207e 100644
>> --- a/src/PVE/LXC/Setup.pm
>> +++ b/src/PVE/LXC/Setup.pm
>> @@ -153,7 +153,7 @@ sub new {
>>              warn "Architecture detection failed: $err" if $err;
>>          }
>>  
>> -        if (!defined($arch)) {
>> +        if (!$arch) {
>>              $arch = 'amd64';
>>              print "Falling back to $arch.\nUse `pct set VMID --arch ARCH` to change.\n";
>>          } else {





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

* Re: [PATCH container 3/3] setup: make the architecture fall back to amd64 for empty strings
  2026-02-06 12:12     ` Daniel Kral
@ 2026-02-06 12:31       ` Fiona Ebner
  0 siblings, 0 replies; 10+ messages in thread
From: Fiona Ebner @ 2026-02-06 12:31 UTC (permalink / raw)
  To: Daniel Kral, pve-devel

Am 06.02.26 um 1:11 PM schrieb Daniel Kral:
> On Fri Feb 6, 2026 at 11:24 AM CET, Fiona Ebner wrote:
>> Am 04.02.26 um 10:17 AM schrieb Daniel Kral:
>>> Otherwise, if the underlying detect_architecture(...) method returns any
>>> false value, the return value of the call to protected_call(...) will
>>
>> Do you mean undef value here? If I return 0 inside a protected call I get 0
>>
>>> return an empty string.
>>
>> not an empty string.
>>
>> There seems to be a difference in behavior between being in a nested
>> protected call, which will return the result from the $sub directly, and
>> a non-nested protected call, which reads the result from the pipe, which
>> also results in an empty string when the result from $sub is undef.
> 
> Good catch, thanks! I only tried it with non-nested protected calls
> which use the output from the pipe and also assumed that the empty
> string comes from the conversion from a falsy value to a string in perl,
> which is the empty string.

Nit: should probably read "from a falsy value that does convert to an
empty string in Perl". Not all falsy values do ;)




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

end of thread, other threads:[~2026-02-06 12:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-04  9:17 [PATCH-SERIES container 0/3] add missing no-op methods for unmanaged CTs Daniel Kral
2026-02-04  9:17 ` [PATCH container 1/3] setup: add no-op check_systemd_nesting " Daniel Kral
2026-02-04  9:35   ` Daniel Kral
2026-02-05 10:12   ` Robert Obkircher
2026-02-05 10:46     ` Daniel Kral
2026-02-04  9:17 ` [PATCH container 2/3] setup: add no-op detect_architecture " Daniel Kral
2026-02-04  9:17 ` [PATCH container 3/3] setup: make the architecture fall back to amd64 for empty strings Daniel Kral
2026-02-06 10:24   ` Fiona Ebner
2026-02-06 12:12     ` Daniel Kral
2026-02-06 12:31       ` Fiona Ebner

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