On 2018年03月23日 22:48, Nikolay Borisov wrote: > Verify that if we have an otherwise clean filesystem, containging collided > DIR_ITEM, btrfs check lowmem's mode can correctly handle those and not produce > any false positives. > > This if fixed by commit titled: > > "btrfs-progs: Fix DIR_ITEM checking in lowmem" > > Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx> Looks pretty good. However a nitpick inlined below. > --- > .../031-lowmem-collission-dir-items/test.sh | 27 ++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > create mode 100755 tests/fsck-tests/031-lowmem-collission-dir-items/test.sh > > diff --git a/tests/fsck-tests/031-lowmem-collission-dir-items/test.sh b/tests/fsck-tests/031-lowmem-collission-dir-items/test.sh > new file mode 100755 > index 0000000..8a01889 > --- /dev/null > +++ b/tests/fsck-tests/031-lowmem-collission-dir-items/test.sh > @@ -0,0 +1,27 @@ > +#!/bin/bash > +# Ensure that running btrfs check in lowmem mode on a fs > +# which contains DIR_ITEM with collissions handles it > +# properly > +source "$TEST_TOP/common" > + > +check_prereq btrfs > +check_prereq mkfs.btrfs > + > +setup_root_helper > +prepare_test_dev > + > +run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV" > +run_check_mount_test_dev > + > +# Create 2 files whose names collide > + > +run_check $SUDO_HELPER touch "$TEST_MNT/5ab4e206~~~~~~~~XVT1U3ZF647YS2PD4AKAG826" > +run_check $SUDO_HELPER touch "$TEST_MNT/5ab4e26a~~~~~~~~AP1C3VQBE79IJOTVOEZIR9YU" > + > +run_check_umount_test_dev > + > +# The fs is clean so lowmem shouldn't produce any warnings > +run_check "$TOP/btrfs" check --mode=lowmem --readonly "$TEST_DEV" I understand this is a pinpoint test case for lowmem mode, but for lowmem mode test, we set TEST_ENABLE_OVERRIDE and the whole test case will use lowmem mode. So it doesn't seem necessary to explicitly call lowmem check here. Just normal run_check "$TOP/btrfs" check would be enough. (And this will also test original mode) > +if [ $? -ne 0 ]; then > + _fail "check --lowmem doesn't handle collissioned DIR_ITEMs correctly" > +fi > IIRC run_check() will check the return value and exit if failure. So this seems not necessary. Thanks, Qu
Attachment:
signature.asc
Description: OpenPGP digital signature
