public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH container] fix #4846: Avoid the outdated noacl mount option on ext4
@ 2023-09-01  9:05 Filip Schauer
  2024-02-14 13:57 ` Filip Schauer
  2024-02-16 13:59 ` Fiona Ebner
  0 siblings, 2 replies; 4+ messages in thread
From: Filip Schauer @ 2023-09-01  9:05 UTC (permalink / raw)
  To: pve-devel

Do not use the 'noacl' mount option when mounting a container disk with
an ext4 file system. The option was removed from the kernel in commit
2d544ec923db

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/PVE/LXC.pm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 1e9af0f..3ebce37 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1764,9 +1764,6 @@ sub __mountpoint_mount {
     }
 
     my $acl = $mountpoint->{acl};
-    if (defined($acl)) {
-	push @$optlist, ($acl ? 'acl' : 'noacl');
-    }
 
     my $optstring = join(',', @$optlist);
     my $readonly = $mountpoint->{ro};
@@ -1787,6 +1784,13 @@ sub __mountpoint_mount {
 
 	$format = 'iso' if $vtype eq 'iso'; # allow to handle iso files
 
+	# Avoid the outdated 'noacl' mount option on ext4 file systems
+	if ($scfg->{type} ne 'zfspool') {
+	    push @$optlist, 'acl' if $acl;
+	} elsif (defined($acl)) {
+	    push @$optlist, ($acl ? 'acl' : 'noacl');
+	}
+
 	if ($format eq 'subvol') {
 	    if ($mount_path) {
 		my (undef, $name) = PVE::Storage::parse_volname($storage_cfg, $volid);
-- 
2.39.2





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

* Re: [pve-devel] [PATCH container] fix #4846: Avoid the outdated noacl mount option on ext4
  2023-09-01  9:05 [pve-devel] [PATCH container] fix #4846: Avoid the outdated noacl mount option on ext4 Filip Schauer
@ 2024-02-14 13:57 ` Filip Schauer
  2024-02-16 13:59 ` Fiona Ebner
  1 sibling, 0 replies; 4+ messages in thread
From: Filip Schauer @ 2024-02-14 13:57 UTC (permalink / raw)
  To: pve-devel

bump

On 01/09/2023 11:05, Filip Schauer wrote:
> Do not use the 'noacl' mount option when mounting a container disk with
> an ext4 file system. The option was removed from the kernel in commit
> 2d544ec923db
>
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
>   src/PVE/LXC.pm | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index 1e9af0f..3ebce37 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1764,9 +1764,6 @@ sub __mountpoint_mount {
>       }
>   
>       my $acl = $mountpoint->{acl};
> -    if (defined($acl)) {
> -	push @$optlist, ($acl ? 'acl' : 'noacl');
> -    }
>   
>       my $optstring = join(',', @$optlist);
>       my $readonly = $mountpoint->{ro};
> @@ -1787,6 +1784,13 @@ sub __mountpoint_mount {
>   
>   	$format = 'iso' if $vtype eq 'iso'; # allow to handle iso files
>   
> +	# Avoid the outdated 'noacl' mount option on ext4 file systems
> +	if ($scfg->{type} ne 'zfspool') {
> +	    push @$optlist, 'acl' if $acl;
> +	} elsif (defined($acl)) {
> +	    push @$optlist, ($acl ? 'acl' : 'noacl');
> +	}
> +
>   	if ($format eq 'subvol') {
>   	    if ($mount_path) {
>   		my (undef, $name) = PVE::Storage::parse_volname($storage_cfg, $volid);




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

* Re: [pve-devel] [PATCH container] fix #4846: Avoid the outdated noacl mount option on ext4
  2023-09-01  9:05 [pve-devel] [PATCH container] fix #4846: Avoid the outdated noacl mount option on ext4 Filip Schauer
  2024-02-14 13:57 ` Filip Schauer
@ 2024-02-16 13:59 ` Fiona Ebner
  2024-02-23 10:50   ` Filip Schauer
  1 sibling, 1 reply; 4+ messages in thread
From: Fiona Ebner @ 2024-02-16 13:59 UTC (permalink / raw)
  To: Proxmox VE development discussion, Filip Schauer

Am 01.09.23 um 11:05 schrieb Filip Schauer:
> Do not use the 'noacl' mount option when mounting a container disk with
> an ext4 file system. The option was removed from the kernel in commit
> 2d544ec923db
> 
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
>  src/PVE/LXC.pm | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index 1e9af0f..3ebce37 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1764,9 +1764,6 @@ sub __mountpoint_mount {
>      }
>  
>      my $acl = $mountpoint->{acl};
> -    if (defined($acl)) {
> -	push @$optlist, ($acl ? 'acl' : 'noacl');
> -    }
>  
>      my $optstring = join(',', @$optlist);

This is the last time optlist is read from, so the below hunk won't have
any effect?

>      my $readonly = $mountpoint->{ro};
> @@ -1787,6 +1784,13 @@ sub __mountpoint_mount {
>  
>  	$format = 'iso' if $vtype eq 'iso'; # allow to handle iso files
>  
> +	# Avoid the outdated 'noacl' mount option on ext4 file systems
> +	if ($scfg->{type} ne 'zfspool') {

While we only format Proxmox VE managed volumes with ext4, this mount
code is potentially called for other FS types too. For example, I can
manually format a mount point with BTRFS - where noacl is apparently
still a valid option. While a niche use case, and potentially something
we could risk breaking until somebody complains, maybe we can detect FS
type and filter based on that?

> +	    push @$optlist, 'acl' if $acl;
> +	} elsif (defined($acl)) {
> +	    push @$optlist, ($acl ? 'acl' : 'noacl');
> +	}
> +
>  	if ($format eq 'subvol') {
>  	    if ($mount_path) {
>  		my (undef, $name) = PVE::Storage::parse_volname($storage_cfg, $volid);




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

* Re: [pve-devel] [PATCH container] fix #4846: Avoid the outdated noacl mount option on ext4
  2024-02-16 13:59 ` Fiona Ebner
@ 2024-02-23 10:50   ` Filip Schauer
  0 siblings, 0 replies; 4+ messages in thread
From: Filip Schauer @ 2024-02-23 10:50 UTC (permalink / raw)
  To: Fiona Ebner, Proxmox VE development discussion

A v2 of this patch is available:

https://lists.proxmox.com/pipermail/pve-devel/2024-February/061932.html

On 16/02/2024 14:59, Fiona Ebner wrote:
> Am 01.09.23 um 11:05 schrieb Filip Schauer:
>> Do not use the 'noacl' mount option when mounting a container disk with
>> an ext4 file system. The option was removed from the kernel in commit
>> 2d544ec923db
>>
>> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
>> ---
>>   src/PVE/LXC.pm | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
>> index 1e9af0f..3ebce37 100644
>> --- a/src/PVE/LXC.pm
>> +++ b/src/PVE/LXC.pm
>> @@ -1764,9 +1764,6 @@ sub __mountpoint_mount {
>>       }
>>   
>>       my $acl = $mountpoint->{acl};
>> -    if (defined($acl)) {
>> -	push @$optlist, ($acl ? 'acl' : 'noacl');
>> -    }
>>   
>>       my $optstring = join(',', @$optlist);
> This is the last time optlist is read from, so the below hunk won't have
> any effect?
>
>>       my $readonly = $mountpoint->{ro};
>> @@ -1787,6 +1784,13 @@ sub __mountpoint_mount {
>>   
>>   	$format = 'iso' if $vtype eq 'iso'; # allow to handle iso files
>>   
>> +	# Avoid the outdated 'noacl' mount option on ext4 file systems
>> +	if ($scfg->{type} ne 'zfspool') {
> While we only format Proxmox VE managed volumes with ext4, this mount
> code is potentially called for other FS types too. For example, I can
> manually format a mount point with BTRFS - where noacl is apparently
> still a valid option. While a niche use case, and potentially something
> we could risk breaking until somebody complains, maybe we can detect FS
> type and filter based on that?
>
>> +	    push @$optlist, 'acl' if $acl;
>> +	} elsif (defined($acl)) {
>> +	    push @$optlist, ($acl ? 'acl' : 'noacl');
>> +	}
>> +
>>   	if ($format eq 'subvol') {
>>   	    if ($mount_path) {
>>   		my (undef, $name) = PVE::Storage::parse_volname($storage_cfg, $volid);




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

end of thread, other threads:[~2024-02-23 10:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-01  9:05 [pve-devel] [PATCH container] fix #4846: Avoid the outdated noacl mount option on ext4 Filip Schauer
2024-02-14 13:57 ` Filip Schauer
2024-02-16 13:59 ` Fiona Ebner
2024-02-23 10:50   ` Filip Schauer

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