linux-unionfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/3] ovl: disable decoding null uuid with redirect dir
@ 2021-05-27 17:45 Vyacheslav Yurkov
  2021-05-27 17:45 ` [PATCH v3 2/3] ovl: add ovl_allow_offline_changes() helper Vyacheslav Yurkov
  2021-05-27 17:45 ` [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories Vyacheslav Yurkov
  0 siblings, 2 replies; 12+ messages in thread
From: Vyacheslav Yurkov @ 2021-05-27 17:45 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 unless 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] 12+ messages in thread

* [PATCH v3 2/3] ovl: add ovl_allow_offline_changes() helper
  2021-05-27 17:45 [PATCH v3 1/3] ovl: disable decoding null uuid with redirect dir Vyacheslav Yurkov
@ 2021-05-27 17:45 ` Vyacheslav Yurkov
  2021-05-27 17:45 ` [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories Vyacheslav Yurkov
  1 sibling, 0 replies; 12+ messages in thread
From: Vyacheslav Yurkov @ 2021-05-27 17:45 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..178daa5e82c9 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] 12+ messages in thread

* [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories
  2021-05-27 17:45 [PATCH v3 1/3] ovl: disable decoding null uuid with redirect dir Vyacheslav Yurkov
  2021-05-27 17:45 ` [PATCH v3 2/3] ovl: add ovl_allow_offline_changes() helper Vyacheslav Yurkov
@ 2021-05-27 17:45 ` Vyacheslav Yurkov
  2021-05-28 11:38   ` Amir Goldstein
  2021-07-19 14:29   ` Miklos Szeredi
  1 sibling, 2 replies; 12+ messages in thread
From: Vyacheslav Yurkov @ 2021-05-27 17:45 UTC (permalink / raw)
  To: Miklos Szeredi, Amir Goldstein; +Cc: linux-unionfs, Vyacheslav Yurkov

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

Enable optimizations only 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] 12+ messages in thread

* Re: [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories
  2021-05-27 17:45 ` [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories Vyacheslav Yurkov
@ 2021-05-28 11:38   ` Amir Goldstein
  2021-06-01  6:53     ` Yurkov, Vyacheslav
  2021-07-19 14:29   ` Miklos Szeredi
  1 sibling, 1 reply; 12+ messages in thread
From: Amir Goldstein @ 2021-05-28 11:38 UTC (permalink / raw)
  To: Vyacheslav Yurkov; +Cc: Miklos Szeredi, overlayfs, Vyacheslav Yurkov

On Thu, May 27, 2021 at 8:46 PM Vyacheslav Yurkov <uvv.mail@gmail.com> wrote:
>
> From: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
>
> Enable optimizations only 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>
> ---

Vyacheslav,

The series looks good.
In case you post another version please add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

No need to repost just for that.

Did you happen to run xfstests on these patches?

If I am not mistaken, tests overlay/068 and overlay/069 provide
test coverage to the check in ovl_lower_uuid_ok() for the case
of lower null uuid (lower overlayfs) and user opt-in to new features
(nfs_export), so tests would have caught the bug you had in v2.

Please verify that I am not wrong (about test catching v2 bug) and
that v3 passes at least:
./check -overlay -g overlay/quick -g overlay/union

Please see README.overlay in xfstests for setting up to run
./check -overlay and the unionmount tests.

Thanks,
Amir.


>  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] 12+ messages in thread

* RE: [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories
  2021-05-28 11:38   ` Amir Goldstein
@ 2021-06-01  6:53     ` Yurkov, Vyacheslav
  2021-06-01  8:33       ` Amir Goldstein
  0 siblings, 1 reply; 12+ messages in thread
From: Yurkov, Vyacheslav @ 2021-06-01  6:53 UTC (permalink / raw)
  To: Amir Goldstein, Vyacheslav Yurkov; +Cc: Miklos Szeredi, overlayfs

[-- Attachment #1: Type: text/plain, Size: 4302 bytes --]

Hi Amir,
Thanks again for the review and a heads-up about the tests. I was not aware they exist.

It took me some time to set them up due to really peculiar Makefile, but I now even have a yocto recipe to build them (will file it upstream later).

The latest master and my v3 both report the same results:
Failures: overlay/005 overlay/065 overlay/075
Failed 3 of 93 tests
(The full log is attached)

I assume the failures come due to my specific configuration, but since master and v3 issue the same results I should be fine here.

v2 indeed caused a few more failures on top of that:
Failures: overlay/005 overlay/065 overlay/070 overlay/071 overlay/075
Failed 5 of 93 tests

Could you please tell me just for my information what's the usual time frame to have my v3 mainlined?

Thanks,
Vyacheslav


- confidential -

> -----Original Message-----
> From: Amir Goldstein <amir73il@gmail.com>
> Sent: Friday, May 28, 2021 13:39
> To: Vyacheslav Yurkov <uvv.mail@gmail.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>; overlayfs <linux-
> unionfs@vger.kernel.org>; Yurkov, Vyacheslav
> <Vyacheslav.Yurkov@bruker.com>
> Subject: Re: [PATCH v3 3/3] ovl: do not set overlay.opaque for new
> directories
> 
> **EXTERNAL EMAIL**
> 
> On Thu, May 27, 2021 at 8:46 PM Vyacheslav Yurkov <uvv.mail@gmail.com>
> wrote:
> >
> > From: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
> >
> > Enable optimizations only 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>
> > ---
> 
> Vyacheslav,
> 
> The series looks good.
> In case you post another version please add:
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> No need to repost just for that.
> 
> Did you happen to run xfstests on these patches?
> 
> If I am not mistaken, tests overlay/068 and overlay/069 provide
> test coverage to the check in ovl_lower_uuid_ok() for the case
> of lower null uuid (lower overlayfs) and user opt-in to new features
> (nfs_export), so tests would have caught the bug you had in v2.
> 
> Please verify that I am not wrong (about test catching v2 bug) and
> that v3 passes at least:
> ./check -overlay -g overlay/quick -g overlay/union
> 
> Please see README.overlay in xfstests for setting up to run
> ./check -overlay and the unionmount tests.
> 
> Thanks,
> Amir.
> 
> 
> >  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
> >

[-- Attachment #2: check-patched.log --]
[-- Type: application/octet-stream, Size: 1587 bytes --]


Tue Jun  1 06:46:36 UTC 2021
Ran: overlay/001 overlay/002 overlay/003 overlay/004 overlay/005 overlay/006 overlay/007 overlay/008 overlay/009 overlay/010 overlay/011 overlay/012 overlay/013 overlay/014 overlay/015 overlay/016 overlay/017 overlay/018 overlay/020 overlay/021 overlay/022 overlay/023 overlay/024 overlay/025 overlay/026 overlay/027 overlay/028 overlay/029 overlay/030 overlay/031 overlay/032 overlay/033 overlay/034 overlay/035 overlay/036 overlay/037 overlay/038 overlay/039 overlay/040 overlay/041 overlay/042 overlay/043 overlay/044 overlay/045 overlay/046 overlay/047 overlay/048 overlay/049 overlay/050 overlay/051 overlay/052 overlay/053 overlay/054 overlay/055 overlay/056 overlay/057 overlay/058 overlay/059 overlay/060 overlay/062 overlay/063 overlay/064 overlay/065 overlay/066 overlay/067 overlay/068 overlay/069 overlay/070 overlay/071 overlay/072 overlay/073 overlay/074 overlay/075 overlay/076 overlay/077 overlay/100 overlay/101 overlay/102 overlay/103 overlay/104 overlay/105 overlay/106 overlay/107 overlay/108 overlay/109 overlay/110 overlay/111 overlay/112 overlay/113 overlay/114 overlay/115 overlay/116 overlay/117
Not run: overlay/001 overlay/004 overlay/008 overlay/015 overlay/020 overlay/021 overlay/025 overlay/032 overlay/045 overlay/046 overlay/056 overlay/064 overlay/100 overlay/101 overlay/102 overlay/103 overlay/104 overlay/105 overlay/106 overlay/107 overlay/108 overlay/109 overlay/110 overlay/111 overlay/112 overlay/113 overlay/114 overlay/115 overlay/116 overlay/117
Failures: overlay/005 overlay/065 overlay/075
Failed 3 of 93 tests

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories
  2021-06-01  6:53     ` Yurkov, Vyacheslav
@ 2021-06-01  8:33       ` Amir Goldstein
  2021-06-01 16:29         ` Yurkov, Vyacheslav
  0 siblings, 1 reply; 12+ messages in thread
From: Amir Goldstein @ 2021-06-01  8:33 UTC (permalink / raw)
  To: Yurkov, Vyacheslav; +Cc: Vyacheslav Yurkov, Miklos Szeredi, overlayfs

On Tue, Jun 1, 2021 at 9:54 AM Yurkov, Vyacheslav
<Vyacheslav.Yurkov@bruker.com> wrote:
>
> Hi Amir,
> Thanks again for the review and a heads-up about the tests. I was not aware they exist.
>
> It took me some time to set them up due to really peculiar Makefile, but I now even have a yocto recipe to build them (will file it upstream later).
>
> The latest master and my v3 both report the same results:
> Failures: overlay/005 overlay/065 overlay/075
> Failed 3 of 93 tests
> (The full log is attached)
>
> I assume the failures come due to my specific configuration, but since master and v3 issue the same results I should be fine here.

Maybe.
Failure of overlay/075 on master is known.
Failure of overlay/065 on master was fixed by xfstests commit
* 6159ae7f - overlay/065: Adapt test to relaxed rules
so you may want to update your xfstests copy.

Failure of overlay/005 is not familiar to me and the
attached log is missing all the output of the test -
it just has the summary.

Worst yet, according to summary, all those test do not run in your setup:
Not run: overlay/001 overlay/004 overlay/008 overlay/015 overlay/020
overlay/021 overlay/025 overlay/032 overlay/045 overlay/046
overlay/056 overlay/064 overlay/100 overlay/101 overlay/102
overlay/103 overlay/104 overlay/105 overlay/106 overlay/107
overlay/108 overlay/109 overlay/110 overlay/111 overlay/112
overlay/113 overlay/114 overlay/115 overlay/116 overlay/117

Can you provide the full log to understand the reason or figure it out yourself
and fix this.
If you are running a special setup that is fine, it doesn't have to
run all the test
(as long as you know why), but in order to verify that your patches did not
break other setups, you need to test with a common setup where all the
above tests run and pass, short of overlay/075 which is a known upstream issue.


>
> v2 indeed caused a few more failures on top of that:
> Failures: overlay/005 overlay/065 overlay/070 overlay/071 overlay/075
> Failed 5 of 93 tests
>

I'm a bit surprised that tests overlay/068 overlay/069 did not fail with v2
Maybe they did not run and you did not notice that in the report?

> Could you please tell me just for my information what's the usual time frame to have my v3 mainlined?
>

Miklos would have to answer this question.
It shouldn't take long if he agrees with the patch.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories
  2021-06-01  8:33       ` Amir Goldstein
@ 2021-06-01 16:29         ` Yurkov, Vyacheslav
  2021-06-01 18:29           ` Amir Goldstein
  0 siblings, 1 reply; 12+ messages in thread
From: Yurkov, Vyacheslav @ 2021-06-01 16:29 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Vyacheslav Yurkov, Miklos Szeredi, overlayfs

[-- Attachment #1: Type: text/plain, Size: 2294 bytes --]

Hi Amir,
See below.

> Maybe.
> Failure of overlay/075 on master is known.
> Failure of overlay/065 on master was fixed by xfstests commit
> * 6159ae7f - overlay/065: Adapt test to relaxed rules
> so you may want to update your xfstests copy.

Using the latest master now, 065 is still failing for me.

> Failure of overlay/005 is not familiar to me and the
> attached log is missing all the output of the test -
> it just has the summary.

The reason was simple, it required xfs as underlying file system (and support in the kernel), but I had only ext4.

> Worst yet, according to summary, all those test do not run in your setup:
> Not run: overlay/001 overlay/004 overlay/008 overlay/015 overlay/020
> overlay/021 overlay/025 overlay/032 overlay/045 overlay/046
> overlay/056 overlay/064 overlay/100 overlay/101 overlay/102
> overlay/103 overlay/104 overlay/105 overlay/106 overlay/107
> overlay/108 overlay/109 overlay/110 overlay/111 overlay/112
> overlay/113 overlay/114 overlay/115 overlay/116 overlay/117
> 
> Can you provide the full log to understand the reason or figure it out yourself
> and fix this.
> If you are running a special setup that is fine, it doesn't have to
> run all the test
> (as long as you know why), but in order to verify that your patches did not
> break other setups, you need to test with a common setup where all the
> above tests run and pass, short of overlay/075 which is a known upstream
> issue.

It seems most of them require unionmount testsuite, which is not a part of xfstests package, and I missed your hint that I should check README.overlay ☹.
Some of the other not-running tests needed more spaces (> 1GB), which I don't have on my device. And a few more required a dedicated user/group on the system.

I added unionmount testsuite and sending my two test results.

> > v2 indeed caused a few more failures on top of that:
> > Failures: overlay/005 overlay/065 overlay/070 overlay/071 overlay/075
> > Failed 5 of 93 tests
> >
> 
> I'm a bit surprised that tests overlay/068 overlay/069 did not fail with v2
> Maybe they did not run and you did not notice that in the report?

It did not indeed.

Anyways, v3 results look pretty good now, I think.

Regards,
Vyacheslav

- confidential -

[-- Attachment #2: xfstests-v2.tar.xz --]
[-- Type: application/octet-stream, Size: 25716 bytes --]

[-- Attachment #3: xfstests-v3.tar.xz --]
[-- Type: application/octet-stream, Size: 25460 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories
  2021-06-01 16:29         ` Yurkov, Vyacheslav
@ 2021-06-01 18:29           ` Amir Goldstein
  2021-06-04 12:32             ` Amir Goldstein
  0 siblings, 1 reply; 12+ messages in thread
From: Amir Goldstein @ 2021-06-01 18:29 UTC (permalink / raw)
  To: Yurkov, Vyacheslav; +Cc: Vyacheslav Yurkov, Miklos Szeredi, overlayfs

On Tue, Jun 1, 2021 at 7:29 PM Yurkov, Vyacheslav
<Vyacheslav.Yurkov@bruker.com> wrote:
>
> Hi Amir,
> See below.
>
> > Maybe.
> > Failure of overlay/075 on master is known.
> > Failure of overlay/065 on master was fixed by xfstests commit
> > * 6159ae7f - overlay/065: Adapt test to relaxed rules
> > so you may want to update your xfstests copy.
>
> Using the latest master now, 065 is still failing for me.
>
> > Failure of overlay/005 is not familiar to me and the
> > attached log is missing all the output of the test -
> > it just has the summary.
>
> The reason was simple, it required xfs as underlying file system (and support in the kernel), but I had only ext4.
>
> > Worst yet, according to summary, all those test do not run in your setup:
> > Not run: overlay/001 overlay/004 overlay/008 overlay/015 overlay/020
> > overlay/021 overlay/025 overlay/032 overlay/045 overlay/046
> > overlay/056 overlay/064 overlay/100 overlay/101 overlay/102
> > overlay/103 overlay/104 overlay/105 overlay/106 overlay/107
> > overlay/108 overlay/109 overlay/110 overlay/111 overlay/112
> > overlay/113 overlay/114 overlay/115 overlay/116 overlay/117
> >
> > Can you provide the full log to understand the reason or figure it out yourself
> > and fix this.
> > If you are running a special setup that is fine, it doesn't have to
> > run all the test
> > (as long as you know why), but in order to verify that your patches did not
> > break other setups, you need to test with a common setup where all the
> > above tests run and pass, short of overlay/075 which is a known upstream
> > issue.
>
> It seems most of them require unionmount testsuite, which is not a part of xfstests package, and I missed your hint that I should check README.overlay ☹.
> Some of the other not-running tests needed more spaces (> 1GB), which I don't have on my device. And a few more required a dedicated user/group on the system.
>

Ok we still need to verify no regressions on those skipped test.
I can run them on my setup when I get the time if you cannot find
another test setup which meets the requirements.

> I added unionmount testsuite and sending my two test results.
>
> > > v2 indeed caused a few more failures on top of that:
> > > Failures: overlay/005 overlay/065 overlay/070 overlay/071 overlay/075
> > > Failed 5 of 93 tests
> > >
> >
> > I'm a bit surprised that tests overlay/068 overlay/069 did not fail with v2
> > Maybe they did not run and you did not notice that in the report?
>
> It did not indeed.
>
> Anyways, v3 results look pretty good now, I think.
>

Yes, looking better.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories
  2021-06-01 18:29           ` Amir Goldstein
@ 2021-06-04 12:32             ` Amir Goldstein
  2021-06-04 14:50               ` Yurkov, Vyacheslav
  0 siblings, 1 reply; 12+ messages in thread
From: Amir Goldstein @ 2021-06-04 12:32 UTC (permalink / raw)
  To: Yurkov, Vyacheslav; +Cc: Vyacheslav Yurkov, Miklos Szeredi, overlayfs

On Tue, Jun 1, 2021 at 9:29 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Tue, Jun 1, 2021 at 7:29 PM Yurkov, Vyacheslav
> <Vyacheslav.Yurkov@bruker.com> wrote:
> >
> > Hi Amir,
> > See below.
> >
> > > Maybe.
> > > Failure of overlay/075 on master is known.
> > > Failure of overlay/065 on master was fixed by xfstests commit
> > > * 6159ae7f - overlay/065: Adapt test to relaxed rules
> > > so you may want to update your xfstests copy.
> >
> > Using the latest master now, 065 is still failing for me.
> >
> > > Failure of overlay/005 is not familiar to me and the
> > > attached log is missing all the output of the test -
> > > it just has the summary.
> >
> > The reason was simple, it required xfs as underlying file system (and support in the kernel), but I had only ext4.
> >
> > > Worst yet, according to summary, all those test do not run in your setup:
> > > Not run: overlay/001 overlay/004 overlay/008 overlay/015 overlay/020
> > > overlay/021 overlay/025 overlay/032 overlay/045 overlay/046
> > > overlay/056 overlay/064 overlay/100 overlay/101 overlay/102
> > > overlay/103 overlay/104 overlay/105 overlay/106 overlay/107
> > > overlay/108 overlay/109 overlay/110 overlay/111 overlay/112
> > > overlay/113 overlay/114 overlay/115 overlay/116 overlay/117
> > >
> > > Can you provide the full log to understand the reason or figure it out yourself
> > > and fix this.
> > > If you are running a special setup that is fine, it doesn't have to
> > > run all the test
> > > (as long as you know why), but in order to verify that your patches did not
> > > break other setups, you need to test with a common setup where all the
> > > above tests run and pass, short of overlay/075 which is a known upstream
> > > issue.
> >
> > It seems most of them require unionmount testsuite, which is not a part of xfstests package, and I missed your hint that I should check README.overlay ☹.
> > Some of the other not-running tests needed more spaces (> 1GB), which I don't have on my device. And a few more required a dedicated user/group on the system.
> >
>
> Ok we still need to verify no regressions on those skipped test.
> I can run them on my setup when I get the time if you cannot find
> another test setup which meets the requirements.
>

Run the overlay/quick tests on your patches.
No regressions.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories
  2021-06-04 12:32             ` Amir Goldstein
@ 2021-06-04 14:50               ` Yurkov, Vyacheslav
  2021-06-04 19:18                 ` Amir Goldstein
  0 siblings, 1 reply; 12+ messages in thread
From: Yurkov, Vyacheslav @ 2021-06-04 14:50 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Vyacheslav Yurkov, Miklos Szeredi, overlayfs

You were a bit faster than me 😊
I was able to extend my system to provide all needed utilities / kernel features and also got all the tests run.

But to not loose my progress, I'd like to contribute my yocto recipe to the meta-openembedded project. It would be great if you could clarify a few things for me.

1) The base test harness is xfstests, URL: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
2) Ovelayfs testsuite, URL: https://github.com/amir73il/unionmount-testsuite.git
3) The test suite also requires fsck.overlay, URL: https://github.com/hisilicon/overlayfs-progs

Are all the URLs are correct?
#2 and #3 are official repositories so to say?

Thanks,
Vyacheslav


- confidential -

> -----Original Message-----
> From: Amir Goldstein <amir73il@gmail.com>
> Sent: Friday, June 4, 2021 14:33
> To: Yurkov, Vyacheslav <Vyacheslav.Yurkov@bruker.com>
> Cc: Vyacheslav Yurkov <uvv.mail@gmail.com>; Miklos Szeredi
> <miklos@szeredi.hu>; overlayfs <linux-unionfs@vger.kernel.org>
> Subject: Re: [PATCH v3 3/3] ovl: do not set overlay.opaque for new
> directories
> 
> > Ok we still need to verify no regressions on those skipped test.
> > I can run them on my setup when I get the time if you cannot find
> > another test setup which meets the requirements.
> >
> 
> Run the overlay/quick tests on your patches.
> No regressions.
> 
> Thanks,
> Amir.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories
  2021-06-04 14:50               ` Yurkov, Vyacheslav
@ 2021-06-04 19:18                 ` Amir Goldstein
  0 siblings, 0 replies; 12+ messages in thread
From: Amir Goldstein @ 2021-06-04 19:18 UTC (permalink / raw)
  To: Yurkov, Vyacheslav
  Cc: Vyacheslav Yurkov, Miklos Szeredi, overlayfs, zhangyi (F)

On Fri, Jun 4, 2021 at 5:51 PM Yurkov, Vyacheslav
<Vyacheslav.Yurkov@bruker.com> wrote:
>
> You were a bit faster than me
> I was able to extend my system to provide all needed utilities / kernel features and also got all the tests run.
>
> But to not loose my progress, I'd like to contribute my yocto recipe to the meta-openembedded project. It would be great if you could clarify a few things for me.
>
> 1) The base test harness is xfstests, URL: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
> 2) Ovelayfs testsuite, URL: https://github.com/amir73il/unionmount-testsuite.git
> 3) The test suite also requires fsck.overlay, URL: https://github.com/hisilicon/overlayfs-progs
>
> Are all the URLs are correct?

Yes.

> #2 and #3 are official repositories so to say?
>

#2 is officially documented in Documentation/filesystems/overlayfs.txt
and actively maintained.

#3 I am not sure how many people use it.
I did not get much attention after the initial release.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories
  2021-05-27 17:45 ` [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories Vyacheslav Yurkov
  2021-05-28 11:38   ` Amir Goldstein
@ 2021-07-19 14:29   ` Miklos Szeredi
  1 sibling, 0 replies; 12+ messages in thread
From: Miklos Szeredi @ 2021-07-19 14:29 UTC (permalink / raw)
  To: Vyacheslav Yurkov; +Cc: Amir Goldstein, overlayfs, Vyacheslav Yurkov

On Thu, 27 May 2021 at 19:46, Vyacheslav Yurkov <uvv.mail@gmail.com> wrote:
>
> From: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
>
> Enable optimizations only 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

Series applied and pushed to vfs.git#overlayfs-next.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-07-19 14:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-27 17:45 [PATCH v3 1/3] ovl: disable decoding null uuid with redirect dir Vyacheslav Yurkov
2021-05-27 17:45 ` [PATCH v3 2/3] ovl: add ovl_allow_offline_changes() helper Vyacheslav Yurkov
2021-05-27 17:45 ` [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories Vyacheslav Yurkov
2021-05-28 11:38   ` Amir Goldstein
2021-06-01  6:53     ` Yurkov, Vyacheslav
2021-06-01  8:33       ` Amir Goldstein
2021-06-01 16:29         ` Yurkov, Vyacheslav
2021-06-01 18:29           ` Amir Goldstein
2021-06-04 12:32             ` Amir Goldstein
2021-06-04 14:50               ` Yurkov, Vyacheslav
2021-06-04 19:18                 ` Amir Goldstein
2021-07-19 14:29   ` Miklos Szeredi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).