Re: [PATCH v2 00/11] overlayfs constant inode numbers

[Vivek Goyal <vgoyal@redhat.com>]

On Tue, Apr 25, 2017 at 04:23:28PM +0300, Amir Goldstein wrote:
> On Tue, Apr 25, 2017 at 3:52 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Tue, Apr 25, 2017 at 03:41:56PM +0300, Amir Goldstein wrote:
> >> On Tue, Apr 25, 2017 at 3:16 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >> > On Mon, Apr 24, 2017 at 12:14:05PM +0300, Amir Goldstein wrote:
> >> >> Miklos,
> >> >>
> >> >> Following your comments on the 'stable inodes' series from last week,
> >> >> this series fixes constant inode numbers for stat(2) with any layer
> >> >> configuration.
> >> >>
> >> >> For the case of all *lower* layers on same fs that supports NFS export,
> >> >> redirect by file handle will be used to optimize the lookup of the copy
> >> >> up origin of non-dir inode.
> >> >
> >> > I was trying to run unionmount-testsuite (original from dhowells) and I
> >> > disabled layer check. Looks like empty directory rename test fails.
> >> >
> >> > ***
> >> > *** ./run --ov --ts=0 rename-empty-dir
> >> > ***
> >> > TEST rename-empty-dir.py:10: Rename empty dir and rename back
> >> >  ./run --rename /mnt/a/empty100 /mnt/a/no_dir100
> >> >  /mnt/a/empty100: Unexpected error: Invalid cross-device link
> >> >
> >>
> >> Strange... I can't find code in recent times when this used to work
> >> It certainly doesn't look like it should work with kernel v4.10
> >> and redirect_dir=off.
> >> I couldn't the point of regression by looking at the change log.
> >> You'd need to bisect to find the regression patch.
> >>
> >> Are you not compiling kernel with redirect_dir?
> >> CONFIG_OVERLAY_FS_REDIRECT_DIR=y
> >
> > I noticed that I am running with REDIRECT_DIR=n.
> >
> > I also re-ran the tests without your patches and test is still broken. So
> > it is not due to your current patch series.
> >
> > It has been long time since I ran these tests. I suspect that we might
> > have changed this behavior during redirect directory patches.
> >
> > So question is, is this a regression or expected behavior. That is with
> > REDIRECT_DIR=n, renames of empty directory will be denied too.
> >
> 
> It must be a regression, although I can't think why anyone would care.
> If one really cares about renaming lower empty directories, why not enable
> REDIRECT_DIR?

I will enable it now. I just had an old config and ran into this.

But this does raise the question unionmount-testsuite need to be
maintained somewhere so that it acts as a baseline to figure out if
new patches broke some existing tests.

I can go by the tree you are maintaining but currently that's broken too
with REDIRECT_DIR=n.

Vivek

> 
> >>
> >> I guess not. If you do compile or mount with -o redirect_dir=on,
> >> you will need some minimal patches to unionmount-testsuite
> >> that set the expectations correctly for directory rename.
> >>
> >> The last stable branch I have from testing v4.10 is this:
> >> https://github.com/amir73il/unionmount-testsuite/commits/ovl_rename_dir
> >>
> >> But you may as well take my most recent branch for testing const ino:
> >> https://github.com/amir73il/unionmount-testsuite/commits/overlayfs-devel
> >
> > I guess I should start using your copy of unionmount-testsuite.
> >
> > Vivek