Re: [PATCH v2 2/3] ovl: add ovl_allow_offline_changes() helper

[Amir Goldstein <amir73il@gmail.com>]

On Thu, May 27, 2021 at 5:06 PM Vyacheslav Yurkov <uvv.mail@gmail.com> wrote:
>
> From: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
>
> Allows to check whether any of extended features are enabled
>
> Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
> ---
>  fs/overlayfs/overlayfs.h | 12 ++++++++++++
>  fs/overlayfs/super.c     |  4 +---
>  2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 6ec73db4bf9e..29d71f253db4 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -262,6 +262,18 @@ static inline bool ovl_open_flags_need_copy_up(int flags)
>         return ((OPEN_FMODE(flags) & FMODE_WRITE) || (flags & O_TRUNC));
>  }
>
> +static inline bool ovl_allow_offline_changes(struct ovl_fs *ofs)
> +{
> +       /*
> +        * To avoid regressions in existing setups with overlay lower offline
> +        * changes, we allow lower changes only if none of the new features
> +        * are used.
> +        */
> +       return (!ofs->config.index && !ofs->config.metacopy &&
> +               !ofs->config.redirect_dir && ofs->config.xino != OVL_XINO_ON);
> +}
> +
> +
>  /* util.c */
>  int ovl_want_write(struct dentry *dentry);
>  void ovl_drop_write(struct dentry *dentry);
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 97ea35fdd933..a248cbad9a8c 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -1599,9 +1599,7 @@ static bool ovl_lower_uuid_ok(struct ovl_fs *ofs, const uuid_t *uuid)
>          * user opted-in to one of the new features that require following the
>          * lower inode of non-dir upper.
>          */
> -       if (!ofs->config.index && !ofs->config.metacopy &&
> -           !ofs->config.redirect_dir && ofs->config.xino != OVL_XINO_ON &&
> -           uuid_is_null(uuid))
> +       if (!ovl_allow_offline_changes(ofs) && uuid_is_null(uuid))

You accidently negated the condition.
IOW, allow_lower_changes and null_uuid are conflicting features.

Thanks,
Amir.

>                 return false;
>
>         for (i = 0; i < ofs->numfs; i++) {
> --
> 2.25.1
>