Re: [pve-devel] [PATCH container v3] fix #3516: fix unmanaged containers

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 20.07.21 13:40, Wolfgang Bumiller wrote:
> On Wed, Jul 14, 2021 at 11:51:51AM +0200, Oguz Bektas wrote:
>> unmanaged containers should run the unified cgroupv2 code from our base
>> plugin so that they can start correctly instead of erroring out
>>
>> Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
>> Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>
>> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
>> ---
>> v2-> v3:
>> * added comment from stoiko's reply
>>
>>
>>  src/PVE/LXC/Setup.pm | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm
>> index 9abdc85..4408dcc 100644
>> --- a/src/PVE/LXC/Setup.pm
>> +++ b/src/PVE/LXC/Setup.pm
>> @@ -424,6 +424,10 @@ sub get_ct_os_release {
>>  sub unified_cgroupv2_support {
>>      my ($self) = @_;
>>  
>> +    # code in base plugin is a generic check and should work
>> +    # for most distributions
>> +    $self->{plugin} //= 'PVE::LXC::Setup::Base'; # unmanaged
> 
> This has the side effect that all later checks for unmanaged containers
> via `$self->{plugin}` are broken.
> Please either move this *into* the `protected_call` below (and add a
> comment that the assignment is temporary due to how `protected_call`
> works), or cleanup this change afterwards (but that would need to be
> `die`-safe (iow. would need an eval around the `protected_call`)
> 

for that it could have just used a local variable, e.g.:

my $plugin = $self->{plugin} // 'PVE::LXC::Setup::Base'; # fallback to base for unmanaged

$self->protected_call(sub { $plugin->unified_cgroupv2_support() });

but I'd rather avoid adding more of those "unmanaged hacks" in general.