[ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Amir Goldstein]

Hi All,

The master branch on my unionmount-testsuite tree [1] has been updated.

The head of the master branch is commit:
6d04664 Add a test for rename over empty union dir

This small update contains:
- A few new simple dir rename and dir remove tests
- A fix for running on kernel <= v4.9 with no redirect_dir support
- A new setup option --squashfs to setup lower as read-only squashfs

With index enabled, there are no known failures.
With index disabled this test fails on non-constant st_ino for lower hardlink
copy up:
  run --ov=1 --samefs --verify rename-mass-5

Thanks,
Amir.

[1] https://github.com/amir73il/unionmount-testsuite

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Miklos Szeredi]

On Tue, Dec 5, 2017 at 9:20 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> Hi All,
>
> The master branch on my unionmount-testsuite tree [1] has been updated.
>
> The head of the master branch is commit:
> 6d04664 Add a test for rename over empty union dir
>
> This small update contains:
> - A few new simple dir rename and dir remove tests
> - A fix for running on kernel <= v4.9 with no redirect_dir support
> - A new setup option --squashfs to setup lower as read-only squashfs
>
> With index enabled, there are no known failures.
> With index disabled this test fails on non-constant st_ino for lower hardlink
> copy up:
>   run --ov=1 --samefs --verify rename-mass-5
>
> Thanks,
> Amir.
>
> [1] https://github.com/amir73il/unionmount-testsuite

Thanks for also taking care of this useful resource!

Miklos

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Vivek Goyal]

On Tue, Dec 05, 2017 at 10:20:25PM +0200, Amir Goldstein wrote:
> Hi All,
> 
> The master branch on my unionmount-testsuite tree [1] has been updated.
> 
> The head of the master branch is commit:
> 6d04664 Add a test for rename over empty union dir
> 
> This small update contains:
> - A few new simple dir rename and dir remove tests
> - A fix for running on kernel <= v4.9 with no redirect_dir support
> - A new setup option --squashfs to setup lower as read-only squashfs
> 

Hi Amir,

It would be nice to also update README file and explain different options.

Also, how to test with different mount options enabled/disable (index,
redirect etc).

> With index enabled, there are no known failures.
> With index disabled this test fails on non-constant st_ino for lower hardlink
> copy up:
>   run --ov=1 --samefs --verify rename-mass-5

I ran "./run --ov" and that seems to fail with 4.15.0-rc2 kernel.

# ./run --ov
***
*** ./run --ov --ts=0 open-plain
***
TEST open-plain.py:10: Open O_RDONLY
/mnt/a/foo100: File on unexpected layer

Vivek

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Amir Goldstein]

On Wed, Dec 6, 2017 at 8:17 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Tue, Dec 05, 2017 at 10:20:25PM +0200, Amir Goldstein wrote:
>> Hi All,
>>
>> The master branch on my unionmount-testsuite tree [1] has been updated.
>>
>> The head of the master branch is commit:
>> 6d04664 Add a test for rename over empty union dir
>>
>> This small update contains:
>> - A few new simple dir rename and dir remove tests
>> - A fix for running on kernel <= v4.9 with no redirect_dir support
>> - A new setup option --squashfs to setup lower as read-only squashfs
>>
>
> Hi Amir,
>
> It would be nice to also update README file and explain different options.

Yes, that would be nice... will get to it some day if someone doesn't
beat me to it.

>
> Also, how to test with different mount options enabled/disable (index,
> redirect etc).
>

A preview for README:
Basically, the testsuite tests for correctness, so if correctness requires
enabling a feature, then testsuite will fail without the feature, but there are
some historical exceptions:
- testsuite predates redirect_dir, so I let the testsuite auto detect
redirect_dir
  and kept the "expect EXDEV" logic for redirect_dir disabled
- until quite recently st_ino was not constant on copy up in most cases,
  so the verification of constant ino was added as an opt-in feature
with --verify


>> With index enabled, there are no known failures.
>> With index disabled this test fails on non-constant st_ino for lower hardlink
>> copy up:
>>   run --ov=1 --samefs --verify rename-mass-5

Thus, this test fails because with index disabled, hardlink is broken on copy up
and does not preserve st_ino.

>
> I ran "./run --ov" and that seems to fail with 4.15.0-rc2 kernel.
>
> # ./run --ov
> ***
> *** ./run --ov --ts=0 open-plain
> ***
> TEST open-plain.py:10: Open O_RDONLY
> /mnt/a/foo100: File on unexpected layer
>

Yes... sorry about that.
I had this patch that adapts layer checks to anonymous bdev for lower layer
for a while and was holding on to it until anonymous bdev was merged.
Finally, anonymous bdev was merged to 4.15.0-rc1, but forgot to
include the patch
in the update, so here goes:

The new head of the master branch is:

commit 15670582f2eca275f4ae0811f7b795e5b083de33
Author: Amir Goldstein <amir73il@gmail.com>
Date:   Fri Jun 23 16:08:49 2017 +0300

    Update layer check for non samefs --verify case

    With samefs setup, files are on overlay st_dev if st_ino is constant
    on copy up and on real st_dev if st_ino is not constant.
    --verify verifies constant st_ino, so it implies overlay st_dev check.
    Without --verify, we allow for both options.

    With non samefs setup, files are on pseudo or upper st_dev if st_ino
    is constant on copy up and on lower or upper st_dev otherwise.
    --verify verifies constant st_ino, so it implies pseudo st_dev check.
    Without --verify we allow for both options.

    Signed-off-by: Amir Goldstein <amir73il@gmail.com>

As far as I tested, this works well on 4.15.0-rc1, v4.14, v4.13, v4.12
without --verify and with or without --samefs.
--verify [--samefs] passes on 4.15.0-rc1 and fails on older kernels
depending on the type of constant st_ino support in those kernels/features.

Please note that --verify checks for constant st_ino, but does NOT check for
constant st_dev, but it does have some convoluted sanity checks about st_dev,
which try to comply with a wide range of kernels, which is what this
last patch is
all about.

Thanks to Chandan for his help reviewing and testing this patch.

Amir.

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Amir Goldstein]

On Wed, Dec 6, 2017 at 9:14 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Wed, Dec 6, 2017 at 8:17 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> On Tue, Dec 05, 2017 at 10:20:25PM +0200, Amir Goldstein wrote:
>>> Hi All,
>>>
>>> The master branch on my unionmount-testsuite tree [1] has been updated.
>>>
>>> The head of the master branch is commit:
>>> 6d04664 Add a test for rename over empty union dir
>>>
>>> This small update contains:
>>> - A few new simple dir rename and dir remove tests
>>> - A fix for running on kernel <= v4.9 with no redirect_dir support
>>> - A new setup option --squashfs to setup lower as read-only squashfs
>>>
>>
>> Hi Amir,
>>
>> It would be nice to also update README file and explain different options.
>
> Yes, that would be nice... will get to it some day if someone doesn't
> beat me to it.
>
>>
>> Also, how to test with different mount options enabled/disable (index,
>> redirect etc).
>>
>
> A preview for README:
> Basically, the testsuite tests for correctness, so if correctness requires
> enabling a feature, then testsuite will fail without the feature, but there are
> some historical exceptions:
> - testsuite predates redirect_dir, so I let the testsuite auto detect
> redirect_dir
>   and kept the "expect EXDEV" logic for redirect_dir disabled
> - until quite recently st_ino was not constant on copy up in most cases,
>   so the verification of constant ino was added as an opt-in feature
> with --verify
>
>
>>> With index enabled, there are no known failures.
>>> With index disabled this test fails on non-constant st_ino for lower hardlink
>>> copy up:
>>>   run --ov=1 --samefs --verify rename-mass-5
>
> Thus, this test fails because with index disabled, hardlink is broken on copy up
> and does not preserve st_ino.
>

Vivek,

I added documentation about all the features I added to unionmount-testsuite at:
https://github.com/amir73il/overlayfs/wiki/Overlayfs-testing#Unionmount_testsuite

Will add that link to README on the next master branch update.
Let me know if there is anything missing for you in this wiki page.

Thanks,
Amir.

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Vivek Goyal]

On Fri, Dec 08, 2017 at 06:29:53PM +0200, Amir Goldstein wrote:
> On Wed, Dec 6, 2017 at 9:14 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Wed, Dec 6, 2017 at 8:17 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >> On Tue, Dec 05, 2017 at 10:20:25PM +0200, Amir Goldstein wrote:
> >>> Hi All,
> >>>
> >>> The master branch on my unionmount-testsuite tree [1] has been updated.
> >>>
> >>> The head of the master branch is commit:
> >>> 6d04664 Add a test for rename over empty union dir
> >>>
> >>> This small update contains:
> >>> - A few new simple dir rename and dir remove tests
> >>> - A fix for running on kernel <= v4.9 with no redirect_dir support
> >>> - A new setup option --squashfs to setup lower as read-only squashfs
> >>>
> >>
> >> Hi Amir,
> >>
> >> It would be nice to also update README file and explain different options.
> >
> > Yes, that would be nice... will get to it some day if someone doesn't
> > beat me to it.
> >
> >>
> >> Also, how to test with different mount options enabled/disable (index,
> >> redirect etc).
> >>
> >
> > A preview for README:
> > Basically, the testsuite tests for correctness, so if correctness requires
> > enabling a feature, then testsuite will fail without the feature, but there are
> > some historical exceptions:
> > - testsuite predates redirect_dir, so I let the testsuite auto detect
> > redirect_dir
> >   and kept the "expect EXDEV" logic for redirect_dir disabled
> > - until quite recently st_ino was not constant on copy up in most cases,
> >   so the verification of constant ino was added as an opt-in feature
> > with --verify
> >
> >
> >>> With index enabled, there are no known failures.
> >>> With index disabled this test fails on non-constant st_ino for lower hardlink
> >>> copy up:
> >>>   run --ov=1 --samefs --verify rename-mass-5
> >
> > Thus, this test fails because with index disabled, hardlink is broken on copy up
> > and does not preserve st_ino.
> >
> 
> Vivek,
> 
> I added documentation about all the features I added to unionmount-testsuite at:
> https://github.com/amir73il/overlayfs/wiki/Overlayfs-testing#Unionmount_testsuite
> 
> Will add that link to README on the next master branch update.
> Let me know if there is anything missing for you in this wiki page.

Hi Amir,

Thanks a lot for this documentation. I tested it and it works for me. So
far I was only running "./run --ov". Now I can do samefs setups and also
verify index feature etc.

Two minor nits/queries.

- Right now "./run --ov --samefs -s" fails if "/base" directory is not
  there. So in mkdir command, add "base" to the list also.

- For the xfs reflink test, I am curious that why have you recommended
  to also enabled "rmapbt=1". Is is required for the tests?

Thanks
Vivek

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Amir Goldstein]

On Mon, Dec 11, 2017 at 9:59 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Fri, Dec 08, 2017 at 06:29:53PM +0200, Amir Goldstein wrote:
>> On Wed, Dec 6, 2017 at 9:14 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> > On Wed, Dec 6, 2017 at 8:17 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> >> On Tue, Dec 05, 2017 at 10:20:25PM +0200, Amir Goldstein wrote:
>> >>> Hi All,
>> >>>
>> >>> The master branch on my unionmount-testsuite tree [1] has been updated.
>> >>>
>> >>> The head of the master branch is commit:
>> >>> 6d04664 Add a test for rename over empty union dir
>> >>>
>> >>> This small update contains:
>> >>> - A few new simple dir rename and dir remove tests
>> >>> - A fix for running on kernel <= v4.9 with no redirect_dir support
>> >>> - A new setup option --squashfs to setup lower as read-only squashfs
>> >>>
>> >>
>> >> Hi Amir,
>> >>
>> >> It would be nice to also update README file and explain different options.
>> >
>> > Yes, that would be nice... will get to it some day if someone doesn't
>> > beat me to it.
>> >
>> >>
>> >> Also, how to test with different mount options enabled/disable (index,
>> >> redirect etc).
>> >>
>> >
>> > A preview for README:
>> > Basically, the testsuite tests for correctness, so if correctness requires
>> > enabling a feature, then testsuite will fail without the feature, but there are
>> > some historical exceptions:
>> > - testsuite predates redirect_dir, so I let the testsuite auto detect
>> > redirect_dir
>> >   and kept the "expect EXDEV" logic for redirect_dir disabled
>> > - until quite recently st_ino was not constant on copy up in most cases,
>> >   so the verification of constant ino was added as an opt-in feature
>> > with --verify
>> >
>> >
>> >>> With index enabled, there are no known failures.
>> >>> With index disabled this test fails on non-constant st_ino for lower hardlink
>> >>> copy up:
>> >>>   run --ov=1 --samefs --verify rename-mass-5
>> >
>> > Thus, this test fails because with index disabled, hardlink is broken on copy up
>> > and does not preserve st_ino.
>> >
>>
>> Vivek,
>>
>> I added documentation about all the features I added to unionmount-testsuite at:
>> https://github.com/amir73il/overlayfs/wiki/Overlayfs-testing#Unionmount_testsuite
>>
>> Will add that link to README on the next master branch update.
>> Let me know if there is anything missing for you in this wiki page.
>
> Hi Amir,
>
> Thanks a lot for this documentation. I tested it and it works for me. So
> far I was only running "./run --ov". Now I can do samefs setups and also
> verify index feature etc.
>
> Two minor nits/queries.
>
> - Right now "./run --ov --samefs -s" fails if "/base" directory is not
>   there. So in mkdir command, add "base" to the list also.

Fix. Thanks for review!

>
> - For the xfs reflink test, I am curious that why have you recommended
>   to also enabled "rmapbt=1". Is is required for the tests?
>

You cannot enable reflink=1 without it.

Amir.

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Vivek Goyal]

On Mon, Dec 11, 2017 at 10:47:15PM +0200, Amir Goldstein wrote:

[..]
> > - For the xfs reflink test, I am curious that why have you recommended
> >   to also enabled "rmapbt=1". Is is required for the tests?
> >
> 
> You cannot enable reflink=1 without it.

I could do that. Here is the xfs_info output which shows rmapbt=0 and
reflink=1.

# xfs_info /mnt/test-xfs
meta-data=/dev/mapper/test--union-lower_layer isize=512    agcount=4, agsize=327680 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1 spinodes=0 rmapbt=0
         =                       reflink=1
data     =                       bsize=4096   blocks=1310720, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0 ftype=1
log      =internal               bsize=4096   blocks=2560, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0

Vivek

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Dave Chinner]

On Mon, Dec 11, 2017 at 10:47:15PM +0200, Amir Goldstein wrote:
> On Mon, Dec 11, 2017 at 9:59 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > - For the xfs reflink test, I am curious that why have you recommended
> >   to also enabled "rmapbt=1". Is is required for the tests?
> >
> 
> You cannot enable reflink=1 without it.

Yes you can. They are two different functions, used for completely
different things. They both landed at the same time, will end up
being fully supported at the same time, and probably get turned on
by default at the same time. But they are independent options...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Amir Goldstein]

On Tue, Dec 12, 2017 at 2:10 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Mon, Dec 11, 2017 at 10:47:15PM +0200, Amir Goldstein wrote:
>> On Mon, Dec 11, 2017 at 9:59 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> > - For the xfs reflink test, I am curious that why have you recommended
>> >   to also enabled "rmapbt=1". Is is required for the tests?
>> >
>>
>> You cannot enable reflink=1 without it.
>
> Yes you can. They are two different functions, used for completely
> different things. They both landed at the same time, will end up
> being fully supported at the same time, and probably get turned on
> by default at the same time. But they are independent options...
>


Thanks for the correction.
I think I'll leave the instructions as is if the features are intended to be
made default at the same time, that is probably the more common setup.

Amir.

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Christoph Hellwig]

I have a customer using only reflink.  And they probably want to keep
it that way as rmap adds quite a lot of overhead.

Re: [ANNOUNCE] unionmount-testsuite: Amir's master branch updated

[Amir Goldstein]

On Tue, Dec 12, 2017 at 4:53 PM, Christoph Hellwig <hch@infradead.org> wrote:
> I have a customer using only reflink.  And they probably want to keep
> it that way as rmap adds quite a lot of overhead.

OK. convinced. Updated wiki to set only reflink feature enabled:
https://github.com/amir73il/overlayfs/wiki/Overlayfs-testing#Download_and_build_xfsprogs_with_XFS_reflink_support

Thanks,
Amir.