On 2018年03月26日 21:24, Nikolay Borisov wrote: > > > On 25.03.2018 14:44, Qu Wenruo wrote: >> >> >> 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 <nbori...@suse.com> >> >> 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 how exactly is this argument supposed to be used. I tried:
TEST_ARGS_CHECK="--mode=lowmem" is forgot. > > TEST_ENABLE_OVERRIDE=true TEST=031\* ./fsck-tests.sh - no --lowmem is > appended > > I modify the TEST_ENABLE_OVERRIDE=false to TEST_ENABLE_OVERRIDE=true in > common.local and then just run TEST=031\* ./fsck-tests.sh and it works. > > TEST_ENABLE_OVERRIDE=true TEST_ARGS_CHECK=--mode=lowmem TEST=031\* > ./fsck-tests.sh - this also works? Yep > > > I'm not entirely sure what the ci config is but at this point I'm > beginning to worry that if I omit the explicit --mode option this test > might never be run in lowmem mode. > > The whole TEST_ENABLE_OVERRIDE situation seems a bit finicky. I'm not familiar with CI things so it may be a good time point to check and enhance it if possible? Thanks, Qu > >> >> 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 >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
signature.asc
Description: OpenPGP digital signature
