[pve-devel] [PATCH container] start/fix #1478: add check for unsupported config

[Fabian Grünbichler]

On Fri, Aug 18, 2017 at 04:30:33PM +0200, Philip Abernethy wrote:
> Adds a check if an unprivileged container is configured to use
> quota on any of its mountpoints. If so an understandable error
> message is given. Ideally I'd like to catch those
> configurations on the GUI, too, to avoid users just running
> into it.
> ---
>  src/PVE/API2/LXC/Status.pm | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/PVE/API2/LXC/Status.pm b/src/PVE/API2/LXC/Status.pm
> index 89a2fca..58ed569 100644
> --- a/src/PVE/API2/LXC/Status.pm
> +++ b/src/PVE/API2/LXC/Status.pm
> @@ -177,6 +177,16 @@ __PACKAGE__->register_method({
>  			PVE::LXC::Config->check_lock($conf);
>  		    }
>  
> +		    my $unprivileged = $conf->{unprivileged} // 0;

0 is the default anyway, so no need for the ' // 0'. you only have one
statement where you use this, so you can just use $conf->{unprivileged}
directly there.

you only need to take care of bools with defaults of 1, many of which
have a wrapper (e.g., PVE::LXC->has_dev_console, or similarly for int
defaults: PVE::LXC->get_tty_count).

> +
> +		    my $uses_quota = 0;
> +		    PVE::LXC::Config->foreach_mountpoint($conf, sub {
> +			my ($ms, $mountpoint) = @_;
> +			$uses_quota = $uses_quota || $mountpoint->{quota};
> +		    });
> +
> +		    die "Quotas are not supported by unprivileged containers.\n" if ($unprivileged && $uses_quota);

I would restructure this, and only check for quota-enabled mount points
if the container is unprivileged ;) one more level of indentation, but
also one less iteration over and parsing of all mountpoints in case
the container is NOT unprivileged.

added benefit: would make adding further unprivileged-incompatibility
checks more readable (and refactorable) in the future.

> +
>  		    my $storage_cfg = cfs_read_file("storage.cfg");
>  
>  		    PVE::LXC::update_lxc_config($vmid, $conf);
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel