generic/553: fix test description

PATCH In review Amir Goldstein 5 participants 14 messages Jun 11, 2019 – Jun 26, 2019

lore

Share & tools

lore

Amir Goldstein

author

2019-06-11 15:39 UTC #0 lore raw

PATCH v2 1/2 generic/553: fix test description

The test only checks copy to immutable file.
Note the kernel fix commit.

Signed-off-by Amir Goldstein <[email protected]>

---

Changes from v1:
- Document kernel fix commit

 tests/generic/553 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

tests/generic/553 +4 -1

index 98ef77cc..58e0d5ef 100755
@@ -4,7 +4,10 @@
 #
 # FS QA Test No. 553
 #
-# Check that we cannot copy_file_range() to/from an immutable file
+# Check that we cannot copy_file_range() to an immutable file
+#
+# This is a regression test for kernel commit:
+#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
 #
 seq=`basename $0`
 seqres=$RESULT_DIR/$seq

Signature · 1 line

2.17.1

Amir Goldstein

author

2019-06-11 15:39 UTC #1 lore raw

PATCH v2 2/2 generic/554: test only copy to active swap file

Depending on filesystem, copying from active swapfile may be allowed, just as read from swapfile may be allowed.

Note the kernel fix commit in test description.

Signed-off-by Amir Goldstein <[email protected]>

---

Eryu,

Per your and Ted's request, I've documented the kernel fix commit in the new copy_range tests. Those commits are now on Darrick's copy-file-range-fixes branch, which is on its way to linux-next and to kernel 5.3.

Thanks,
Amir.

Changes from v1:
- Document kernel fix commit

 tests/generic/554     | 6 ++++--
 tests/generic/554.out | 1 -
 2 files changed, 4 insertions(+), 3 deletions(-)

tests/generic/554 +4 -2

index 10ae4035..fa19d580 100755
@@ -4,7 +4,10 @@
 #
 # FS QA Test No. 554
 #
-# Check that we cannot copy_file_range() to/from a swapfile
+# Check that we cannot copy_file_range() to a swapfile
+#
+# This is a regression test for kernel commit:
+#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
 #
 seq=`basename $0`
 seqres=$RESULT_DIR/$seq

@@ -46,7 +49,6 @@ echo swap files return ETXTBUSY
 _format_swapfile $SCRATCH_MNT/swapfile 16m
 swapon $SCRATCH_MNT/swapfile
 $XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/file" $SCRATCH_MNT/swapfile
-$XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/swapfile" $SCRATCH_MNT/copy
 swapoff $SCRATCH_MNT/swapfile
 
 # success, all done

tests/generic/554.out +0 -1

index ffaa7b0a..19385a05 100644
@@ -1,4 +1,3 @@
 QA output created by 554
 swap files return ETXTBUSY
 copy_range: Text file busy
-copy_range: Text file busy

Signature · 1 line

2.17.1

Amir Goldstein

author Reviewed-by Darrick J. Wong on v2 2/2

2019-06-11 15:41 UTC #2 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

Quoted 7 lines · Amir Goldstein

Depending on filesystem, copying from active swapfile may be allowed, just as read from swapfile may be allowed.
Note the kernel fix commit in test description.
Signed-off-by Amir Goldstein <[email protected]>

Forgot to add:

Reviewed-by Darrick J. Wong <[email protected]>

Quoted 56 lines

---

Eryu,

Thanks,
Amir.

Changes from v1:
- Document kernel fix commit

 tests/generic/554     | 6 ++++--
 tests/generic/554.out | 1 -
 2 files changed, 4 insertions(+), 3 deletions(-)

tests/generic/554 +4 -2

index 10ae4035..fa19d580 100755
@@ -4,7 +4,10 @@
 #
 # FS QA Test No. 554
 #
-# Check that we cannot copy_file_range() to/from a swapfile
+# Check that we cannot copy_file_range() to a swapfile
+#
+# This is a regression test for kernel commit:
+#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
 #
 seq=`basename $0`
 seqres=$RESULT_DIR/$seq

@@ -46,7 +49,6 @@ echo swap files return ETXTBUSY
 _format_swapfile $SCRATCH_MNT/swapfile 16m
 swapon $SCRATCH_MNT/swapfile
 $XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/file" $SCRATCH_MNT/swapfile
-$XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/swapfile" $SCRATCH_MNT/copy
 swapoff $SCRATCH_MNT/swapfile
 
 # success, all done

tests/generic/554.out +0 -2

index ffaa7b0a..19385a05 100644
@@ -1,4 +1,3 @@
 QA output created by 554
 swap files return ETXTBUSY
 copy_range: Text file busy
-copy_range: Text file busy
--

2.17.1

Theodore Ts'o

2019-06-11 15:59 UTC #3 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

In reply to Amir Goldstein

Quoted 11 lines · Amir Goldstein

Depending on filesystem, copying from active swapfile may be allowed, just as read from swapfile may be allowed.
Note the kernel fix commit in test description.
Signed-off-by Amir Goldstein <[email protected]>
Per your and Ted's request, I've documented the kernel fix commit in the new copy_range tests. Those commits are now on Darrick's copy-file-range-fixes branch, which is on its way to linux-next and to kernel 5.3.

Thanks! Are we sure at this point that the commit won't need to be modified / rebased in Darrick's tree?

tests/generic/554 line 10

Quoted 2 lines

tests/generic/554 +2 -0

+# This is a regression test for kernel commit:
+#   a31713517dac ("vfs: introduce generic_file_rw_checks()")

- Ted

Amir Goldstein

author

2019-06-11 16:07 UTC #4 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

Quoted 9 lines · Theodore Ts'o

Quoted 7 lines · Amir Goldstein
Depending on filesystem, copying from active swapfile may be allowed, just as read from swapfile may be allowed.
Note the kernel fix commit in test description.
Signed-off-by Amir Goldstein <[email protected]>

Quoted 8 lines

Quoted 4 lines
Per your and Ted's request, I've documented the kernel fix commit in the new copy_range tests. Those commits are now on Darrick's copy-file-range-fixes branch, which is on its way to linux-next and to kernel 5.3.
Thanks! Are we sure at this point that the commit won't need to be modified / rebased in Darrick's tree?

Yes. The intention is that fs maintainers can now rebase their v5.3 development branch on this non-rewindable branch.

Note that you do still need to merge the fixes from Darrick's branch for ext4 to pass this test despite relaxing the "copy from swapfile" test case.

Thanks,
Amir.

Murphy Zhou

2019-06-18 09:02 UTC #5 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

In reply to Amir Goldstein

Quoted 2 lines · Amir Goldstein

Depending on filesystem, copying from active swapfile may be allowed, just as read from swapfile may be allowed.

...snip..

tests/generic/554 line 10

Quoted 2 lines

tests/generic/554 +2 -0

+# This is a regression test for kernel commit:
+#   a31713517dac ("vfs: introduce generic_file_rw_checks()")

Would you mind updating sha1 after it get merged to Linus tree?

That would be helpful for people tracking this issue.

Thanks,
Murphy

Quoted 23 lines

#
seq=basename $0
seqres=$RESULT_DIR/$seq

tests/generic/554 +0 -1

@@ -46,7 +49,6 @@ echo swap files return ETXTBUSY
 _format_swapfile $SCRATCH_MNT/swapfile 16m
 swapon $SCRATCH_MNT/swapfile
 $XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/file" $SCRATCH_MNT/swapfile
-$XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/swapfile" $SCRATCH_MNT/copy
 swapoff $SCRATCH_MNT/swapfile
 
 # success, all done

tests/generic/554.out +0 -1

index ffaa7b0a..19385a05 100644
@@ -1,4 +1,3 @@
 QA output created by 554
 swap files return ETXTBUSY
 copy_range: Text file busy
-copy_range: Text file busy

Signature · 1 line

2.17.1

Amir Goldstein

author

2019-06-18 09:16 UTC #6 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

Quoted 5 lines · Murphy Zhou

Quoted 2 lines · Amir Goldstein
Depending on filesystem, copying from active swapfile may be allowed, just as read from swapfile may be allowed.

Quoted 9 lines

...snip..
tests/generic/554 line 10
Quoted 2 lines
tests/generic/554 +2 -0
+# This is a regression test for kernel commit:
+#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
Would you mind updating sha1 after it get merged to Linus tree?
That would be helpful for people tracking this issue.

This is the commit id in linux-next and expected to stay the same when the fix is merged to Linus tree for 5.3.

Thanks,
Amir.

Murphy Zhou

2019-06-18 09:36 UTC #7 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

Quoted 13 lines · Amir Goldstein

Quoted 12 lines · Murphy Zhou
Quoted 2 lines · Amir Goldstein
Depending on filesystem, copying from active swapfile may be allowed, just as read from swapfile may be allowed.
...snip..
tests/generic/554 line 10
Quoted 2 lines
tests/generic/554 +2 -0
+# This is a regression test for kernel commit:
+#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
Would you mind updating sha1 after it get merged to Linus tree?

Quoted 5 lines

Quoted 2 lines
That would be helpful for people tracking this issue.
This is the commit id in linux-next and expected to stay the same when the fix is merged to Linus tree for 5.3.

That's perfect. :)

Quoted 3 lines

Thanks,
Amir.

Theodore Ts'o

2019-06-18 15:02 UTC #8 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

In reply to Amir Goldstein

Quoted 9 lines · Amir Goldstein

Quoted 5 lines · Murphy Zhou
Would you mind updating sha1 after it get merged to Linus tree?
That would be helpful for people tracking this issue.
This is the commit id in linux-next and expected to stay the same when the fix is merged to Linus tree for 5.3.

When I talked to Darrick last week, that was *not* the sense I got from him. It's not necessarily guaranteed to be stable just yet...

- Ted

Darrick J. Wong

2019-06-18 15:11 UTC #9 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

Quoted 5 lines · Theodore Ts'o

Quoted 4 lines · Amir Goldstein
Quoted 3 lines · Murphy Zhou
Would you mind updating sha1 after it get merged to Linus tree?

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
That would be helpful for people tracking this issue.
This is the commit id in linux-next and expected to stay the same when the fix is merged to Linus tree for 5.3.
When I talked to Darrick last week, that was *not* the sense I got from him. It's not necessarily guaranteed to be stable just yet...

Darrick hasn't gotten any complaints about the copy-file-range-fixes branch (which has been in for-next for a week now), so he thinks that commit id (a31713517dac) should be stable from here on out.

(Note that doesn't guarantee that Linus will pull said branch...)

--D

Quoted 1 line

- Ted

Yang Xu

2019-06-25 10:29 UTC #10 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

Quoted 11 lines · Darrick J. Wong

Quoted 10 lines · Theodore Ts'o
Quoted 7 lines · Amir Goldstein
Quoted 4 lines · Murphy Zhou
Would you mind updating sha1 after it get merged to Linus tree?
That would be helpful for people tracking this issue.
This is the commit id in linux-next and expected to stay the same when the fix is merged to Linus tree for 5.3.
When I talked to Darrick last week, that was *not* the sense I got from him. It's not necessarily guaranteed to be stable just yet...

Quoted 7 lines

Darrick hasn't gotten any complaints about the copy-file-range-fixes branch (which has been in for-next for a week now), so he thinks that commit id (a31713517dac) should be stable from here on out.
(Note that doesn't guarantee that Linus will pull said branch...)
--D

Hi Amir

The kernel fix commit message is right? :-) Because when I backport this patch into 5.2.0-rc6+ kernel, generic/554(553) also fails, it should be commit a5544984af4 (vfs: add missing checks to copy_file_range). By the way, a31713517dac ("vfs: introduce generic_file_rw_checks()") doesn't check for immutable and swap file.

I think we can change this message after the fix is merged to Linus tree for 5.3. What do you think about it?

Thanks
Yang Xu

Quoted 3 lines

Quoted 1 line
- Ted

Amir Goldstein

author

2019-06-25 10:35 UTC #11 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

Quoted 20 lines · Yang Xu

Quoted 17 lines · Darrick J. Wong
Quoted 10 lines · Theodore Ts'o
Quoted 7 lines · Amir Goldstein
Quoted 4 lines · Murphy Zhou
Would you mind updating sha1 after it get merged to Linus tree?
That would be helpful for people tracking this issue.
This is the commit id in linux-next and expected to stay the same when the fix is merged to Linus tree for 5.3.
When I talked to Darrick last week, that was *not* the sense I got from him. It's not necessarily guaranteed to be stable just yet...
Darrick hasn't gotten any complaints about the copy-file-range-fixes branch (which has been in for-next for a week now), so he thinks that commit id (a31713517dac) should be stable from here on out.
(Note that doesn't guarantee that Linus will pull said branch...)

Quoted 8 lines

Quoted 1 line
--D
Hi Amir
The kernel fix commit message is right? :-) Because when I backport this patch into 5.2.0-rc6+ kernel, generic/554(553) also fails, it should be commit a5544984af4 (vfs: add missing checks to copy_file_range). By the way, a31713517dac ("vfs: introduce generic_file_rw_checks()") doesn't check for immutable and swap file.
I think we can change this message after the fix is merged to Linus tree for 5.3. What do you think about it?

You are right. Documented commit is wrong.
The correct commit in linux-next is:
96e6e8f4a68d ("vfs: add missing checks to copy_file_range")

(Not sure where you got a5544984af4 from?)
Let's fix that after the commit is upstream.

Obviously, you would need to backport the entire series and not just this one commit to stable kernel.

Thanks,
Amir.

Yang Xu

2019-06-26 01:18 UTC #12 lore raw

Re: [PATCH v2 2/2] generic/554: test only copy to active swap file

Quoted 27 lines · Amir Goldstein

Quoted 26 lines · Yang Xu
Quoted 11 lines · Darrick J. Wong
Quoted 10 lines · Theodore Ts'o
Quoted 7 lines · Amir Goldstein
Quoted 4 lines · Murphy Zhou
Would you mind updating sha1 after it get merged to Linus tree?
That would be helpful for people tracking this issue.
This is the commit id in linux-next and expected to stay the same when the fix is merged to Linus tree for 5.3.
When I talked to Darrick last week, that was *not* the sense I got from him. It's not necessarily guaranteed to be stable just yet...
Quoted 7 lines
Darrick hasn't gotten any complaints about the copy-file-range-fixes branch (which has been in for-next for a week now), so he thinks that commit id (a31713517dac) should be stable from here on out.
(Note that doesn't guarantee that Linus will pull said branch...)
--D
Hi Amir
The kernel fix commit message is right? :-) Because when I backport this patch into 5.2.0-rc6+ kernel, generic/554(553) also fails, it should be commit a5544984af4 (vfs: add missing checks to copy_file_range). By the way, a31713517dac ("vfs: introduce generic_file_rw_checks()") doesn't check for immutable and swap file.

Quoted 6 lines

Quoted 1 line
I think we can change this message after the fix is merged to Linus tree for 5.3. What do you think about it?
You are right. Documented commit is wrong.
The correct commit in linux-next is:
96e6e8f4a68d ("vfs: add missing checks to copy_file_range")
(Not sure where you got a5544984af4 from?)

I get a5544984af4 from Darrick.wong copy-file-range-fixes branch.

Quoted 4 lines

Let's fix that after the commit is upstream.
Obviously, you would need to backport the entire series and not just this one commit to stable kernel.

Yes. I got it.

Quoted 5 lines

Thanks,
Amir.

Amir Goldstein

author Reviewed-by Darrick J. Wong on v2 1/2

2019-06-11 15:40 UTC #13 lore raw

In reply to Amir Goldstein

Quoted 5 lines · Amir Goldstein

The test only checks copy to immutable file.
Note the kernel fix commit.
Signed-off-by Amir Goldstein <[email protected]>

Forgot to add:

Reviewed-by Darrick J. Wong <[email protected]>

Quoted 27 lines

---

Changes from v1:
- Document kernel fix commit

 tests/generic/553 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

tests/generic/553 +4 -2

index 98ef77cc..58e0d5ef 100755
@@ -4,7 +4,10 @@
 #
 # FS QA Test No. 553
 #
-# Check that we cannot copy_file_range() to/from an immutable file
+# Check that we cannot copy_file_range() to an immutable file
+#
+# This is a regression test for kernel commit:
+#   a31713517dac ("vfs: introduce generic_file_rw_checks()")
 #
 seq=`basename $0`
 seqres=$RESULT_DIR/$seq
--

2.17.1