[pve-devel] applied: [PATCH v3 container 2/2] fix #1451: add mountoptions to lxc

[Thomas Lamprecht]

On 7/5/19 1:27 PM, Oguz Bektas wrote:
> for now allows:
> * noexec
> * noatime
> * nosuid
> * nodev
> 
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
> 
> v2 -> v3:
> * have list of supported mount options only at one place

applied, but with some addition to the commit message and a followup,
see below

> 
>  src/PVE/LXC.pm        | 12 ++++++++++--
>  src/PVE/LXC/Config.pm | 13 +++++++++++++
>  2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index 4922fb0..13ead7f 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1415,11 +1415,19 @@ sub mountpoint_mount {
>  
>      die "unknown snapshot path for '$volid'" if !$storage && defined($snapname);
>  
> -    my $optstring = '';
> +    my @mountoptions= split(/;/, $mountpoint->{mountoptions});

produced a 'Use of uninitialized value in split', if not set..

> +    my $optlist = [];
> +    my $allowed_options = PVE::LXC::Config::get_mount_options();
> +    foreach my $opt (@mountoptions) {
> +	push @$optlist, $opt if $opt =~ $allowed_options

I changed the get_mount_options into a is_valid_mount_option method,
makes above a bit easier to write/read.

> +    }
> +
>      my $acl = $mountpoint->{acl};
>      if (defined($acl)) {
> -	$optstring .= ($acl ? 'acl' : 'noacl');
> +	push @$optlist, ($acl ? 'acl' : 'noacl');
>      }
> +
> +    my $optstring = join(',', @$optlist);
>      my $readonly = $mountpoint->{ro};
>  
>      my @extra_opts;
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index 8dcd73c..71788ba 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -216,6 +216,12 @@ sub __snapshot_foreach_volume {
>  
>  cfs_register_file('/lxc/', \&parse_pct_config, \&write_pct_config);
>  
> +my $mount_option = qr/(noatime|nodev|nosuid|noexec)/;
> +
> +sub get_mount_options {
> +    return $mount_option;
> +}
> +
>  my $rootfs_desc = {
>      volume => {
>  	type => 'string',
> @@ -236,6 +242,13 @@ my $rootfs_desc = {
>  	description => 'Explicitly enable or disable ACL support.',
>  	optional => 1,
>      },
> +    mountoptions => {
> +	optional => 1,
> +	type => 'string',
> +	description => 'Extra mount options for rootfs/mps.',
> +	format_description => 'opt[;opt...]',
> +	pattern => qr/$mount_option(;$mount_option)*/,
> +    },

this is in the rootfs description, where noexec and nodev highly probably
does *not* make sense, or?

At least LXC always tries to start /sbin/init, so if /sbin is not on it's
own MP this will fail all CT starts, and as currently no nice error message
is visible in the task log, it is not always easy to understand the cause.
(albeit that should be solved in making the error visible in the task log..)

Maybe at least filter those out in the webUI for rootfs?

>      ro => {
>  	type => 'boolean',
>  	description => 'Read-only mount point',
>