On Fri, Apr 10, 2020 at 12:49 AM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>
> On Thu, Apr 09, 2020 at 07:39:02PM +0300, Amir Goldstein wrote:
> > Similar to the way that a conflict between metacopy=on,redirect_dir=off
> > is resolved, also resolve conflicts between nfs_export=on,index=off and
> > nfs_export=on,metacopy=on.
> >
> > An explicit mount option wins over a default config value.
> > Both explicit mount options result in an error.
> >
> > Without this change the xfstests group overlay/exportfs are skipped if
> > metacopy is enabled by default.
> >
> > Reported-by: Chengguang Xu <cgxu519@xxxxxxxxxxxx>
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> > Documentation/filesystems/overlayfs.rst | 7 ++--
> > fs/overlayfs/super.c | 48 +++++++++++++++++++++++++
> > 2 files changed, 53 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> > index c9d2bf96b02d..660dbaf0b9b8 100644
> > --- a/Documentation/filesystems/overlayfs.rst
> > +++ b/Documentation/filesystems/overlayfs.rst
> > @@ -365,8 +365,8 @@ pointed by REDIRECT. This should not be possible on local system as setting
> > "trusted." xattrs will require CAP_SYS_ADMIN. But it should be possible
> > for untrusted layers like from a pen drive.
> >
> > -Note: redirect_dir={off|nofollow|follow[*]} conflicts with metacopy=on, and
> > -results in an error.
> > +Note: redirect_dir={off|nofollow|follow[*]} and nfs_export=on mount options
> > +conflict with metacopy=on, and will result in an error.
> >
> > [*] redirect_dir=follow only conflicts with metacopy=on if upperdir=... is
> > given.
> > @@ -560,6 +560,9 @@ When the NFS export feature is enabled, all directory index entries are
> > verified on mount time to check that upper file handles are not stale.
> > This verification may cause significant overhead in some cases.
> >
> > +Note: the mount options index=off,nfs_export=on are conflicting and will
> > +result in an error.
> > +
> >
> > Testsuite
> > ---------
> > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > index 732ad5495c92..fbd6207acdbf 100644
> > --- a/fs/overlayfs/super.c
> > +++ b/fs/overlayfs/super.c
> > @@ -470,6 +470,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
> > char *p;
> > int err;
> > bool metacopy_opt = false, redirect_opt = false;
> > + bool nfs_export_opt = false, index_opt = false;
> >
> > config->redirect_mode = kstrdup(ovl_redirect_mode_def(), GFP_KERNEL);
> > if (!config->redirect_mode)
> > @@ -519,18 +520,22 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
> >
> > case OPT_INDEX_ON:
> > config->index = true;
> > + index_opt = true;
> > break;
> >
> > case OPT_INDEX_OFF:
> > config->index = false;
> > + index_opt = true;
> > break;
> >
> > case OPT_NFS_EXPORT_ON:
> > config->nfs_export = true;
> > + nfs_export_opt = true;
> > break;
> >
> > case OPT_NFS_EXPORT_OFF:
> > config->nfs_export = false;
> > + nfs_export_opt = true;
> > break;
> >
> > case OPT_XINO_ON:
> > @@ -552,6 +557,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
> >
> > case OPT_METACOPY_OFF:
> > config->metacopy = false;
> > + metacopy_opt = true;
>
> Hi Amir,
>
> I am wondering why metacopy_opt needs to be set for OPT_METACOPY_OFF case.
> In this case config->metacopy=false and it does not conflict with
> config->nfs_export at all. So there is no need to know if metacopy=off
> was specified as mount option or not.
>
It's true. We can drop that one.
I just liked it better that the meaning of the _opt vars are
"was this value set explicitly", even before we get to using it.
Thanks,
Amir.
