* [PATCH] generic: test lseek with seek data mode on a one byte file
@ 2023-01-09 10:34 fdmanana
2023-01-09 11:20 ` David Disseldorp
0 siblings, 1 reply; 3+ messages in thread
From: fdmanana @ 2023-01-09 10:34 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, Filipe Manana
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
+
+_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
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] generic: test lseek with seek data mode on a one byte file
2023-01-09 10:34 [PATCH] generic: test lseek with seek data mode on a one byte file fdmanana
@ 2023-01-09 11:20 ` David Disseldorp
2023-01-09 11:25 ` Filipe Manana
0 siblings, 1 reply; 3+ messages in thread
From: David Disseldorp @ 2023-01-09 11:20 UTC (permalink / raw)
To: fdmanana; +Cc: fstests, linux-btrfs, Filipe Manana
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] generic: test lseek with seek data mode on a one byte file
2023-01-09 11:20 ` David Disseldorp
@ 2023-01-09 11:25 ` Filipe Manana
0 siblings, 0 replies; 3+ messages in thread
From: Filipe Manana @ 2023-01-09 11:25 UTC (permalink / raw)
To: David Disseldorp; +Cc: fstests, linux-btrfs, Filipe Manana
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-09 11:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-09 10:34 [PATCH] generic: test lseek with seek data mode on a one byte file fdmanana
2023-01-09 11:20 ` David Disseldorp
2023-01-09 11:25 ` Filipe Manana
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.