On Tue, Feb 7, 2017 at 2:47 PM, Eryu Guan <eguan@xxxxxxxxxx> wrote: > On Fri, Jan 27, 2017 at 10:56:05AM +0200, Amir Goldstein wrote: >> Optionally configure TEST/SCRATCH_BASE_DEV, for overlay base fs >> to be mounted/unmounted before/after running tests. For example: >> >> +export TEST_BASE_DEV=/dev/mapper/base-test >> export TEST_BASE_MNT=/mnt/base/test >> +export SCRATCH_BASE_DEV=/dev/mapper/base-scratch >> export SCRATCH_BASE_MNT=/mnt/base/scratch >> export TEST_DIR=/mnt/test >> export SCRATCH_MNT=/mnt/scratch >> export FSTYP=overlay >> >> With this change, the base fs is mounted before running tests, >> unmounted after running tests and recycled on _test_cycle_mount >> along with the overlay mounts. >> This helps catching overlayfs bugs related to leaking objects in >> underlying (base) fs. >> >> The standard way that overlay tests work is: >> - _scratch_mkfs >> - setup lower/upper dir files >> - _scratch_mount (or custom overlay mount) >> - (sometimes) _scratch_unmount/_scratch_mount recycle >> >> To preserve expected tests behavior, the semantics are: >> - _scratch_mkfs mounts the base fs, cleans the lower/upper dirs and >> keeps base fs mounted. >> - _scratch_mount mounts or recycles base fs and mounts overlay >> - _scratch_unmount unmounts overlay and base fs > > I think we should do some validation on *_BASE_DEV first, to see if they > could be mounted; if they're mounted already, check if they're mounted > at the expected place (*_BASE_MNT). Otherwise I see test failures, which > are hard to tell which part went wrong just from the failure diff > message. > OK. will add some sanity for OVL_*_BASE_DEV > e.g. I tried clearing filesystem signatures (wipefs -a), mounting > SCRATCH_BASE_DEV to other random dirs, I saw failures like: > > overlay/002 1s ... [failed, exit status 1] - output mismatch (see /root/workspace/xfstests/results//overlay/002.out.bad) > --- tests/overlay/002.out 2016-07-01 23:17:01.029000000 +0800 > +++ /root/workspace/xfstests/results//overlay/002.out.bad 2017-02-07 18:47:22.425000000 +0800 > @@ -1,3 +1,3 @@ > QA output created by 002 > -wrote 65536/65536 bytes at offset 0 > -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > +umount: /mnt/ovl/scratch: not mounted > +failed to unmount /mnt/ovl/scratch/ovl > ... > (Run 'diff -u tests/overlay/002.out /root/workspace/xfstests/results//overlay/002.out.bad' to see the entire diff) > overlay/003 1s ... - output mismatch (see /root/workspace/xfstests/results//overlay/003.out.bad) > --- tests/overlay/003.out 2016-07-01 23:17:01.029000000 +0800 > +++ /root/workspace/xfstests/results//overlay/003.out.bad 2017-02-07 18:47:23.041000000 +0800 > @@ -1,2 +1,12 @@ > QA output created by 003 > +mount: wrong fs type, bad option, bad superblock on /dev/sda6, > + missing codepage or helper program, or other error > + > + In some cases useful info is found in syslog - try > + dmesg | tail or so. > +mount: wrong fs type, bad option, bad superblock on /dev/sda6, > ... > (Run 'diff -u tests/overlay/003.out /root/workspace/xfstests/results//overlay/003.out.bad' to see the entire diff) > > And since overlayfs configuration is going to be a bit more complex > after this patchset, can you please update README to document overlayfs > test as well? (Sorry, I forgot to write doc when I introduced overlayfs > support..) Yes, it took me some time reading the code to understand how to configure overlayfs first time I tried :-) Will do my best to improve README > > Thanks a lot for all your effort! > > Eryu > >> >> Tests that use _scratch_unmount to unmount a custom overlay mount >> and expect to have access to overlay base dir, were converted to use >> explicit umount $SCRATCH_MNT instead. >> --- >> common/config | 6 +++++- >> common/rc | 42 ++++++++++++++++++++++++++++++++++++++++++ >> tests/overlay/003 | 3 ++- >> tests/overlay/004 | 3 ++- >> tests/overlay/014 | 5 +++-- >> 5 files changed, 54 insertions(+), 5 deletions(-) >> >> diff --git a/common/config b/common/config >> index db5721c..f431a8d 100644 >> --- a/common/config >> +++ b/common/config >> @@ -35,7 +35,9 @@ >> # RMT_TAPE_DEV - the remote tape device for the xfsdump tests >> # RMT_IRIXTAPE_DEV- the IRIX remote tape device for the xfsdump tests >> # RMT_TAPE_USER - remote user for tape device >> +# TEST_BASE_DEV - device for base fs containing overlay test dirs >> # TEST_BASE_MNT - mount point for base fs of overlay test dirs >> +# SCRATCH_BASE_DEV- device for base fs containing overlay scratch dirs >> # SCRATCH_BASE_MNT- mount point for base fs of overlay scratch dirs >> # >> # - These can be added to $HOST_CONFIG_DIR (witch default to ./config) >> @@ -470,11 +472,12 @@ _config_overlay() >> [ -z "$TEST_DEV" ] || export TEST_BASE_DIR="$TEST_DEV" >> [ -z "$SCRATCH_DEV" ] || export SCRATCH_BASE_DIR="$SCRATCH_DEV" >> >> - # 2. set SCRATCH/TEST_BASE_MNT to configure base fs. >> + # 2. set SCRATCH/TEST_BASE_DEV/MNT to configure base fs. >> # SCRATCH/TEST_DEV are derived from SCRATCH/TEST_BASE_MNT >> # and therein, overlay fs directories will be created. >> [ -n "$TEST_BASE_MNT" ] || return >> >> + _check_device TEST_BASE_DEV optional $TEST_BASE_DEV >> if [ ! -d "$TEST_BASE_MNT" ]; then >> echo "common/config: Error: \$TEST_BASE_MNT ($TEST_BASE_MNT) is not a directory" >> exit 1 >> @@ -486,6 +489,7 @@ _config_overlay() >> >> [ -n "$SCRATCH_BASE_MNT" ] || return >> >> + _check_device SCRATCH_BASE_DEV optional $SCRATCH_BASE_DEV >> if [ ! -d "$SCRATCH_BASE_MNT" ]; then >> echo "common/config: Error: \$SCRATCH_BASE_MNT ($SCRATCH_BASE_MNT) is not a directory" >> exit 1 >> diff --git a/common/rc b/common/rc >> index f5ab869..5033ed4 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -320,24 +320,62 @@ _overlay_mount() >> $SELINUX_MOUNT_OPTIONS $* $dir $mnt >> } >> >> +_overlay_test_base_mount() >> +{ >> + if [ -n "$TEST_BASE_DEV" -a -n "$TEST_BASE_MNT" ]; then >> + _mount $TEST_BASE_MOUNT_OPTIONS \ >> + $SELINUX_MOUNT_OPTIONS \ >> + $TEST_BASE_DEV $TEST_BASE_MNT >> + fi >> +} >> + >> _overlay_test_mount() >> { >> + _overlay_test_base_mount >> _overlay_mount $TEST_BASE_DIR $TEST_DIR $* >> } >> >> +_overlay_scratch_base_mount() >> +{ >> + if [ -n "$SCRATCH_BASE_DEV" -a -n "$SCRATCH_BASE_MNT" ]; then >> + _mount $SCRATCH_BASE_MOUNT_OPTIONS \ >> + $SELINUX_MOUNT_OPTIONS \ >> + $SCRATCH_BASE_DEV $SCRATCH_BASE_MNT >> + fi >> +} >> + >> +_overlay_scratch_base_unmount() >> +{ >> + if [ -n "$SCRATCH_BASE_DEV" -a -n "$SCRATCH_BASE_MNT" ]; then >> + $UMOUNT_PROG $SCRATCH_BASE_MNT >> + fi >> +} >> + >> _overlay_scratch_mount() >> { >> + # base fs may be mounted after overlay mkfs >> + _overlay_scratch_base_unmount 2>/dev/null >> + _overlay_scratch_base_mount >> _overlay_mount $SCRATCH_BASE_DIR $SCRATCH_MNT $* >> } >> >> +_overlay_test_base_unmount() >> +{ >> + if [ -n "$TEST_BASE_DEV" -a -n "$TEST_BASE_MNT" ]; then >> + $UMOUNT_PROG $TEST_BASE_MNT >> + fi >> +} >> + >> _overlay_test_unmount() >> { >> $UMOUNT_PROG $TEST_DIR >> + _overlay_test_base_unmount >> } >> >> _overlay_scratch_unmount() >> { >> $UMOUNT_PROG $SCRATCH_MNT >> + _overlay_scratch_base_unmount >> } >> >> _scratch_mount() >> @@ -643,7 +681,11 @@ _scratch_cleanup_files() >> overlay) >> # Avoid rm -rf /* if we messed up >> [ -n "$SCRATCH_BASE_DIR" ] || return >> + # overlay 'mkfs' needs to make sure base fs is mounted and clean >> + _overlay_scratch_base_unmount 2>/dev/null >> + _overlay_scratch_base_mount >> rm -rf $SCRATCH_BASE_DIR/* >> + # leave base fs mouted so tests can setup lower dir >> ;; >> *) >> [ -n "$SCRATCH_MNT" ] || return >> diff --git a/tests/overlay/003 b/tests/overlay/003 >> index bb59700..81aea85 100755 >> --- a/tests/overlay/003 >> +++ b/tests/overlay/003 >> @@ -89,7 +89,8 @@ rm -rf ${SCRATCH_MNT}/* >> # nothing should be listed >> ls ${SCRATCH_MNT}/ >> >> -_scratch_unmount >> +# unmount overlayfs but not base fs >> +$UMOUNT_PROG $SCRATCH_MNT >> >> rm -rf $lowerdir >> echo "Silence is golden" >> diff --git a/tests/overlay/004 b/tests/overlay/004 >> index 6d78f9f..26ac547 100755 >> --- a/tests/overlay/004 >> +++ b/tests/overlay/004 >> @@ -85,7 +85,8 @@ _user_do "chmod g+t ${SCRATCH_MNT}/attr_file2 > /dev/null 2>&1" >> _user_do "chmod u-X ${SCRATCH_MNT}/attr_file2 > /dev/null 2>&1" >> stat -c %a ${SCRATCH_MNT}/attr_file2 >> >> -_scratch_unmount >> +# unmount overlayfs but not base fs >> +$UMOUNT_PROG $SCRATCH_MNT >> >> # check mode bits of the file that has been copied up, and >> # the file that should not have been copied up. >> diff --git a/tests/overlay/014 b/tests/overlay/014 >> index 7426c31..6519432 100755 >> --- a/tests/overlay/014 >> +++ b/tests/overlay/014 >> @@ -73,7 +73,8 @@ mkdir -p $lowerdir1/testdir/d >> _overlay_mount_dirs $lowerdir1 $lowerdir2 $workdir $SCRATCH_BASE_DIR $SCRATCH_MNT >> rm -rf $SCRATCH_MNT/testdir >> mkdir -p $SCRATCH_MNT/testdir/visibledir >> -_scratch_unmount >> +# unmount overlayfs but not base fs >> +$UMOUNT_PROG $SCRATCH_MNT >> >> # mount overlay again, with lowerdir1 and lowerdir2 as multiple lowerdirs, >> # and create a new file in testdir, triggers copyup from lowerdir, >> @@ -84,7 +85,7 @@ touch $SCRATCH_MNT/testdir/visiblefile >> >> # umount and mount overlay again, buggy kernel treats the copied-up dir as >> # opaque, visibledir is not seen in merged dir. >> -_scratch_unmount >> +$UMOUNT_PROG $SCRATCH_MNT >> _overlay_mount_dirs "$lowerdir2:$lowerdir1" $upperdir $workdir \ >> $SCRATCH_BASE_DIR $SCRATCH_MNT >> ls $SCRATCH_MNT/testdir >> -- >> 2.7.4 >> -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html