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

[Fabian Grünbichler]

On Tue, Aug 22, 2017 at 01:26:14PM +0200, Dominik Csapak wrote:
> On 08/22/2017 12:32 PM, 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.
> > ---
> > Applied your suggestions.
> > $unprivileged doesn't use // 1 any more.
> > Whether quotas are used will only be checked for unprivileged containers.
> > 
> >   src/PVE/API2/LXC/Status.pm | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> > 
> > diff --git a/src/PVE/API2/LXC/Status.pm b/src/PVE/API2/LXC/Status.pm
> > index 89a2fca..106b0eb 100644
> > --- a/src/PVE/API2/LXC/Status.pm
> > +++ b/src/PVE/API2/LXC/Status.pm
> > @@ -177,6 +177,18 @@ __PACKAGE__->register_method({
> >   			PVE::LXC::Config->check_lock($conf);
> >   		    }
> > +		    my $unprivileged = $conf->{unprivileged};
> > +
> > +		    if ($unprivileged) {
> > +			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 ($uses_quota);
> 
> would it not be better to die directly in the loop, so that we do not have
> to check the remaining mountpoints if we find an invalid configuration?

@Philip:

if you do send a v3, please drop the $unprivileged altogether in favor
of $conf->{unprivileged}, and fix the commit message (it should start
with 'fix #XXX:' or 'close: #XXX', to allow matching
patches/commits/package changelogs to bug tracker entries).