On Fri, Nov 18, 2016 at 5:37 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > On Thu, Nov 17, 2016 at 12:00 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: >> >> On Mon, Nov 14, 2016 at 5:25 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >> > On Sun, Nov 13, 2016 at 12:00 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >> >> >> Looks goods, except for the case of change from relative to absolute >> >> redirect of the victim dentry. IIUC, ovl_set_redirect() will return immediately >> >> because ovl_dentry_is_redirect() and will not get to setting the absolute >> >> redirect. >> >> >> > >> > I added some more tests to catch this problem at: >> > https://github.com/amir73il/unionmount-testsuite.git #ovl_rename_dir >> >> Thanks for testing. >> >> Force pushed updated version to the usual place: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git #redirect >> > > Found one typo and one bug in error that can cause crash on dput(ERR_PTR(err)): > > diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig > index 21ddac7..0daac51 100644 > --- a/fs/overlayfs/Kconfig > +++ b/fs/overlayfs/Kconfig > @@ -15,7 +15,7 @@ config OVERLAY_FS_REDIRECT_DIR > help > If this config option is enabled then overlay filesystems will use > redirects when renaming directories by default. In this case it is > - still possible possible to turn off redirects globally with the > + still possible to turn off redirects globally with the > "redirect_dir=off" module option or on a filesystem instance basis > with the "redirect_dir=off" mount option. > > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c > index 5eaa9f9..a19fc5c 100644 > --- a/fs/overlayfs/namei.c > +++ b/fs/overlayfs/namei.c > @@ -105,11 +105,12 @@ static int ovl_lookup_single(struct dentry > *base, struct ovl_lookup_data *d, > > this = lookup_one_len_unlocked(name, base, namelen); > if (IS_ERR(this)) { > - if (PTR_ERR(this) == -ENOENT || > - PTR_ERR(this) == -ENAMETOOLONG) { > + err = PTR_ERR(this); > + if (err == -ENOENT || err == -ENAMETOOLONG) { > this = NULL; > + goto out; > } > - goto out; > + return err; > } > if (!this->d_inode) > goto put_and_out; > I just realized that this bug is already in overlayfs-next, so posted a patch to fix it. >> This also has the xattr feature thing replaced with mount option, >> module param and kernel config option. >> > > I like the kernel config/module param/mount option for > enabling/disabling the feature. > > But I still think that we should write the features xattr on the first > redirect rename. > The features xattr tell us what can be found on the layer, so we would > be wise to > keep it around for all sorts of backward compatibility aspect. > > Amir. -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
