On Wed, Oct 18, 2017 at 3:55 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > On Wed, Oct 18, 2017 at 07:09:37AM +0300, Amir Goldstein wrote: >> On Wed, Oct 18, 2017 at 12:05 AM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: >> > Right now my understanding is that origin xattr is created for all copied >> > up files if index=on. And if index=off, then we create it for all type >> > of files except hardlinks (nlink != 1). >> > >> > With metadata only copy up, I will still require origin xattr to copy up >> > data later, so create it even for hardlinks even with index=off. I am >> > hoping it does not break anything because we do OVL_INDEX test before >> > using inode number of origin. >> >> I still have a nudging buzz that loosing the information about this upper >> being a broken hardlink may come back to bite us some time, but can't >> put my finger on how. > > Hi Amir, > > Can't we do stat() on ORIGIN and check nlink to figure out if it is a broken > hard link or not. (If it ever becomes a problem in future). IOW, we still > have a way to figure out if this is broken hard link or not. Just that > it involves extra step of doing stat() and its not the presence/absense > of ORIGIN. > >> >> To reduce the level of that buzz, I suggest to set origin for broken hardlink >> only when doing metacopy and then break the origin association when >> removing metacopy xattr. > > While removing metacopy xattr, I will not have any idea whether it is > broken hard link or not. I guess I will have to use above trick and do > stat() on ORIGIN and detect broken hard links. > > I feel really odd about special casing this broken hard link case and > setting and clearing ORIGIN with metacopy flags. It makes code > twisted and complicated. > > Given stat() should allow us to figure out if something is broken hard > link or not, does it make you feel any better about it. I understand your position about this. Since I cannot think of a concrete use case where keeping ORIGIN would hurt us, I'll withdraw my comment. > >> >> In any case, this patch needs to update the comment above setting >> origin xattr that claims to break the association with broken hardlink. > > Ok, Will do. > > Vivek >> >> > >> > Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx> >> > --- >> > fs/overlayfs/copy_up.c | 4 +--- >> > 1 file changed, 1 insertion(+), 3 deletions(-) >> > >> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c >> > index c441f9387a1b..0a6294ea64d5 100644 >> > --- a/fs/overlayfs/copy_up.c >> > +++ b/fs/overlayfs/copy_up.c >> > @@ -538,9 +538,6 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c) >> > c->stat.nlink > 1) >> > indexed = true; >> > >> > - if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || indexed) >> > - c->origin = true; >> > - >> > if (indexed) { >> > c->destdir = ovl_indexdir(c->dentry->d_sb); >> > err = ovl_get_index_name(c->lowerpath.dentry, &c->destname); >> > @@ -596,6 +593,7 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry, >> > .parent = parent, >> > .dentry = dentry, >> > .workdir = ovl_workdir(dentry), >> > + .origin = true, >> > }; >> > >> > if (WARN_ON(!ctx.workdir)) >> > -- >> > 2.13.5 >> > -- 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