On Wed, Mar 7, 2018 at 5:15 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> On Wed, Mar 07, 2018 at 09:48:22AM +0200, Amir Goldstein wrote:
>> On Tue, Mar 6, 2018 at 10:54 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>> > Set redirect on metacopy files upon rename. This will help find data dentry
>> > in lower dirs.
>> >
>> > Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>
>> > ---
>> > fs/overlayfs/dir.c | 13 +++++++++----
>> > 1 file changed, 9 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
>> > index cdeae4bdbfce..b955f6fede06 100644
>> > --- a/fs/overlayfs/dir.c
>> > +++ b/fs/overlayfs/dir.c
>> > @@ -1048,9 +1048,11 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
>> > err = ovl_set_redirect(old, samedir);
>> > else if (!old_opaque && ovl_type_merge(new->d_parent))
>> > err = ovl_set_opaque_xerr(old, olddentry, -EXDEV);
>> > - if (err)
>> > - goto out_dput;
>> > - }
>> > + } else if (ovl_is_metacopy_dentry(old))
>> > + err = ovl_set_redirect(old, false);
>>
>> You should use {} in the else statement as well.
>
> ok.
>
>>
>> Q: why do you set samedir: = false here?
>> A: because other hardlink aliasses cannot follow a relative redirect, right?
>
> Right. If we create a hardlink later then it will need absolute redirect
> if both dentries are not in same dir.
>
>>
>> This is a subtle detail that should be documented,
>
> Ok, will do.
>
>> but also
>> maybe do use samedir if nlink == 1?
>
> Hmm.., so initially we could put a relative redirct (if nlink=1) and later
> if we create a link, we could replace relative redirect with an absolute
> redirect? I see we already have logic to do that for the case of rename.
>
> Now only thing I need to figure out in ovl_link() whethre two dentries
> are in same dir or not. I am assuming I can just check parent dentry
> pointers and see if these two have same parent or not.
Yes or we can just convert to absolute path anyway for nlink > 1.
>
> In fact, we probably don't even have to check for nlink=1. Only when
> we create a upper hard link, then we need to make sure we replace relative
> hardlink with absolute one. I will play with it and see how it goes.
>
Yes. alomst true. but we do need to check for lower nlink > 1,
because in that case (when index=on) upper hardlinks are created on
copy up not only on ovl_link(), so easiest is to just start with
absolute redirect
on rename of lower hardlink.
Thanks,
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
