All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] ovl: disable uuid when redirect_dir is used
@ 2021-05-27 14:05 Vyacheslav Yurkov
  2021-05-27 14:05 ` [PATCH v2 2/3] ovl: add ovl_allow_offline_changes() helper Vyacheslav Yurkov
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Vyacheslav Yurkov @ 2021-05-27 14:05 UTC (permalink / raw)
  To: Miklos Szeredi, Amir Goldstein; +Cc: linux-unionfs, Vyacheslav Yurkov

From: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>

Currently decoding origin with lower null uuid is not allowed when user
opted-in to one of the new features that require following the lower inode
of non-dir upper (index, xino, metacopy). Now we add redirect_dir too to
that feature list.

Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
---
 fs/overlayfs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index b01d4147520d..97ea35fdd933 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1600,7 +1600,7 @@ static bool ovl_lower_uuid_ok(struct ovl_fs *ofs, const uuid_t *uuid)
 	 * lower inode of non-dir upper.
 	 */
 	if (!ofs->config.index && !ofs->config.metacopy &&
-	    ofs->config.xino != OVL_XINO_ON &&
+	    !ofs->config.redirect_dir && ofs->config.xino != OVL_XINO_ON &&
 	    uuid_is_null(uuid))
 		return false;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v2 2/3] ovl: add ovl_allow_offline_changes() helper
  2021-05-27 14:05 [PATCH v2 1/3] ovl: disable uuid when redirect_dir is used Vyacheslav Yurkov
@ 2021-05-27 14:05 ` Vyacheslav Yurkov
  2021-05-27 17:32   ` Amir Goldstein
  2021-05-27 14:05 ` [PATCH v2 3/3] ovl: do not set overlay.opaque for new directories Vyacheslav Yurkov
  2021-05-27 17:39 ` [PATCH v2 1/3] ovl: disable uuid when redirect_dir is used Amir Goldstein
  2 siblings, 1 reply; 6+ messages in thread
From: Vyacheslav Yurkov @ 2021-05-27 14:05 UTC (permalink / raw)
  To: Miklos Szeredi, Amir Goldstein; +Cc: linux-unionfs, Vyacheslav Yurkov

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))
 		return false;
 
 	for (i = 0; i < ofs->numfs; i++) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v2 3/3] ovl: do not set overlay.opaque for new directories
  2021-05-27 14:05 [PATCH v2 1/3] ovl: disable uuid when redirect_dir is used Vyacheslav Yurkov
  2021-05-27 14:05 ` [PATCH v2 2/3] ovl: add ovl_allow_offline_changes() helper Vyacheslav Yurkov
@ 2021-05-27 14:05 ` Vyacheslav Yurkov
  2021-05-27 17:34   ` Amir Goldstein
  2021-05-27 17:39 ` [PATCH v2 1/3] ovl: disable uuid when redirect_dir is used Amir Goldstein
  2 siblings, 1 reply; 6+ messages in thread
From: Vyacheslav Yurkov @ 2021-05-27 14:05 UTC (permalink / raw)
  To: Miklos Szeredi, Amir Goldstein; +Cc: linux-unionfs, Vyacheslav Yurkov

From: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>

Disable optimizations if user opted-in for any of extended features.
If optimization is enabled, it breaks existing use case when a lower layer
directory appears after directory was created on a merged layer. If
overlay.opaque is applied, new files on lower layer are not visible.

Consider the following scenario:
- /lower and /upper are mounted to /merged
- directory /merged/new-dir is created with a file test1
- overlay is unmounted
- directory /lower/new-dir is created with a file test2
- overlay is mounted again

If opaque is applied by default, file test2 is not going to be visible
without explicitly clearing the overlay.opaque attribute

Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
---
 fs/overlayfs/dir.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 93efe7048a77..03a22954fe61 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -320,6 +320,7 @@ static bool ovl_type_origin(struct dentry *dentry)
 static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
 			    struct ovl_cattr *attr)
 {
+	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
 	struct inode *udir = upperdir->d_inode;
 	struct dentry *newdentry;
@@ -338,7 +339,8 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
 	if (IS_ERR(newdentry))
 		goto out_unlock;
 
-	if (ovl_type_merge(dentry->d_parent) && d_is_dir(newdentry)) {
+	if (ovl_type_merge(dentry->d_parent) && d_is_dir(newdentry) &&
+	    !ovl_allow_offline_changes(ofs)) {
 		/* Setting opaque here is just an optimization, allow to fail */
 		ovl_set_opaque(dentry, newdentry);
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] ovl: add ovl_allow_offline_changes() helper
  2021-05-27 14:05 ` [PATCH v2 2/3] ovl: add ovl_allow_offline_changes() helper Vyacheslav Yurkov
@ 2021-05-27 17:32   ` Amir Goldstein
  0 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2021-05-27 17:32 UTC (permalink / raw)
  To: Vyacheslav Yurkov; +Cc: Miklos Szeredi, overlayfs, Vyacheslav Yurkov

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] ovl: do not set overlay.opaque for new directories
  2021-05-27 14:05 ` [PATCH v2 3/3] ovl: do not set overlay.opaque for new directories Vyacheslav Yurkov
@ 2021-05-27 17:34   ` Amir Goldstein
  0 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2021-05-27 17:34 UTC (permalink / raw)
  To: Vyacheslav Yurkov; +Cc: Miklos Szeredi, overlayfs, Vyacheslav Yurkov

On Thu, May 27, 2021 at 5:06 PM Vyacheslav Yurkov <uvv.mail@gmail.com> wrote:
>
> From: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
>
> Disable optimizations if user opted-in for any of extended features.

The code is correct. The comment is negated.
Your setup is a default setup - you did NOT opt-in to any new feature.
Only in setups like those we disable the optimization.

Thanks,
Amir.

> If optimization is enabled, it breaks existing use case when a lower layer
> directory appears after directory was created on a merged layer. If
> overlay.opaque is applied, new files on lower layer are not visible.
>
> Consider the following scenario:
> - /lower and /upper are mounted to /merged
> - directory /merged/new-dir is created with a file test1
> - overlay is unmounted
> - directory /lower/new-dir is created with a file test2
> - overlay is mounted again
>
> If opaque is applied by default, file test2 is not going to be visible
> without explicitly clearing the overlay.opaque attribute
>
> Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
> ---
>  fs/overlayfs/dir.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 93efe7048a77..03a22954fe61 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -320,6 +320,7 @@ static bool ovl_type_origin(struct dentry *dentry)
>  static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
>                             struct ovl_cattr *attr)
>  {
> +       struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
>         struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
>         struct inode *udir = upperdir->d_inode;
>         struct dentry *newdentry;
> @@ -338,7 +339,8 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
>         if (IS_ERR(newdentry))
>                 goto out_unlock;
>
> -       if (ovl_type_merge(dentry->d_parent) && d_is_dir(newdentry)) {
> +       if (ovl_type_merge(dentry->d_parent) && d_is_dir(newdentry) &&
> +           !ovl_allow_offline_changes(ofs)) {
>                 /* Setting opaque here is just an optimization, allow to fail */
>                 ovl_set_opaque(dentry, newdentry);
>         }
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] ovl: disable uuid when redirect_dir is used
  2021-05-27 14:05 [PATCH v2 1/3] ovl: disable uuid when redirect_dir is used Vyacheslav Yurkov
  2021-05-27 14:05 ` [PATCH v2 2/3] ovl: add ovl_allow_offline_changes() helper Vyacheslav Yurkov
  2021-05-27 14:05 ` [PATCH v2 3/3] ovl: do not set overlay.opaque for new directories Vyacheslav Yurkov
@ 2021-05-27 17:39 ` Amir Goldstein
  2 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2021-05-27 17:39 UTC (permalink / raw)
  To: Vyacheslav Yurkov; +Cc: Miklos Szeredi, overlayfs, Vyacheslav Yurkov

On Thu, May 27, 2021 at 5:06 PM Vyacheslav Yurkov <uvv.mail@gmail.com> wrote:
>
> From: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
>
> Currently decoding origin with lower null uuid is not allowed when user
> opted-in to one of the new features that require following the lower inode

Confusing. It is not allowed *unless* user opted-in...
I admit the comment could have been more clear.

The commit subject is also not accurate, maybe:
 "ovl: disable decoding null uuid with redirect dir"

Thanks,
Amir.

> of non-dir upper (index, xino, metacopy). Now we add redirect_dir too to
> that feature list.
>
> Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
> ---
>  fs/overlayfs/super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index b01d4147520d..97ea35fdd933 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -1600,7 +1600,7 @@ static bool ovl_lower_uuid_ok(struct ovl_fs *ofs, const uuid_t *uuid)
>          * lower inode of non-dir upper.
>          */
>         if (!ofs->config.index && !ofs->config.metacopy &&
> -           ofs->config.xino != OVL_XINO_ON &&
> +           !ofs->config.redirect_dir && ofs->config.xino != OVL_XINO_ON &&
>             uuid_is_null(uuid))
>                 return false;
>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-27 17:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-27 14:05 [PATCH v2 1/3] ovl: disable uuid when redirect_dir is used Vyacheslav Yurkov
2021-05-27 14:05 ` [PATCH v2 2/3] ovl: add ovl_allow_offline_changes() helper Vyacheslav Yurkov
2021-05-27 17:32   ` Amir Goldstein
2021-05-27 14:05 ` [PATCH v2 3/3] ovl: do not set overlay.opaque for new directories Vyacheslav Yurkov
2021-05-27 17:34   ` Amir Goldstein
2021-05-27 17:39 ` [PATCH v2 1/3] ovl: disable uuid when redirect_dir is used Amir Goldstein

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.