Re: [PATCH v2] ovl: return error on mount if metacopy cannot be enabled

[Vivek Goyal <vgoyal@redhat.com>]

On Wed, Oct 31, 2018 at 01:35:38PM +0100, Miklos Szeredi wrote:
> On Wed, Oct 31, 2018 at 1:26 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Wed, Oct 31, 2018 at 2:19 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> 
> > My 'strict' patch did not try to deal with implicitly enabling redirect_dir
> > Vivek posted a different patch to deal with that.
> 
> What I mean, is that sections of your patch dealing with the
> redirect_dir conflict and sections of Vivek's patch dealing with
> _enforce are completely unnecessary if we just define the rules of
> implicit enable/disable between metacopy and redirect_dir.
> 
> >>                         config->redirect_mode = match_strdup(&args[0]);
> >>                         if (!config->redirect_mode)
> >>                                 return -ENOMEM;
> >> +
> >> +                       /* "redirect_dir=off" implies "metacopy=off" */
> >> +                       if (strcmp(config->redirect_mode, "off") == 0 ||
> >> +                           strcmp(config->redirect_mode, "nofollow") == 0)
> >> +                               config->metacopy = false;
> >
> > This look completely counter to the 'strict' behavior definition.
> > If user specified metacopy=on, we are not allowed to end up with metacopy=off.
> 
> What about "metacopy=on,metacopy=off"?

I would think that ordering will matter. metacopy=off will override,
metacopy=on (because its later in the order).

> 
> What I'm saying is that "metacopy=on,redirect_dir=off" should be
> exactly equivalent to the above, if taking the implicit disable rule
> into account.

I am wondering why do we have to accept nonsense options possibilities.
Why not simply reject them with -EINVAL. IMHO, that behavior is much
simpler to deal with than implicitly disabling metacopy because
redirect_dir=off.

If some options don't make sense together, then it makes sense to return
error and let user understand it and fix it. 

Thanks
Vivek