Re: [PATCH v2 3/3] overlay: mount/unmount base fs before/after running tests

Amir Goldstein
2017-02-07T13:24:11Z

On Tue, Feb 7, 2017 at 2:47 PM, Eryu Guan <eguan@redhat.com> 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
>>