RE: [PATCH v3 3/3] ovl: do not set overlay.opaque for new directories

["Yurkov, Vyacheslav" <Vyacheslav.Yurkov@bruker.com>]

[-- 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