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 > Sent: Friday, May 28, 2021 13:39 > To: Vyacheslav Yurkov > Cc: Miklos Szeredi ; overlayfs unionfs@vger.kernel.org>; Yurkov, Vyacheslav > > 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 > wrote: > > > > From: Vyacheslav Yurkov > > > > 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, > > The series looks good. > In case you post another version please add: > Reviewed-by: Amir Goldstein > > 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 > >