lkml.org 
[lkml]   [2016]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 3/3] ovl: redirect on rename-dir
On Fri, Nov 18, 2016 at 5:37 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Thu, Nov 17, 2016 at 12:00 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>
>> On Mon, Nov 14, 2016 at 5:25 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> > On Sun, Nov 13, 2016 at 12:00 PM, Amir Goldstein <amir73il@gmail.com> 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.

\
 
 \ /
  Last update: 2016-11-20 12:59    [W:0.078 / U:19.732 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site