| Message ID | 2ba1050ace4aa65eff3082580b449fe1e97a3c5b.1673260375.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | generic: test lseek with seek data mode on a one byte file | expand |
Looks fine: Reviewed-by: David Disseldorp <ddiss@suse.de> Minor nit below... On Mon, 9 Jan 2023 10:34:15 +0000, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > Test that seeking for data on a 1 byte file works correctly, the returned > offset should be 0 if the start offset is 0. > > This is a regression test motivated by a btrfs bug introduced in kernel > 6.1, which got recently fixed by the following kernel commit: > > 2f2e84ca6066 ("btrfs: fix off-by-one in delalloc search during lseek") > > Signed-off-by: Filipe Manana <fdmanana@suse.com> > --- > src/seek_sanity_test.c | 36 ++++++++++++++++++++++++++++++++++++ > tests/generic/706 | 36 ++++++++++++++++++++++++++++++++++++ > tests/generic/706.out | 2 ++ > 3 files changed, 74 insertions(+) > create mode 100755 tests/generic/706 > create mode 100644 tests/generic/706.out > > diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c > index 8a586f74..48b3ccc0 100644 > --- a/src/seek_sanity_test.c > +++ b/src/seek_sanity_test.c > @@ -292,6 +292,41 @@ out: > return ret; > } > > +/* > + * Test seeking for data on a 1 byte file, both when there's delalloc and also > + * after delalloc is flushed. > + */ > +static int test22(int fd, int testnum) > +{ > + const char buf = 'X'; > + int ret; > + > + ret = do_pwrite(fd, &buf, 1, 0); > + if (ret) > + return ret; > + > + /* > + * Our file as a size of 1 byte and that byte is in delalloc. Seeking > + * for data, with a start offset of 0, should return file offset 0. > + */ > + ret = do_lseek(testnum, 1, fd, 1, SEEK_DATA, 0, 0); > + if (ret) > + return ret; > + > + /* Flush all delalloc. */ > + ret = fsync(fd); > + if (ret) { > + fprintf(stderr, "fsync failed: %s (%d)\n", strerror(errno), errno); > + return ret; > + } > + > + /* > + * We should get the same result we got when we had delalloc, 0 is the > + * offset with data. > + */ > + return do_lseek(testnum, 2, fd, 1, SEEK_DATA, 0, 0); > +} > + > /* > * Make sure hole size is properly reported when punched in the middle of a file > */ > @@ -1131,6 +1166,7 @@ struct testrec seek_tests[] = { > { 19, test19, "Test file SEEK_DATA from middle of a large hole" }, > { 20, test20, "Test file SEEK_DATA from middle of a huge hole" }, > { 21, test21, "Test file SEEK_HOLE that was created by PUNCH_HOLE" }, > + { 22, test22, "Test a 1 byte file" }, > }; > > static int run_test(struct testrec *tr) > diff --git a/tests/generic/706 b/tests/generic/706 > new file mode 100755 > index 00000000..b3e7aa7c > --- /dev/null > +++ b/tests/generic/706 > @@ -0,0 +1,36 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (C) 2023 SUSE Linux Products GmbH. All Rights Reserved. > +# > +# FS QA Test 706 > +# > +# Test that seeking for data on a 1 byte file works correctly, the returned > +# offset should be 0 if the start offset is 0. > +# > +. ./common/preamble > +_begin_fstest auto quick seek > + > +[ $FSTYP == "btrfs" ] && > + _fixed_by_kernel_commit 2f2e84ca6066 \ > + "btrfs: fix off-by-one in delalloc search during lseek" > + > +_supported_fs generic > +_require_test > +_require_seek_data_hole > +_require_test_program "seek_sanity_test" > + > +test_file=$TEST_DIR/seek_sanity_testfile.$seq Nit: test_file set and use below should be quoted. > + > +_cleanup() > +{ > + cd / > + rm -f $tmp.* > + rm -f $test_file > +} > + > +_run_seek_sanity_test -s 22 -e 22 $test_file > $seqres.full 2>&1 || > + _fail "seek sanity check failed!" Nit: I think it'd be nicer to avoid the redirect and _fail here, instead filtering the FS magic / alloc size strings. I'm okay with it as-is though. Cheers, David > + > +echo "Silence is golden" > +status=0 > +exit > diff --git a/tests/generic/706.out b/tests/generic/706.out > new file mode 100644 > index 00000000..577697c6 > --- /dev/null > +++ b/tests/generic/706.out > @@ -0,0 +1,2 @@ > +QA output created by 706 > +Silence is golden
On Mon, Jan 9, 2023 at 11:19 AM David Disseldorp <ddiss@suse.de> wrote: > > Looks fine: > Reviewed-by: David Disseldorp <ddiss@suse.de> > > Minor nit below... > > On Mon, 9 Jan 2023 10:34:15 +0000, fdmanana@kernel.org wrote: > > > From: Filipe Manana <fdmanana@suse.com> > > > > Test that seeking for data on a 1 byte file works correctly, the returned > > offset should be 0 if the start offset is 0. > > > > This is a regression test motivated by a btrfs bug introduced in kernel > > 6.1, which got recently fixed by the following kernel commit: > > > > 2f2e84ca6066 ("btrfs: fix off-by-one in delalloc search during lseek") > > > > Signed-off-by: Filipe Manana <fdmanana@suse.com> > > --- > > src/seek_sanity_test.c | 36 ++++++++++++++++++++++++++++++++++++ > > tests/generic/706 | 36 ++++++++++++++++++++++++++++++++++++ > > tests/generic/706.out | 2 ++ > > 3 files changed, 74 insertions(+) > > create mode 100755 tests/generic/706 > > create mode 100644 tests/generic/706.out > > > > diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c > > index 8a586f74..48b3ccc0 100644 > > --- a/src/seek_sanity_test.c > > +++ b/src/seek_sanity_test.c > > @@ -292,6 +292,41 @@ out: > > return ret; > > } > > > > +/* > > + * Test seeking for data on a 1 byte file, both when there's delalloc and also > > + * after delalloc is flushed. > > + */ > > +static int test22(int fd, int testnum) > > +{ > > + const char buf = 'X'; > > + int ret; > > + > > + ret = do_pwrite(fd, &buf, 1, 0); > > + if (ret) > > + return ret; > > + > > + /* > > + * Our file as a size of 1 byte and that byte is in delalloc. Seeking > > + * for data, with a start offset of 0, should return file offset 0. > > + */ > > + ret = do_lseek(testnum, 1, fd, 1, SEEK_DATA, 0, 0); > > + if (ret) > > + return ret; > > + > > + /* Flush all delalloc. */ > > + ret = fsync(fd); > > + if (ret) { > > + fprintf(stderr, "fsync failed: %s (%d)\n", strerror(errno), errno); > > + return ret; > > + } > > + > > + /* > > + * We should get the same result we got when we had delalloc, 0 is the > > + * offset with data. > > + */ > > + return do_lseek(testnum, 2, fd, 1, SEEK_DATA, 0, 0); > > +} > > + > > /* > > * Make sure hole size is properly reported when punched in the middle of a file > > */ > > @@ -1131,6 +1166,7 @@ struct testrec seek_tests[] = { > > { 19, test19, "Test file SEEK_DATA from middle of a large hole" }, > > { 20, test20, "Test file SEEK_DATA from middle of a huge hole" }, > > { 21, test21, "Test file SEEK_HOLE that was created by PUNCH_HOLE" }, > > + { 22, test22, "Test a 1 byte file" }, > > }; > > > > static int run_test(struct testrec *tr) > > diff --git a/tests/generic/706 b/tests/generic/706 > > new file mode 100755 > > index 00000000..b3e7aa7c > > --- /dev/null > > +++ b/tests/generic/706 > > @@ -0,0 +1,36 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (C) 2023 SUSE Linux Products GmbH. All Rights Reserved. > > +# > > +# FS QA Test 706 > > +# > > +# Test that seeking for data on a 1 byte file works correctly, the returned > > +# offset should be 0 if the start offset is 0. > > +# > > +. ./common/preamble > > +_begin_fstest auto quick seek > > + > > +[ $FSTYP == "btrfs" ] && > > + _fixed_by_kernel_commit 2f2e84ca6066 \ > > + "btrfs: fix off-by-one in delalloc search during lseek" > > + > > +_supported_fs generic > > +_require_test > > +_require_seek_data_hole > > +_require_test_program "seek_sanity_test" > > + > > +test_file=$TEST_DIR/seek_sanity_testfile.$seq > > Nit: test_file set and use below should be quoted. Well, in theory yes, due to paths with spaces. We do have many places in common/rc and other files that refer to TEST_DIR and SCRATCH_MNT without any quoting, so it's already borken for paths with spaces. So I left it like that, just like generic/539 and other seek tests. > > > + > > +_cleanup() > > +{ > > + cd / > > + rm -f $tmp.* > > + rm -f $test_file > > +} > > + > > +_run_seek_sanity_test -s 22 -e 22 $test_file > $seqres.full 2>&1 || > > + _fail "seek sanity check failed!" > > Nit: I think it'd be nicer to avoid the redirect and _fail here, instead > filtering the FS magic / alloc size strings. I'm okay with it as-is > though. This is the pattern all seek tests do, for example generic/539. So I left it like that for consistency. Thanks. > > Cheers, David > > > + > > +echo "Silence is golden" > > +status=0 > > +exit > > diff --git a/tests/generic/706.out b/tests/generic/706.out > > new file mode 100644 > > index 00000000..577697c6 > > --- /dev/null > > +++ b/tests/generic/706.out > > @@ -0,0 +1,2 @@ > > +QA output created by 706 > > +Silence is golden >
diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c index 8a586f74..48b3ccc0 100644 --- a/src/seek_sanity_test.c +++ b/src/seek_sanity_test.c @@ -292,6 +292,41 @@ out: return ret; } +/* + * Test seeking for data on a 1 byte file, both when there's delalloc and also + * after delalloc is flushed. + */ +static int test22(int fd, int testnum) +{ + const char buf = 'X'; + int ret; + + ret = do_pwrite(fd, &buf, 1, 0); + if (ret) + return ret; + + /* + * Our file as a size of 1 byte and that byte is in delalloc. Seeking + * for data, with a start offset of 0, should return file offset 0. + */ + ret = do_lseek(testnum, 1, fd, 1, SEEK_DATA, 0, 0); + if (ret) + return ret; + + /* Flush all delalloc. */ + ret = fsync(fd); + if (ret) { + fprintf(stderr, "fsync failed: %s (%d)\n", strerror(errno), errno); + return ret; + } + + /* + * We should get the same result we got when we had delalloc, 0 is the + * offset with data. + */ + return do_lseek(testnum, 2, fd, 1, SEEK_DATA, 0, 0); +} + /* * Make sure hole size is properly reported when punched in the middle of a file */ @@ -1131,6 +1166,7 @@ struct testrec seek_tests[] = { { 19, test19, "Test file SEEK_DATA from middle of a large hole" }, { 20, test20, "Test file SEEK_DATA from middle of a huge hole" }, { 21, test21, "Test file SEEK_HOLE that was created by PUNCH_HOLE" }, + { 22, test22, "Test a 1 byte file" }, }; static int run_test(struct testrec *tr) diff --git a/tests/generic/706 b/tests/generic/706 new file mode 100755 index 00000000..b3e7aa7c --- /dev/null +++ b/tests/generic/706 @@ -0,0 +1,36 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2023 SUSE Linux Products GmbH. All Rights Reserved. +# +# FS QA Test 706 +# +# Test that seeking for data on a 1 byte file works correctly, the returned +# offset should be 0 if the start offset is 0. +# +. ./common/preamble +_begin_fstest auto quick seek + +[ $FSTYP == "btrfs" ] && + _fixed_by_kernel_commit 2f2e84ca6066 \ + "btrfs: fix off-by-one in delalloc search during lseek" + +_supported_fs generic +_require_test +_require_seek_data_hole +_require_test_program "seek_sanity_test" + +test_file=$TEST_DIR/seek_sanity_testfile.$seq + +_cleanup() +{ + cd / + rm -f $tmp.* + rm -f $test_file +} + +_run_seek_sanity_test -s 22 -e 22 $test_file > $seqres.full 2>&1 || + _fail "seek sanity check failed!" + +echo "Silence is golden" +status=0 +exit diff --git a/tests/generic/706.out b/tests/generic/706.out new file mode 100644 index 00000000..577697c6 --- /dev/null +++ b/tests/generic/706.out @@ -0,0 +1,2 @@ +QA output created by 706 +Silence is golden