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.
Vivek
> break;
>
> default:
> @@ -601,6 +607,48 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
> }
> }
>
> + /* Resolve nfs_export -> index dependency */
> + if (config->nfs_export && !config->index) {
> + if (nfs_export_opt && index_opt) {
> + pr_err("conflicting options: nfs_export=on,index=off\n");
> + return -EINVAL;
> + }
> + if (index_opt) {
> + /*
> + * There was an explicit index=off that resulted
> + * in this conflict.
> + */
> + pr_info("disabling nfs_export due to index=off\n");
> + config->nfs_export = false;
> + } else {
> + /* Automatically enable index otherwise. */
> + config->index = true;
> + }
> + }
> +
> + /* Resolve nfs_export -> !metacopy dependency */
> + if (config->nfs_export && config->metacopy) {
> + if (nfs_export_opt && metacopy_opt) {
> + pr_err("conflicting options: nfs_export=on,metacopy=on\n");
> + return -EINVAL;
> + }
> + if (metacopy_opt) {
> + /*
> + * There was an explicit metacopy=on that resulted
> + * in this conflict.
> + */
> + pr_info("disabling nfs_export due to metacopy=on\n");
> + config->nfs_export = false;
> + } else {
> + /*
> + * There was an explicit nfs_export=on that resulted
> + * in this conflict.
> + */
> + pr_info("disabling metacopy due to nfs_export=on\n");
> + config->metacopy = false;
> + }
> + }
> +
> return 0;
> }
>
> --
> 2.17.1
>
