* [PATCH v2 0/3] fix btrfs-corrupt-block options value and offset
@ 2024-03-22 11:16 Anand Jain
2024-03-22 11:16 ` [PATCH v2 1/3] common/btrfs: refactor _require_btrfs_corrupt_block to check option Anand Jain
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Anand Jain @ 2024-03-22 11:16 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, fdmanana
The btrfs-corrupt-block options -v and -o have been replaced with
--value and --offset, so this patchset makes fixes accordingly.
v1:
https://lore.kernel.org/all/eb2493499d2f30f43afa09e980589bb4f15e9789.1710984595.git.anand.jain@oracle.com/
Anand Jain (3):
common/btrfs: refactor _require_btrfs_corrupt_block to check option
btrfs/290: fix btrfs_corrupt_block options
common/verity: fix btrfs-corrupt-block -v option
common/btrfs | 15 +++++++++++++++
common/verity | 5 +++--
tests/btrfs/290 | 24 +++++++++++++++---------
3 files changed, 33 insertions(+), 11 deletions(-)
--
2.39.3
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 1/3] common/btrfs: refactor _require_btrfs_corrupt_block to check option 2024-03-22 11:16 [PATCH v2 0/3] fix btrfs-corrupt-block options value and offset Anand Jain @ 2024-03-22 11:16 ` Anand Jain 2024-04-24 14:37 ` Zorro Lang 2024-03-22 11:16 ` [PATCH v2 2/3] btrfs/290: fix btrfs_corrupt_block options Anand Jain 2024-03-22 11:16 ` [PATCH v2 3/3] common/verity: fix btrfs-corrupt-block -v option Anand Jain 2 siblings, 1 reply; 7+ messages in thread From: Anand Jain @ 2024-03-22 11:16 UTC (permalink / raw) To: fstests; +Cc: linux-btrfs, fdmanana The -v and -o short options in btrfs-corrupt-block were introduced and replaced with the long options --value and --offset in the same btrfs-progs release 5.19 by the following commits: b2ada0594116 ("btrfs-progs: corrupt-block: corrupt generic item data") 22ffee3c6cf2 ("btrfs-progs: corrupt-block: use only long options for value and offset") We hope that if these commits are backported, they are both backported at the same time. Use only the long options of btrfs-corrupt-block in the test cases. Also, check if btrfs-corrupt-block has the options --value and --offset. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- common/btrfs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/common/btrfs b/common/btrfs index ae13fb55cbc6..a8340fdd4748 100644 --- a/common/btrfs +++ b/common/btrfs @@ -659,7 +659,22 @@ _btrfs_buffered_read_on_mirror() _require_btrfs_corrupt_block() { + # An optional arg1 argument to also check the option. + local opt=$1 + local ret + _require_command "$BTRFS_CORRUPT_BLOCK_PROG" btrfs-corrupt-block + + if [ -z "$opt" ]; then + return + fi + + $BTRFS_CORRUPT_BLOCK_PROG -h 2>&1 | grep -q -- " --$opt " + ret=$? + + if [ $ret != 0 ]; then + _notrun "Require $BTRFS_CORRUPT_BLOCK_PROG option --$opt" + fi } _require_btrfs_send_version() -- 2.39.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] common/btrfs: refactor _require_btrfs_corrupt_block to check option 2024-03-22 11:16 ` [PATCH v2 1/3] common/btrfs: refactor _require_btrfs_corrupt_block to check option Anand Jain @ 2024-04-24 14:37 ` Zorro Lang 0 siblings, 0 replies; 7+ messages in thread From: Zorro Lang @ 2024-04-24 14:37 UTC (permalink / raw) To: Anand Jain; +Cc: fstests, linux-btrfs, fdmanana On Fri, Mar 22, 2024 at 04:46:39PM +0530, Anand Jain wrote: > The -v and -o short options in btrfs-corrupt-block were introduced and > replaced with the long options --value and --offset in the same > btrfs-progs release 5.19 by the following commits: > > b2ada0594116 ("btrfs-progs: corrupt-block: corrupt generic item data") > 22ffee3c6cf2 ("btrfs-progs: corrupt-block: use only long options for value and offset") > > We hope that if these commits are backported, they are both backported at > the same time. > > Use only the long options of btrfs-corrupt-block in the test cases. Also, > check if btrfs-corrupt-block has the options --value and --offset. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > common/btrfs | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/common/btrfs b/common/btrfs > index ae13fb55cbc6..a8340fdd4748 100644 > --- a/common/btrfs > +++ b/common/btrfs > @@ -659,7 +659,22 @@ _btrfs_buffered_read_on_mirror() > > _require_btrfs_corrupt_block() > { > + # An optional arg1 argument to also check the option. > + local opt=$1 > + local ret > + > _require_command "$BTRFS_CORRUPT_BLOCK_PROG" btrfs-corrupt-block > + > + if [ -z "$opt" ]; then > + return > + fi > + > + $BTRFS_CORRUPT_BLOCK_PROG -h 2>&1 | grep -q -- " --$opt " I'm wondering if we can use the "-w" option to replace the two blanks around the --$opt (due to I don't know if there always be two blanks), likes: grep -qw -- "--$opt" ? > + ret=$? > + > + if [ $ret != 0 ]; then The "ret" looks like useless at here. If there's not other place uses this return value, we can use "$?" directly. If no objection from you, I can help to change that when I merge. Others looks good to me. Reviewed-by: Zorro Lang <zlang@redhat.com> > + _notrun "Require $BTRFS_CORRUPT_BLOCK_PROG option --$opt" > + fi > } > > _require_btrfs_send_version() > -- > 2.39.3 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] btrfs/290: fix btrfs_corrupt_block options 2024-03-22 11:16 [PATCH v2 0/3] fix btrfs-corrupt-block options value and offset Anand Jain 2024-03-22 11:16 ` [PATCH v2 1/3] common/btrfs: refactor _require_btrfs_corrupt_block to check option Anand Jain @ 2024-03-22 11:16 ` Anand Jain 2024-04-24 14:39 ` Zorro Lang 2024-03-22 11:16 ` [PATCH v2 3/3] common/verity: fix btrfs-corrupt-block -v option Anand Jain 2 siblings, 1 reply; 7+ messages in thread From: Anand Jain @ 2024-03-22 11:16 UTC (permalink / raw) To: fstests; +Cc: linux-btrfs, fdmanana Checks if the running btrfs-corrupt-block also has the options value and offset. Remove btrfs-corrupt-block command's STDOUT and STDERR output redirection to /dev/null. Without this, debugging wasn't possible. I also noticed that command is quiet when successfull, so no redirect to $seqres.full is required. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- tests/btrfs/290 | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/tests/btrfs/290 b/tests/btrfs/290 index 61e741faeb45..281333b200f9 100755 --- a/tests/btrfs/290 +++ b/tests/btrfs/290 @@ -31,7 +31,8 @@ _require_odirect _require_xfs_io_command "falloc" _require_xfs_io_command "pread" _require_xfs_io_command "pwrite" -_require_btrfs_corrupt_block +_require_btrfs_corrupt_block "value" +_require_btrfs_corrupt_block "offset" _disable_fsverity_signatures get_ino() { @@ -58,7 +59,7 @@ corrupt_inline() { _scratch_unmount # inline data starts at disk_bytenr # overwrite the first u64 with random bogus junk - $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f disk_bytenr $SCRATCH_DEV > /dev/null 2>&1 + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f disk_bytenr $SCRATCH_DEV _scratch_mount validate $f } @@ -72,7 +73,7 @@ corrupt_prealloc_to_reg() { _scratch_unmount # ensure non-zero at the pre-allocated region on disk # set extent type from prealloc (2) to reg (1) - $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type -v 1 $SCRATCH_DEV >/dev/null 2>&1 + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type --value 1 $SCRATCH_DEV _scratch_mount # now that it's a regular file, reading actually looks at the previously # preallocated region, so ensure that has non-zero contents. @@ -88,7 +89,7 @@ corrupt_reg_to_prealloc() { _fsv_enable $f _scratch_unmount # set type from reg (1) to prealloc (2) - $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type -v 2 $SCRATCH_DEV >/dev/null 2>&1 + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type --value 2 $SCRATCH_DEV _scratch_mount validate $f } @@ -104,7 +105,8 @@ corrupt_punch_hole() { _fsv_enable $f _scratch_unmount # change disk_bytenr to 0, representing a hole - $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 4096 -f disk_bytenr -v 0 $SCRATCH_DEV > /dev/null 2>&1 + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 4096 -f disk_bytenr --value 0 \ + $SCRATCH_DEV _scratch_mount validate $f } @@ -118,7 +120,8 @@ corrupt_plug_hole() { _fsv_enable $f _scratch_unmount # change disk_bytenr to some value, plugging the hole - $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 4096 -f disk_bytenr -v 13639680 $SCRATCH_DEV > /dev/null 2>&1 + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 4096 -f disk_bytenr \ + --value 13639680 $SCRATCH_DEV _scratch_mount validate $f } @@ -132,7 +135,8 @@ corrupt_verity_descriptor() { _scratch_unmount # key for the descriptor item is <inode, BTRFS_VERITY_DESC_ITEM_KEY, 1>, # 88 is X. So we write 5 Xs to the start of the descriptor - $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 -v 88 -o 0 -b 5 $SCRATCH_DEV > /dev/null 2>&1 + $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 --value 88 --offset 0 -b 5 \ + $SCRATCH_DEV _scratch_mount validate $f } @@ -144,7 +148,8 @@ corrupt_root_hash() { local ino=$(get_ino $f) _fsv_enable $f _scratch_unmount - $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 -v 88 -o 16 -b 1 $SCRATCH_DEV > /dev/null 2>&1 + $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 --value 88 --offset 16 -b 1 \ + $SCRATCH_DEV _scratch_mount validate $f } @@ -159,7 +164,8 @@ corrupt_merkle_tree() { # key for the descriptor item is <inode, BTRFS_VERITY_MERKLE_ITEM_KEY, 0>, # 88 is X. So we write 5 Xs to somewhere in the middle of the first # merkle item - $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 -v 88 -o 100 -b 5 $SCRATCH_DEV > /dev/null 2>&1 + $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 --value 88 --offset 100 \ + -b 5 $SCRATCH_DEV _scratch_mount validate $f } -- 2.39.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] btrfs/290: fix btrfs_corrupt_block options 2024-03-22 11:16 ` [PATCH v2 2/3] btrfs/290: fix btrfs_corrupt_block options Anand Jain @ 2024-04-24 14:39 ` Zorro Lang 0 siblings, 0 replies; 7+ messages in thread From: Zorro Lang @ 2024-04-24 14:39 UTC (permalink / raw) To: Anand Jain; +Cc: fstests, linux-btrfs, fdmanana On Fri, Mar 22, 2024 at 04:46:40PM +0530, Anand Jain wrote: > Checks if the running btrfs-corrupt-block also has the options value and > offset. > > Remove btrfs-corrupt-block command's STDOUT and STDERR output redirection > to /dev/null. Without this, debugging wasn't possible. I also noticed that > command is quiet when successfull, so no redirect to $seqres.full is required. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- Good to me, Reviewed-by: Zorro Lang <zlang@redhat.com> > tests/btrfs/290 | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/tests/btrfs/290 b/tests/btrfs/290 > index 61e741faeb45..281333b200f9 100755 > --- a/tests/btrfs/290 > +++ b/tests/btrfs/290 > @@ -31,7 +31,8 @@ _require_odirect > _require_xfs_io_command "falloc" > _require_xfs_io_command "pread" > _require_xfs_io_command "pwrite" > -_require_btrfs_corrupt_block > +_require_btrfs_corrupt_block "value" > +_require_btrfs_corrupt_block "offset" > _disable_fsverity_signatures > > get_ino() { > @@ -58,7 +59,7 @@ corrupt_inline() { > _scratch_unmount > # inline data starts at disk_bytenr > # overwrite the first u64 with random bogus junk > - $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f disk_bytenr $SCRATCH_DEV > /dev/null 2>&1 > + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f disk_bytenr $SCRATCH_DEV > _scratch_mount > validate $f > } > @@ -72,7 +73,7 @@ corrupt_prealloc_to_reg() { > _scratch_unmount > # ensure non-zero at the pre-allocated region on disk > # set extent type from prealloc (2) to reg (1) > - $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type -v 1 $SCRATCH_DEV >/dev/null 2>&1 > + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type --value 1 $SCRATCH_DEV > _scratch_mount > # now that it's a regular file, reading actually looks at the previously > # preallocated region, so ensure that has non-zero contents. > @@ -88,7 +89,7 @@ corrupt_reg_to_prealloc() { > _fsv_enable $f > _scratch_unmount > # set type from reg (1) to prealloc (2) > - $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type -v 2 $SCRATCH_DEV >/dev/null 2>&1 > + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type --value 2 $SCRATCH_DEV > _scratch_mount > validate $f > } > @@ -104,7 +105,8 @@ corrupt_punch_hole() { > _fsv_enable $f > _scratch_unmount > # change disk_bytenr to 0, representing a hole > - $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 4096 -f disk_bytenr -v 0 $SCRATCH_DEV > /dev/null 2>&1 > + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 4096 -f disk_bytenr --value 0 \ > + $SCRATCH_DEV > _scratch_mount > validate $f > } > @@ -118,7 +120,8 @@ corrupt_plug_hole() { > _fsv_enable $f > _scratch_unmount > # change disk_bytenr to some value, plugging the hole > - $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 4096 -f disk_bytenr -v 13639680 $SCRATCH_DEV > /dev/null 2>&1 > + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 4096 -f disk_bytenr \ > + --value 13639680 $SCRATCH_DEV > _scratch_mount > validate $f > } > @@ -132,7 +135,8 @@ corrupt_verity_descriptor() { > _scratch_unmount > # key for the descriptor item is <inode, BTRFS_VERITY_DESC_ITEM_KEY, 1>, > # 88 is X. So we write 5 Xs to the start of the descriptor > - $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 -v 88 -o 0 -b 5 $SCRATCH_DEV > /dev/null 2>&1 > + $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 --value 88 --offset 0 -b 5 \ > + $SCRATCH_DEV > _scratch_mount > validate $f > } > @@ -144,7 +148,8 @@ corrupt_root_hash() { > local ino=$(get_ino $f) > _fsv_enable $f > _scratch_unmount > - $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 -v 88 -o 16 -b 1 $SCRATCH_DEV > /dev/null 2>&1 > + $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 --value 88 --offset 16 -b 1 \ > + $SCRATCH_DEV > _scratch_mount > validate $f > } > @@ -159,7 +164,8 @@ corrupt_merkle_tree() { > # key for the descriptor item is <inode, BTRFS_VERITY_MERKLE_ITEM_KEY, 0>, > # 88 is X. So we write 5 Xs to somewhere in the middle of the first > # merkle item > - $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 -v 88 -o 100 -b 5 $SCRATCH_DEV > /dev/null 2>&1 > + $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 --value 88 --offset 100 \ > + -b 5 $SCRATCH_DEV > _scratch_mount > validate $f > } > -- > 2.39.3 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] common/verity: fix btrfs-corrupt-block -v option 2024-03-22 11:16 [PATCH v2 0/3] fix btrfs-corrupt-block options value and offset Anand Jain 2024-03-22 11:16 ` [PATCH v2 1/3] common/btrfs: refactor _require_btrfs_corrupt_block to check option Anand Jain 2024-03-22 11:16 ` [PATCH v2 2/3] btrfs/290: fix btrfs_corrupt_block options Anand Jain @ 2024-03-22 11:16 ` Anand Jain 2024-04-24 14:45 ` Zorro Lang 2 siblings, 1 reply; 7+ messages in thread From: Anand Jain @ 2024-03-22 11:16 UTC (permalink / raw) To: fstests; +Cc: linux-btrfs, fdmanana The btrfs-corrupt-block -v has been replaced with --value so fix it. _fsv_scratch_corrupt_merkle_tree() uses the btrfs-corrupt-block --value option, so add the "value" prerequisite in the function _require_fsverity_corruption. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- common/verity | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/common/verity b/common/verity index 03d175ce1b7a..59b67e12010a 100644 --- a/common/verity +++ b/common/verity @@ -191,7 +191,7 @@ _require_fsverity_corruption() { _require_xfs_io_command "fiemap" if [ $FSTYP == "btrfs" ]; then - _require_btrfs_corrupt_block + _require_btrfs_corrupt_block "value" fi } @@ -402,7 +402,8 @@ _fsv_scratch_corrupt_merkle_tree() # in the default filesystem tree (-r 5) and corrupt one byte (-b 1) at # $offset (-o $offset) with the ascii representation of the byte we read # (-v $ascii) - $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 -v $ascii -o $offset -b 1 $SCRATCH_DEV + $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 \ + --value $ascii --offset $offset -b 1 $SCRATCH_DEV (( offset += 1 )) done _scratch_mount -- 2.39.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/3] common/verity: fix btrfs-corrupt-block -v option 2024-03-22 11:16 ` [PATCH v2 3/3] common/verity: fix btrfs-corrupt-block -v option Anand Jain @ 2024-04-24 14:45 ` Zorro Lang 0 siblings, 0 replies; 7+ messages in thread From: Zorro Lang @ 2024-04-24 14:45 UTC (permalink / raw) To: Anand Jain; +Cc: fstests, linux-btrfs, fdmanana On Fri, Mar 22, 2024 at 04:46:41PM +0530, Anand Jain wrote: > The btrfs-corrupt-block -v has been replaced with --value so fix it. > > _fsv_scratch_corrupt_merkle_tree() uses the btrfs-corrupt-block > --value option, so add the "value" prerequisite in the function > _require_fsverity_corruption. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- It makes sense to me, as an option is changed. But I think it'll cause _notrun on old btrfs-corrupt-block command/version. If btrfs list feels good for that (no objection), then that's fine. Reviewed-by: Zorro Lang <zlang@redhat.com> > common/verity | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/common/verity b/common/verity > index 03d175ce1b7a..59b67e12010a 100644 > --- a/common/verity > +++ b/common/verity > @@ -191,7 +191,7 @@ _require_fsverity_corruption() > { > _require_xfs_io_command "fiemap" > if [ $FSTYP == "btrfs" ]; then > - _require_btrfs_corrupt_block > + _require_btrfs_corrupt_block "value" > fi > } > > @@ -402,7 +402,8 @@ _fsv_scratch_corrupt_merkle_tree() > # in the default filesystem tree (-r 5) and corrupt one byte (-b 1) at > # $offset (-o $offset) with the ascii representation of the byte we read > # (-v $ascii) > - $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 -v $ascii -o $offset -b 1 $SCRATCH_DEV > + $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 \ > + --value $ascii --offset $offset -b 1 $SCRATCH_DEV > (( offset += 1 )) > done > _scratch_mount > -- > 2.39.3 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-04-24 14:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-22 11:16 [PATCH v2 0/3] fix btrfs-corrupt-block options value and offset Anand Jain 2024-03-22 11:16 ` [PATCH v2 1/3] common/btrfs: refactor _require_btrfs_corrupt_block to check option Anand Jain 2024-04-24 14:37 ` Zorro Lang 2024-03-22 11:16 ` [PATCH v2 2/3] btrfs/290: fix btrfs_corrupt_block options Anand Jain 2024-04-24 14:39 ` Zorro Lang 2024-03-22 11:16 ` [PATCH v2 3/3] common/verity: fix btrfs-corrupt-block -v option Anand Jain 2024-04-24 14:45 ` Zorro Lang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox