Re: [PATCH v2 5/7] overlay: test concurrent copy up of lower hardlinks

[Eryu Guan <eguan@redhat.com>]

On Tue, Jul 11, 2017 at 11:23:50PM +0300, Amir Goldstein wrote:
> On Wed, Jul 5, 2017 at 1:46 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Wed, Jul 5, 2017 at 12:59 PM, Eryu Guan <eguan@redhat.com> wrote:
> >> On Tue, Jul 04, 2017 at 02:40:32PM +0300, Amir Goldstein wrote:
> >>> Two tasks make a modification concurrently on two hardlinks of a large
> >>> lower inode.  The copy up should be triggered by one of the tasks and the
> >>> other should be waiting for copy up to complete.  Both copy up targets
> >>> should end up being upper hardlinks and both metadata changes should be
> >>> visible in both hardlinks.
> >>>
> >>> With kernel <= v4.12, hardlinks are broken on copy up, meaning that copy up
> >>
> >> So this will be fixed in 4.13-rc1 kernel?
> >
> > It is fixed on current overlayfs-next branch. Yes.
> >
> 
> Eryu,
> 
> I realize that my answer was not accurate.
> These tests do pass with current overlayfs-next branch, but
> only with non default kernel config CONFIG_OVERLAY_FS_INDEX=y.

Thanks for the heads-up!

> 
> This may be the right way to test, meaning that default kernel
> config reports failed tests related to hardlinks which are really broken
> on copy up with default kernel config.

So these hardlink tests are still valid tests and the failures should be
fixed eventually, even for OVERLAY_FS_INDEX=n kernels? If so, I think we
can just keep the tests unchanged, just like all other tests that are
targeting un-fixed bugs. Then the only issue is the commit log is not so
accurate.

Otherwise, I prefer your opt-in way, making these tests _notrun
(assmuing they're not valid tests for this kernel config).

Thanks,
Eryu

> 
> Another way to go at it is having these tests
> "_require_fs_module_param index" (CONFIG_OVERLAY_FS_INDEX
> sets the default of this parameter) and then opt-in to indexing in the test
> using the index=on mount option.
> 
> This way, those test will notrun on old kernels and pass
> on new kernels regardless of the kernel config option.
> Then they will be testing that "hardlinks are not broken IF
> admin opts-in for indexing".
> 
> I wonder which of the test methodologies you prefer.
> If you like the second one better I can send a patch to make those
> tests depend on and opt-in for indexing.
> 
> Thanks,
> Amir.