* [PATCH xfstest v2 1/4] overlay: add filesystem check helper
2017-12-28 11:49 [PATCH xfstest v2 0/4] overlay: add fsck.overlay basic tests zhangyi (F)
@ 2017-12-28 11:49 ` zhangyi (F)
2017-12-28 12:12 ` Amir Goldstein
2017-12-28 11:49 ` [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test zhangyi (F)
` (2 subsequent siblings)
3 siblings, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-28 11:49 UTC (permalink / raw)
To: linux-unionfs, fstests
Cc: miklos, amir73il, eguan, yi.zhang, miaoxie, yangerkun
Add filesystem check helpers for the upcoming fsck.overlay utility,
and hook them to _check_test_fs and _check_scratch_fs. This helper
works only if fsck.overlay exists.
[ _check_test_fs/_check_scratch_fs part picked from Amir's patch, thanks ]
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
common/config | 1 +
common/overlay | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
common/rc | 4 +--
3 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/common/config b/common/config
index d0fbfe5..0505b71 100644
--- a/common/config
+++ b/common/config
@@ -236,6 +236,7 @@ case "$HOSTOS" in
export MKFS_REISER4_PROG="`set_prog_path mkfs.reiser4`"
export E2FSCK_PROG="`set_prog_path e2fsck`"
export TUNE2FS_PROG="`set_prog_path tune2fs`"
+ export FSCK_OVERLAY_PROG="`set_prog_path fsck.overlay`"
;;
esac
diff --git a/common/overlay b/common/overlay
index 1da4ab1..e7f0e62 100644
--- a/common/overlay
+++ b/common/overlay
@@ -151,3 +151,83 @@ _require_scratch_overlay_feature()
_notrun "${FSTYP} feature '${feature}' cannot be enabled on ${SCRATCH_DEV}"
_scratch_unmount
}
+
+_overlay_fsck_dirs()
+{
+ local lowerdir=$1
+ local upperdir=$2
+ local workdir=$3
+ local options=$4
+
+ [[ ! -x "$FSCK_OVERLAY_PROG" ]] && return 0
+
+ $FSCK_OVERLAY_PROG -o lowerdir=$lowerdir -o upperdir=$upperdir \
+ -o workdir=$workdir $options
+}
+
+_overlay_check_dirs()
+{
+ local lowerdir=$1
+ local upperdir=$2
+ local workdir=$3
+ local err=0
+
+ _overlay_fsck_dirs $* $FSCK_OPTIONS >>$tmp.fsck 2>&1
+ if [ $? -ne 0 ]; then
+ _log_err "_overlay_check_fs: overlayfs on $lowerdir,$upperdir,$workdir is inconsistent"
+ echo "*** fsck.overlay output ***" >>$seqres.full
+ cat $tmp.fsck >>$seqres.full
+ echo "*** end fsck.overlay output" >>$seqres.full
+
+ echo "*** mount output ***" >>$seqres.full
+ _mount >>$seqres.full
+ echo "*** end mount output" >>$seqres.full
+ err=1
+ fi
+ rm -f $tmp.fsck
+
+ if [ $err != 0 ]; then
+ status=1
+ if [ "$iam" != "check" ]; then
+ exit 1
+ fi
+ return 1
+ fi
+
+ return 0
+}
+
+_overlay_check_fs()
+{
+ local base_dev=$3
+ local base_mnt=$4
+
+ [ "$FSTYP" = overlay ] || return 0
+
+ # Base fs needs to be mounted to check overlay dirs
+ local mounted=""
+ [ -z "$base_dev" ] || mounted=`_is_mounted $base_dev`
+ [ -n "$mounted" ] || _overlay_base_mount $*
+
+ # No need to umount overlay for dir checks with default -n option
+ _overlay_check_dirs $base_mnt/$OVL_LOWER $base_mnt/$OVL_UPPER \
+ $base_mnt/$OVL_WORK
+ local ret=$?
+
+ [ -n "$mounted" ] || _overlay_base_unmount "$base_dev" "$base_mnt"
+ return $ret
+}
+
+_check_overlay_test_fs()
+{
+ _overlay_check_fs OVL_BASE_TEST_DEV OVL_BASE_TEST_DIR \
+ "$OVL_BASE_TEST_DEV" "$OVL_BASE_TEST_DIR" \
+ $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS
+}
+
+_check_overlay_scratch_fs()
+{
+ _overlay_check_fs OVL_BASE_SCRATCH_DEV OVL_BASE_SCRATCH_MNT \
+ "$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT" \
+ $OVL_BASE_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
+}
diff --git a/common/rc b/common/rc
index 4c053a5..79d5f25 100644
--- a/common/rc
+++ b/common/rc
@@ -2507,7 +2507,7 @@ _check_test_fs()
# no way to check consistency for GlusterFS
;;
overlay)
- # no way to check consistency for overlay
+ _check_overlay_test_fs
;;
pvfs2)
;;
@@ -2562,7 +2562,7 @@ _check_scratch_fs()
# no way to check consistency for GlusterFS
;;
overlay)
- # no way to check consistency for overlay
+ _check_overlay_scratch_fs
;;
pvfs2)
;;
--
2.5.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH xfstest v2 1/4] overlay: add filesystem check helper
2017-12-28 11:49 ` [PATCH xfstest v2 1/4] overlay: add filesystem check helper zhangyi (F)
@ 2017-12-28 12:12 ` Amir Goldstein
0 siblings, 0 replies; 18+ messages in thread
From: Amir Goldstein @ 2017-12-28 12:12 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add filesystem check helpers for the upcoming fsck.overlay utility,
> and hook them to _check_test_fs and _check_scratch_fs. This helper
> works only if fsck.overlay exists.
>
> [ _check_test_fs/_check_scratch_fs part picked from Amir's patch, thanks ]
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
If this patch doesn't change in following posting, please add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
> common/config | 1 +
> common/overlay | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> common/rc | 4 +--
> 3 files changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/common/config b/common/config
> index d0fbfe5..0505b71 100644
> --- a/common/config
> +++ b/common/config
> @@ -236,6 +236,7 @@ case "$HOSTOS" in
> export MKFS_REISER4_PROG="`set_prog_path mkfs.reiser4`"
> export E2FSCK_PROG="`set_prog_path e2fsck`"
> export TUNE2FS_PROG="`set_prog_path tune2fs`"
> + export FSCK_OVERLAY_PROG="`set_prog_path fsck.overlay`"
> ;;
> esac
>
> diff --git a/common/overlay b/common/overlay
> index 1da4ab1..e7f0e62 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -151,3 +151,83 @@ _require_scratch_overlay_feature()
> _notrun "${FSTYP} feature '${feature}' cannot be enabled on ${SCRATCH_DEV}"
> _scratch_unmount
> }
> +
> +_overlay_fsck_dirs()
> +{
> + local lowerdir=$1
> + local upperdir=$2
> + local workdir=$3
> + local options=$4
> +
> + [[ ! -x "$FSCK_OVERLAY_PROG" ]] && return 0
> +
> + $FSCK_OVERLAY_PROG -o lowerdir=$lowerdir -o upperdir=$upperdir \
> + -o workdir=$workdir $options
> +}
> +
> +_overlay_check_dirs()
> +{
> + local lowerdir=$1
> + local upperdir=$2
> + local workdir=$3
> + local err=0
> +
> + _overlay_fsck_dirs $* $FSCK_OPTIONS >>$tmp.fsck 2>&1
> + if [ $? -ne 0 ]; then
> + _log_err "_overlay_check_fs: overlayfs on $lowerdir,$upperdir,$workdir is inconsistent"
> + echo "*** fsck.overlay output ***" >>$seqres.full
> + cat $tmp.fsck >>$seqres.full
> + echo "*** end fsck.overlay output" >>$seqres.full
> +
> + echo "*** mount output ***" >>$seqres.full
> + _mount >>$seqres.full
> + echo "*** end mount output" >>$seqres.full
> + err=1
> + fi
> + rm -f $tmp.fsck
> +
> + if [ $err != 0 ]; then
> + status=1
> + if [ "$iam" != "check" ]; then
> + exit 1
> + fi
> + return 1
> + fi
> +
> + return 0
> +}
> +
> +_overlay_check_fs()
> +{
> + local base_dev=$3
> + local base_mnt=$4
> +
> + [ "$FSTYP" = overlay ] || return 0
> +
> + # Base fs needs to be mounted to check overlay dirs
> + local mounted=""
> + [ -z "$base_dev" ] || mounted=`_is_mounted $base_dev`
> + [ -n "$mounted" ] || _overlay_base_mount $*
> +
> + # No need to umount overlay for dir checks with default -n option
> + _overlay_check_dirs $base_mnt/$OVL_LOWER $base_mnt/$OVL_UPPER \
> + $base_mnt/$OVL_WORK
> + local ret=$?
> +
> + [ -n "$mounted" ] || _overlay_base_unmount "$base_dev" "$base_mnt"
> + return $ret
> +}
> +
> +_check_overlay_test_fs()
> +{
> + _overlay_check_fs OVL_BASE_TEST_DEV OVL_BASE_TEST_DIR \
> + "$OVL_BASE_TEST_DEV" "$OVL_BASE_TEST_DIR" \
> + $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS
> +}
> +
> +_check_overlay_scratch_fs()
> +{
> + _overlay_check_fs OVL_BASE_SCRATCH_DEV OVL_BASE_SCRATCH_MNT \
> + "$OVL_BASE_SCRATCH_DEV" "$OVL_BASE_SCRATCH_MNT" \
> + $OVL_BASE_MOUNT_OPTIONS $SELINUX_MOUNT_OPTIONS
> +}
> diff --git a/common/rc b/common/rc
> index 4c053a5..79d5f25 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2507,7 +2507,7 @@ _check_test_fs()
> # no way to check consistency for GlusterFS
> ;;
> overlay)
> - # no way to check consistency for overlay
> + _check_overlay_test_fs
> ;;
> pvfs2)
> ;;
> @@ -2562,7 +2562,7 @@ _check_scratch_fs()
> # no way to check consistency for GlusterFS
> ;;
> overlay)
> - # no way to check consistency for overlay
> + _check_overlay_scratch_fs
> ;;
> pvfs2)
> ;;
> --
> 2.5.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test
2017-12-28 11:49 [PATCH xfstest v2 0/4] overlay: add fsck.overlay basic tests zhangyi (F)
2017-12-28 11:49 ` [PATCH xfstest v2 1/4] overlay: add filesystem check helper zhangyi (F)
@ 2017-12-28 11:49 ` zhangyi (F)
2017-12-28 12:11 ` Amir Goldstein
` (2 more replies)
2017-12-28 11:49 ` [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test zhangyi (F)
2017-12-28 11:49 ` [PATCH xfstest v2 4/4] overlay: add fsck.overlay impure xattr test zhangyi (F)
3 siblings, 3 replies; 18+ messages in thread
From: zhangyi (F) @ 2017-12-28 11:49 UTC (permalink / raw)
To: linux-unionfs, fstests
Cc: miklos, amir73il, eguan, yi.zhang, miaoxie, yangerkun
Add fsck.overlay test case to test it how to deal with orphan/valid
whiteouts in underlying directories of overlayfs.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
tests/overlay/201 | 232 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/overlay/201.out | 10 +++
tests/overlay/group | 1 +
3 files changed, 243 insertions(+)
create mode 100755 tests/overlay/201
create mode 100644 tests/overlay/201.out
diff --git a/tests/overlay/201 b/tests/overlay/201
new file mode 100755
index 0000000..6331b61
--- /dev/null
+++ b/tests/overlay/201
@@ -0,0 +1,232 @@
+#! /bin/bash
+# FS QA Test 201
+#
+# Test fsck.overlay how to deal with whiteouts in overlayfs.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Huawei. All Rights Reserved.
+# Author: zhangyi (F) <yi.zhang@huawei.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
+
+# remove all files from previous tests
+_scratch_mkfs
+
+OVL_REDIRECT_XATTR="trusted.overlay.redirect"
+OVL_OPAQUE_XATTR="trusted.overlay.opaque"
+OVL_OPAQUE_XATTR_VAL="y"
+
+# Check whiteout
+check_whiteout()
+{
+ local target=$1
+
+ target_type=`stat -c "%F:%t,%T" $target`
+
+ [[ $target_type == "character special file:0,0" ]] || \
+ echo "Valid whiteout removed incorrectly"
+}
+
+# Create a whiteout
+make_whiteout()
+{
+ local target=$1
+
+ mknod $target c 0 0
+}
+
+# Create an opaque directory
+make_opaque_dir()
+{
+ local target=$1
+
+ mkdir -p $target
+ $SETFATTR_PROG -n $OVL_OPAQUE_XATTR -v $OVL_OPAQUE_XATTR_VAL $target
+}
+
+# Create a redirect directory
+make_redirect_dir()
+{
+ local target=$1
+ local value=$2
+
+ mkdir -p $target
+ $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
+}
+
+# Create test directories
+lowerdir=$OVL_BASE_SCRATCH_MNT/lower
+lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
+upperdir=$OVL_BASE_SCRATCH_MNT/upper
+workdir=$OVL_BASE_SCRATCH_MNT/workdir
+
+mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
+
+# Test orphan whiteout in lower and upper layer, should remove
+echo "+ Orphan whiteout"
+make_whiteout $lowerdir/foo
+make_whiteout $upperdir/foo
+make_whiteout $upperdir/bar
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+ls $lowerdir
+ls $upperdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid whiteout covering lower target, should not remove
+echo "+ Valid whiteout"
+touch $lowerdir2/foo $lowerdir2/bar
+make_whiteout $upperdir/foo
+make_whiteout $lowerdir/bar
+
+_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
+ $seqres.full 2>&1 || _fail "fsck should not fail"
+check_whiteout $upperdir/foo
+check_whiteout $lowerdir/bar
+rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
+
+# Test orphan whiteout in opaque directory, should remove
+echo "+ Orphan whiteout(2)"
+mkdir $lowerdir/testdir
+touch $lowerdir/testdir/foo
+make_opaque_dir $upperdir/testdir
+make_whiteout $upperdir/testdir/foo
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+ls $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test orphan whiteout whose parent path is not an merged directory,
+# should remove
+echo "+ Orphan whiteout(3)"
+mkdir $lowerdir2/testdir1 $lowerdir2/testdir2 $lowerdir2/testdir3
+touch $lowerdir2/testdir1/foo $lowerdir2/testdir2/foo $lowerdir2/testdir3/foo
+mkdir $upperdir/testdir1 $upperdir/testdir2 $upperdir/testdir3
+touch $lowerdir/testdir1
+make_whiteout $lowerdir/testdir2
+make_opaque_dir $lowerdir/testdir3
+make_whiteout $upperdir/testdir1/foo
+make_whiteout $upperdir/testdir2/foo
+make_whiteout $upperdir/testdir3/foo
+
+_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
+ $seqres.full 2>&1 || _fail "fsck should not fail"
+ls $upperdir/testdir1
+ls $upperdir/testdir2
+ls $upperdir/testdir3
+rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
+
+# Test orphan whiteout in redirect directory, should remove
+echo "+ Orphan whiteout(4)"
+mkdir $lowerdir/testdir
+mkdir $lowerdir/origin
+touch $lowerdir/testdir/foo
+make_redirect_dir $upperdir/testdir "origin"
+make_whiteout $upperdir/testdir/foo
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+ls $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid whiteout in redirect directory cover file in lower
+# redirect origin directory, should not remove
+echo "+ Valid whiteout(2)"
+mkdir $lowerdir/origin
+touch $lowerdir/origin/foo
+make_redirect_dir $upperdir/testdir "origin"
+make_whiteout $upperdir/testdir/foo
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+check_whiteout $upperdir/testdir/foo
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid whiteout covering lower target whose parent directory
+# merge with a redirect directory in the middle layer, should not remove.
+echo "+ Valid whiteout(3)"
+mkdir -p $lowerdir2/origin/subdir
+touch $lowerdir2/origin/subdir/foo
+make_redirect_dir $lowerdir/testdir "origin"
+mkdir -p $upperdir/testdir/subdir
+make_whiteout $upperdir/testdir/subdir/foo
+
+_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p \
+ >> $seqres.full 2>&1 || _fail "fsck should not fail"
+check_whiteout $upperdir/testdir/subdir/foo
+rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
+
+# Test invalid whiteout in opaque subdirectory in a redirect directory,
+# should remove
+echo "+ Orphan whiteout(5)"
+mkdir -p $lowerdir/origin/subdir
+touch $lowerdir/origin/subdir/foo
+make_redirect_dir $upperdir/testdir "origin"
+make_opaque_dir $upperdir/testdir/subdir
+make_whiteout $upperdir/testdir/subdir/foo
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+ls $upperdir/testdir/subdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid whiteout in reidrect subdirectory in a opaque directory
+# covering lower target, should not remove
+echo "+ Valid whiteout(4)"
+mkdir $lowerdir/origin
+touch $lowerdir/origin/foo
+make_opaque_dir $upperdir/testdir
+make_redirect_dir $upperdir/testdir/subdir "/origin"
+make_whiteout $upperdir/testdir/subdir/foo
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+check_whiteout $upperdir/testdir/subdir/foo
+rm -rf $lowerdir/* $upperdir/*
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/201.out b/tests/overlay/201.out
new file mode 100644
index 0000000..157bb85
--- /dev/null
+++ b/tests/overlay/201.out
@@ -0,0 +1,10 @@
+QA output created by 201
++ Orphan whiteout
++ Valid whiteout
++ Orphan whiteout(2)
++ Orphan whiteout(3)
++ Orphan whiteout(4)
++ Valid whiteout(2)
++ Valid whiteout(3)
++ Orphan whiteout(5)
++ Valid whiteout(4)
diff --git a/tests/overlay/group b/tests/overlay/group
index 7e541e4..7c5fcbb 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -49,3 +49,4 @@
044 auto quick copyup hardlink nonsamefs
047 auto quick copyup hardlink
048 auto quick copyup hardlink
+201 auto quick fsck
--
2.5.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test
2017-12-28 11:49 ` [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test zhangyi (F)
@ 2017-12-28 12:11 ` Amir Goldstein
2017-12-29 1:27 ` zhangyi (F)
2018-01-02 20:05 ` Vivek Goyal
2018-01-03 15:04 ` Vivek Goyal
2 siblings, 1 reply; 18+ messages in thread
From: Amir Goldstein @ 2017-12-28 12:11 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add fsck.overlay test case to test it how to deal with orphan/valid
> whiteouts in underlying directories of overlayfs.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Very nice!
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
minor suggestions below
> ---
> tests/overlay/201 | 232 ++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/overlay/201.out | 10 +++
> tests/overlay/group | 1 +
> 3 files changed, 243 insertions(+)
> create mode 100755 tests/overlay/201
> create mode 100644 tests/overlay/201.out
>
> diff --git a/tests/overlay/201 b/tests/overlay/201
> new file mode 100755
> index 0000000..6331b61
> --- /dev/null
> +++ b/tests/overlay/201
> @@ -0,0 +1,232 @@
> +#! /bin/bash
> +# FS QA Test 201
> +#
> +# Test fsck.overlay how to deal with whiteouts in overlayfs.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Huawei. All Rights Reserved.
> +# Author: zhangyi (F) <yi.zhang@huawei.com>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs overlay
> +_supported_os Linux
> +_require_scratch
> +_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
> +
> +# remove all files from previous tests
> +_scratch_mkfs
> +
> +OVL_REDIRECT_XATTR="trusted.overlay.redirect"
> +OVL_OPAQUE_XATTR="trusted.overlay.opaque"
> +OVL_OPAQUE_XATTR_VAL="y"
> +
> +# Check whiteout
> +check_whiteout()
> +{
> + local target=$1
> +
> + target_type=`stat -c "%F:%t,%T" $target`
> +
> + [[ $target_type == "character special file:0,0" ]] || \
> + echo "Valid whiteout removed incorrectly"
> +}
> +
> +# Create a whiteout
> +make_whiteout()
> +{
> + local target=$1
> +
> + mknod $target c 0 0
> +}
> +
> +# Create an opaque directory
> +make_opaque_dir()
> +{
> + local target=$1
> +
> + mkdir -p $target
> + $SETFATTR_PROG -n $OVL_OPAQUE_XATTR -v $OVL_OPAQUE_XATTR_VAL $target
> +}
> +
> +# Create a redirect directory
> +make_redirect_dir()
> +{
> + local target=$1
> + local value=$2
> +
> + mkdir -p $target
> + $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
> +}
> +
> +# Create test directories
> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
> +
> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
Would be nice to pack these mkdir after rm -rf as make_dirs helper
to run before every test case, instead of mkdir once and rm -rf after each test.
> +
> +# Test orphan whiteout in lower and upper layer, should remove
> +echo "+ Orphan whiteout"
> +make_whiteout $lowerdir/foo
> +make_whiteout $upperdir/foo
> +make_whiteout $upperdir/bar
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +ls $lowerdir
> +ls $upperdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid whiteout covering lower target, should not remove
> +echo "+ Valid whiteout"
> +touch $lowerdir2/foo $lowerdir2/bar
> +make_whiteout $upperdir/foo
> +make_whiteout $lowerdir/bar
> +
> +_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
> + $seqres.full 2>&1 || _fail "fsck should not fail"
> +check_whiteout $upperdir/foo
> +check_whiteout $lowerdir/bar
> +rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
> +
> +# Test orphan whiteout in opaque directory, should remove
> +echo "+ Orphan whiteout(2)"
> +mkdir $lowerdir/testdir
> +touch $lowerdir/testdir/foo
> +make_opaque_dir $upperdir/testdir
> +make_whiteout $upperdir/testdir/foo
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +ls $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test orphan whiteout whose parent path is not an merged directory,
> +# should remove
> +echo "+ Orphan whiteout(3)"
> +mkdir $lowerdir2/testdir1 $lowerdir2/testdir2 $lowerdir2/testdir3
> +touch $lowerdir2/testdir1/foo $lowerdir2/testdir2/foo $lowerdir2/testdir3/foo
> +mkdir $upperdir/testdir1 $upperdir/testdir2 $upperdir/testdir3
How about mkdir $upperdir/testdir4 (case of parent is a pure upper dir)
> +touch $lowerdir/testdir1
> +make_whiteout $lowerdir/testdir2
> +make_opaque_dir $lowerdir/testdir3
> +make_whiteout $upperdir/testdir1/foo
> +make_whiteout $upperdir/testdir2/foo
> +make_whiteout $upperdir/testdir3/foo
> +
> +_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p >> \
> + $seqres.full 2>&1 || _fail "fsck should not fail"
> +ls $upperdir/testdir1
> +ls $upperdir/testdir2
> +ls $upperdir/testdir3
> +rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
> +
> +# Test orphan whiteout in redirect directory, should remove
> +echo "+ Orphan whiteout(4)"
> +mkdir $lowerdir/testdir
> +mkdir $lowerdir/origin
> +touch $lowerdir/testdir/foo
> +make_redirect_dir $upperdir/testdir "origin"
> +make_whiteout $upperdir/testdir/foo
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +ls $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid whiteout in redirect directory cover file in lower
> +# redirect origin directory, should not remove
> +echo "+ Valid whiteout(2)"
> +mkdir $lowerdir/origin
> +touch $lowerdir/origin/foo
> +make_redirect_dir $upperdir/testdir "origin"
> +make_whiteout $upperdir/testdir/foo
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +check_whiteout $upperdir/testdir/foo
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid whiteout covering lower target whose parent directory
> +# merge with a redirect directory in the middle layer, should not remove.
> +echo "+ Valid whiteout(3)"
> +mkdir -p $lowerdir2/origin/subdir
> +touch $lowerdir2/origin/subdir/foo
> +make_redirect_dir $lowerdir/testdir "origin"
> +mkdir -p $upperdir/testdir/subdir
> +make_whiteout $upperdir/testdir/subdir/foo
> +
> +_overlay_fsck_dirs "$lowerdir:$lowerdir2" $upperdir $workdir -p \
> + >> $seqres.full 2>&1 || _fail "fsck should not fail"
> +check_whiteout $upperdir/testdir/subdir/foo
> +rm -rf $lowerdir/* $lowerdir2/* $upperdir/*
> +
> +# Test invalid whiteout in opaque subdirectory in a redirect directory,
> +# should remove
> +echo "+ Orphan whiteout(5)"
> +mkdir -p $lowerdir/origin/subdir
> +touch $lowerdir/origin/subdir/foo
> +make_redirect_dir $upperdir/testdir "origin"
> +make_opaque_dir $upperdir/testdir/subdir
> +make_whiteout $upperdir/testdir/subdir/foo
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +ls $upperdir/testdir/subdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid whiteout in reidrect subdirectory in a opaque directory
> +# covering lower target, should not remove
> +echo "+ Valid whiteout(4)"
> +mkdir $lowerdir/origin
> +touch $lowerdir/origin/foo
> +make_opaque_dir $upperdir/testdir
> +make_redirect_dir $upperdir/testdir/subdir "/origin"
> +make_whiteout $upperdir/testdir/subdir/foo
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +check_whiteout $upperdir/testdir/subdir/foo
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/overlay/201.out b/tests/overlay/201.out
> new file mode 100644
> index 0000000..157bb85
> --- /dev/null
> +++ b/tests/overlay/201.out
> @@ -0,0 +1,10 @@
> +QA output created by 201
> ++ Orphan whiteout
> ++ Valid whiteout
> ++ Orphan whiteout(2)
> ++ Orphan whiteout(3)
> ++ Orphan whiteout(4)
> ++ Valid whiteout(2)
> ++ Valid whiteout(3)
> ++ Orphan whiteout(5)
> ++ Valid whiteout(4)
> diff --git a/tests/overlay/group b/tests/overlay/group
> index 7e541e4..7c5fcbb 100644
> --- a/tests/overlay/group
> +++ b/tests/overlay/group
> @@ -49,3 +49,4 @@
> 044 auto quick copyup hardlink nonsamefs
> 047 auto quick copyup hardlink
> 048 auto quick copyup hardlink
> +201 auto quick fsck
> --
> 2.5.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test
2017-12-28 12:11 ` Amir Goldstein
@ 2017-12-29 1:27 ` zhangyi (F)
0 siblings, 0 replies; 18+ messages in thread
From: zhangyi (F) @ 2017-12-29 1:27 UTC (permalink / raw)
To: Amir Goldstein
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On 2017/12/28 20:11, Amir Goldstein Wrote:
> On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> Add fsck.overlay test case to test it how to deal with orphan/valid
>> whiteouts in underlying directories of overlayfs.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>
> Very nice!
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>
> minor suggestions below
>
Thanks a lot for your suggestions!
[..]
>> +# Create test directories
>> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
>> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
>> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
>> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
>> +
>> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
>
> Would be nice to pack these mkdir after rm -rf as make_dirs helper
> to run before every test case, instead of mkdir once and rm -rf after each test.
>
It looks better, will change it for all 3 test cases.
[..]
>> +# Test orphan whiteout whose parent path is not an merged directory,
>> +# should remove
>> +echo "+ Orphan whiteout(3)"
>> +mkdir $lowerdir2/testdir1 $lowerdir2/testdir2 $lowerdir2/testdir3
>> +touch $lowerdir2/testdir1/foo $lowerdir2/testdir2/foo $lowerdir2/testdir3/foo
>> +mkdir $upperdir/testdir1 $upperdir/testdir2 $upperdir/testdir3
>
> How about mkdir $upperdir/testdir4 (case of parent is a pure upper dir)
>
Yes, it's perfect, will add.
Thanks,
Yi.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test
2017-12-28 11:49 ` [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test zhangyi (F)
2017-12-28 12:11 ` Amir Goldstein
@ 2018-01-02 20:05 ` Vivek Goyal
2018-01-03 1:02 ` zhangyi (F)
2018-01-03 15:04 ` Vivek Goyal
2 siblings, 1 reply; 18+ messages in thread
From: Vivek Goyal @ 2018-01-02 20:05 UTC (permalink / raw)
To: zhangyi (F)
Cc: linux-unionfs, fstests, miklos, amir73il, eguan, miaoxie,
yangerkun
On Thu, Dec 28, 2017 at 07:49:31PM +0800, zhangyi (F) wrote:
[..]
> +# Create test directories
> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
> +
> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
Minor nit. Why $workdir has been specified twice. I saw it in another test
as well.
Vivek
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test
2018-01-02 20:05 ` Vivek Goyal
@ 2018-01-03 1:02 ` zhangyi (F)
0 siblings, 0 replies; 18+ messages in thread
From: zhangyi (F) @ 2018-01-03 1:02 UTC (permalink / raw)
To: Vivek Goyal
Cc: linux-unionfs, fstests, miklos, amir73il, eguan, miaoxie,
yangerkun
On 2018/1/3 4:05, Vivek Goyal Wrote:
> On Thu, Dec 28, 2017 at 07:49:31PM +0800, zhangyi (F) wrote:
>
> [..]
>> +# Create test directories
>> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
>> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
>> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
>> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
>> +
>> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
>
> Minor nit. Why $workdir has been specified twice. I saw it in another test
> as well.
>
Will fix, thanks!
zhangyi.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test
2017-12-28 11:49 ` [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test zhangyi (F)
2017-12-28 12:11 ` Amir Goldstein
2018-01-02 20:05 ` Vivek Goyal
@ 2018-01-03 15:04 ` Vivek Goyal
2018-01-04 1:15 ` zhangyi (F)
2 siblings, 1 reply; 18+ messages in thread
From: Vivek Goyal @ 2018-01-03 15:04 UTC (permalink / raw)
To: zhangyi (F)
Cc: linux-unionfs, fstests, miklos, amir73il, eguan, miaoxie,
yangerkun
On Thu, Dec 28, 2017 at 07:49:31PM +0800, zhangyi (F) wrote:
[..]
> +# Test orphan whiteout in lower and upper layer, should remove
> +echo "+ Orphan whiteout"
> +make_whiteout $lowerdir/foo
> +make_whiteout $upperdir/foo
> +make_whiteout $upperdir/bar
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
I ran fsck.overlay manually (V3 patches) with one orphan whiteout in
upper. I get following output.
fsck.overlay -o lowerdir=lower,upperdir=upper,workdir=work
Orphan whiteout: /root/fsck-overlay-testing/upper/foo Remove ? [y]:
y
fsck.overlay:[Error]: Cannot getxattr /root/fsck-overlay-testing/upper/foo trusted.overlay.origin: No such file or directory
Filesystem clean
This message about "overlay.origin" not being there, should not be an
error. You have already figured out, its an orphan whiteout, and user
already asked you to remove it. So checking or origin after that can
calling it Error might not make much sense. Also on top, later I get
"Filesystem clean".
I think in this case, user should not see any message until and unless a
error actually has happened which prevents from cleaning orphan whiteout.
Vivek
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test
2018-01-03 15:04 ` Vivek Goyal
@ 2018-01-04 1:15 ` zhangyi (F)
0 siblings, 0 replies; 18+ messages in thread
From: zhangyi (F) @ 2018-01-04 1:15 UTC (permalink / raw)
To: Vivek Goyal
Cc: linux-unionfs, fstests, miklos, amir73il, eguan, miaoxie,
yangerkun
On 2018/1/3 23:04, Vivek Goyal Wrote:
> On Thu, Dec 28, 2017 at 07:49:31PM +0800, zhangyi (F) wrote:
>
> [..]
>> +# Test orphan whiteout in lower and upper layer, should remove
>> +echo "+ Orphan whiteout"
>> +make_whiteout $lowerdir/foo
>> +make_whiteout $upperdir/foo
>> +make_whiteout $upperdir/bar
>> +
>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
>> + _fail "fsck should not fail"
>
> I ran fsck.overlay manually (V3 patches) with one orphan whiteout in
> upper. I get following output.
>
> fsck.overlay -o lowerdir=lower,upperdir=upper,workdir=work
> Orphan whiteout: /root/fsck-overlay-testing/upper/foo Remove ? [y]:
> y
> fsck.overlay:[Error]: Cannot getxattr /root/fsck-overlay-testing/upper/foo trusted.overlay.origin: No such file or directory
> Filesystem clean
>
> This message about "overlay.origin" not being there, should not be an
> error. You have already figured out, its an orphan whiteout, and user
> already asked you to remove it. So checking or origin after that can
> calling it Error might not make much sense. Also on top, later I get
> "Filesystem clean".
>
> I think in this case, user should not see any message until and unless a
> error actually has happened which prevents from cleaning orphan whiteout.
>
Oh, this is a bug introduced by "fsck.overlay: add impure xattr check", thanks
for testing and pointing this out. I will fix in nest version.
Thanks,
zhangyi.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test
2017-12-28 11:49 [PATCH xfstest v2 0/4] overlay: add fsck.overlay basic tests zhangyi (F)
2017-12-28 11:49 ` [PATCH xfstest v2 1/4] overlay: add filesystem check helper zhangyi (F)
2017-12-28 11:49 ` [PATCH xfstest v2 2/4] overlay: add fsck.overlay whiteout test zhangyi (F)
@ 2017-12-28 11:49 ` zhangyi (F)
2017-12-28 12:37 ` Amir Goldstein
2017-12-28 11:49 ` [PATCH xfstest v2 4/4] overlay: add fsck.overlay impure xattr test zhangyi (F)
3 siblings, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-28 11:49 UTC (permalink / raw)
To: linux-unionfs, fstests
Cc: miklos, amir73il, eguan, yi.zhang, miaoxie, yangerkun
Add fsck.overlay test case to test it how to deal with invalid/valid/
duplicate redirect xattr in underlying directories of overlayfs.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
tests/overlay/202 | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/overlay/202.out | 10 +++
tests/overlay/group | 1 +
3 files changed, 245 insertions(+)
create mode 100755 tests/overlay/202
create mode 100644 tests/overlay/202.out
diff --git a/tests/overlay/202 b/tests/overlay/202
new file mode 100755
index 0000000..91bff47
--- /dev/null
+++ b/tests/overlay/202
@@ -0,0 +1,234 @@
+#! /bin/bash
+# FS QA Test 202
+#
+# Test fsck.overlay how to deal with redirect xattr in overlayfs.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Huawei. All Rights Reserved.
+# Author: zhangyi (F) <yi.zhang@huawei.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+_require_attrs
+_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
+
+# remove all files from previous tests
+_scratch_mkfs
+
+OVL_REDIRECT_XATTR="trusted.overlay.redirect"
+OVL_OPAQUE_XATTR="trusted.overlay.opaque"
+OVL_OPAQUE_XATTR_VAL="y"
+
+# Create a redirect directory
+make_redirect_dir()
+{
+ local target=$1
+ local value=$2
+
+ mkdir -p $target
+ $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
+}
+
+# Check redirect xattr
+check_redirect()
+{
+ local expect=$1
+ local target=$2
+
+ value=$($GETFATTR_PROG --absolute-names --only-values \
+ -n $OVL_REDIRECT_XATTR $target)
+
+ [[ $value == $expect ]] || echo "Redirect xattr incorrect"
+}
+
+# Check opaque xattr
+check_opaque()
+{
+ local target=$1
+
+ value=$($GETFATTR_PROG --absolute-names --only-values \
+ -n $OVL_OPAQUE_XATTR $target)
+
+ [[ $value == $OVL_OPAQUE_XATTR_VAL ]] || echo "Opaque xattr incorrect"
+}
+
+# Check whiteout
+check_whiteout()
+{
+ local target=$1
+
+ target_type=`stat -c "%F:%t,%T" $target`
+
+ [[ $target_type == "character special file:0,0" ]] || \
+ echo "Whiteout missing"
+}
+
+# Create test directories
+lowerdir=$OVL_BASE_SCRATCH_MNT/lower
+lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
+upperdir=$OVL_BASE_SCRATCH_MNT/upper
+workdir=$OVL_BASE_SCRATCH_MNT/workdir
+
+mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
+
+# Test invalid redirect xattr point to a nonexistent origin, should remove
+echo "+ Invalid redirect"
+make_redirect_dir $upperdir/testdir "abc"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
+rm -rf $upperdir/*
+
+# Test invalid redirect xattr point to a file origin, should remove
+echo "+ Invalid redirect(2)"
+touch $lowerdir/origin
+make_redirect_dir $upperdir/testdir "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid redirect xattr point to a directory origin in the same directory,
+# should not remove
+echo "+ Valid redirect"
+mkdir $lowerdir/origin
+mknod $upperdir/origin c 0 0
+make_redirect_dir $upperdir/testdir "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+check_redirect "origin" $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid redirect xattr point to a directory origin in different directories
+# should not remove
+echo "+ Valid redirect(2)"
+mkdir $lowerdir/origin
+mknod $upperdir/origin c 0 0
+make_redirect_dir $upperdir/testdir1/testdir2 "/origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+check_redirect "/origin" $upperdir/testdir1/testdir2
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid redirect xattr but missing whiteout to cover lower target,
+# should fix whiteout
+echo "+ Missing whiteout"
+mkdir $lowerdir/origin
+make_redirect_dir $upperdir/testdir "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+check_redirect "origin" $upperdir/testdir
+check_whiteout $upperdir/origin
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid redirect xattrs exchanged by rename, should not remove
+echo "+ Valid redirect(3)"
+mkdir $lowerdir/testdir1 $lowerdir/testdir2
+make_redirect_dir $upperdir/testdir1 "testdir2"
+make_redirect_dir $upperdir/testdir2 "testdir1"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+check_redirect "testdir2" $upperdir/testdir1
+check_redirect "testdir1" $upperdir/testdir2
+rm -rf $lowerdir/* $upperdir/*
+
+# Test valid redirect xattr with general directory cover lower origin
+# target, should ask user this directory is merged or not by default
+# and do nothing in auto mode
+echo "+ Valid redirect(4)"
+mkdir $lowerdir/origin
+mkdir $upperdir/origin
+make_redirect_dir $upperdir/testdir "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+check_redirect "origin" $upperdir/testdir
+$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR $upperdir/testdir
+
+echo "n" > $tmp.input
+_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+check_redirect "origin" $upperdir/testdir
+check_opaque $upperdir/origin
+rm -rf $lowerdir/* $upperdir/*
+
+# Test invalid redirect xattr with lower same name directory exists,
+# should remove invalid xattr, and ask user directory merge or not
+# by default
+echo "+ Invalid redirect(3)"
+mkdir $lowerdir/testdir
+make_redirect_dir $upperdir/testdir "origin"
+
+echo "y" > $tmp.input
+echo "n" >> $tmp.input
+_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
+check_opaque $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test duplicate redirect directories point to one origin, should fail in
+# auto mode, should remove the duplicate one in yes mode
+echo "+ Duplicate redirect"
+mkdir $lowerdir/origin
+mknod $upperdir/origin c 0 0
+make_redirect_dir $upperdir/testdir1 "origin"
+make_redirect_dir $upperdir/testdir2 "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 && \
+ _fail "fsck should fail"
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -y >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+redirect_1=`check_redirect "origin" $upperdir/testdir1 2>/dev/null`
+redirect_2=`check_redirect "origin" $upperdir/testdir2 2>/dev/null`
+[[ $redirect_1 == $redirect_2 ]] && echo "Redirect xattr incorrect"
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/202.out b/tests/overlay/202.out
new file mode 100644
index 0000000..ae50d96
--- /dev/null
+++ b/tests/overlay/202.out
@@ -0,0 +1,10 @@
+QA output created by 202
++ Invalid redirect
++ Invalid redirect(2)
++ Valid redirect
++ Valid redirect(2)
++ Missing whiteout
++ Valid redirect(3)
++ Valid redirect(4)
++ Invalid redirect(3)
++ Duplicate redirect
diff --git a/tests/overlay/group b/tests/overlay/group
index 7c5fcbb..e39b5e0 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -50,3 +50,4 @@
047 auto quick copyup hardlink
048 auto quick copyup hardlink
201 auto quick fsck
+202 auto quick fsck
--
2.5.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test
2017-12-28 11:49 ` [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test zhangyi (F)
@ 2017-12-28 12:37 ` Amir Goldstein
2017-12-29 1:49 ` zhangyi (F)
2017-12-30 4:06 ` zhangyi (F)
0 siblings, 2 replies; 18+ messages in thread
From: Amir Goldstein @ 2017-12-28 12:37 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add fsck.overlay test case to test it how to deal with invalid/valid/
> duplicate redirect xattr in underlying directories of overlayfs.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Nice. See suggestions to improve.
> ---
> tests/overlay/202 | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/overlay/202.out | 10 +++
> tests/overlay/group | 1 +
> 3 files changed, 245 insertions(+)
> create mode 100755 tests/overlay/202
> create mode 100644 tests/overlay/202.out
>
> diff --git a/tests/overlay/202 b/tests/overlay/202
> new file mode 100755
> index 0000000..91bff47
> --- /dev/null
> +++ b/tests/overlay/202
> @@ -0,0 +1,234 @@
> +#! /bin/bash
> +# FS QA Test 202
> +#
> +# Test fsck.overlay how to deal with redirect xattr in overlayfs.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Huawei. All Rights Reserved.
> +# Author: zhangyi (F) <yi.zhang@huawei.com>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/attr
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs overlay
> +_supported_os Linux
> +_require_scratch
> +_require_attrs
> +_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
> +
> +# remove all files from previous tests
> +_scratch_mkfs
> +
> +OVL_REDIRECT_XATTR="trusted.overlay.redirect"
> +OVL_OPAQUE_XATTR="trusted.overlay.opaque"
> +OVL_OPAQUE_XATTR_VAL="y"
> +
> +# Create a redirect directory
> +make_redirect_dir()
> +{
> + local target=$1
> + local value=$2
> +
> + mkdir -p $target
> + $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
> +}
> +
> +# Check redirect xattr
> +check_redirect()
> +{
> + local expect=$1
> + local target=$2
Suggest to flip target/expect order to match target/value order in
make_redirect_dir
Then for the tests that expect empty redirect call check_redirect with
empty expect,
e.g. check_redirect $upperdir/testdir ""
of course "" is not strictly needed, but may be more readable
Even better use helper check_not_redirect() to do that
> +
> + value=$($GETFATTR_PROG --absolute-names --only-values \
> + -n $OVL_REDIRECT_XATTR $target)
> +
> + [[ $value == $expect ]] || echo "Redirect xattr incorrect"
> +}
> +
> +# Check opaque xattr
> +check_opaque()
> +{
> + local target=$1
Here too, I suggest:
local expect=${2-$OVL_OPAQUE_XATTR_VAL}
And then tests can either call check_opaque $target "" or add a helper
check_not_opaque() to do that
> +
> + value=$($GETFATTR_PROG --absolute-names --only-values \
> + -n $OVL_OPAQUE_XATTR $target)
> +
> + [[ $value == $OVL_OPAQUE_XATTR_VAL ]] || echo "Opaque xattr incorrect"
> +}
> +
> +# Check whiteout
> +check_whiteout()
> +{
> + local target=$1
> +
> + target_type=`stat -c "%F:%t,%T" $target`
> +
> + [[ $target_type == "character special file:0,0" ]] || \
> + echo "Whiteout missing"
> +}
> +
> +# Create test directories
> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
> +
> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
Again, suggest make_dirs helper with rm -rf before and drop
all individual rm -rf after tests.
> +
> +# Test invalid redirect xattr point to a nonexistent origin, should remove
> +echo "+ Invalid redirect"
> +make_redirect_dir $upperdir/testdir "abc"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
> +rm -rf $upperdir/*
> +
> +# Test invalid redirect xattr point to a file origin, should remove
> +echo "+ Invalid redirect(2)"
> +touch $lowerdir/origin
> +make_redirect_dir $upperdir/testdir "origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid redirect xattr point to a directory origin in the same directory,
> +# should not remove
> +echo "+ Valid redirect"
> +mkdir $lowerdir/origin
> +mknod $upperdir/origin c 0 0
> +make_redirect_dir $upperdir/testdir "origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +check_redirect "origin" $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid redirect xattr point to a directory origin in different directories
> +# should not remove
> +echo "+ Valid redirect(2)"
> +mkdir $lowerdir/origin
> +mknod $upperdir/origin c 0 0
> +make_redirect_dir $upperdir/testdir1/testdir2 "/origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +check_redirect "/origin" $upperdir/testdir1/testdir2
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid redirect xattr but missing whiteout to cover lower target,
> +# should fix whiteout
> +echo "+ Missing whiteout"
> +mkdir $lowerdir/origin
> +make_redirect_dir $upperdir/testdir "origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +check_redirect "origin" $upperdir/testdir
> +check_whiteout $upperdir/origin
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid redirect xattrs exchanged by rename, should not remove
> +echo "+ Valid redirect(3)"
> +mkdir $lowerdir/testdir1 $lowerdir/testdir2
> +make_redirect_dir $upperdir/testdir1 "testdir2"
> +make_redirect_dir $upperdir/testdir2 "testdir1"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +check_redirect "testdir2" $upperdir/testdir1
> +check_redirect "testdir1" $upperdir/testdir2
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test valid redirect xattr with general directory cover lower origin
> +# target, should ask user this directory is merged or not by default
> +# and do nothing in auto mode
> +echo "+ Valid redirect(4)"
> +mkdir $lowerdir/origin
> +mkdir $upperdir/origin
> +make_redirect_dir $upperdir/testdir "origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +check_redirect "origin" $upperdir/testdir
> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR $upperdir/testdir
> +
> +echo "n" > $tmp.input
This approach seems a bit fragile to me.
Maybe you will want to add some questions in the future.
To me it makes better sense to check behavior of fsck -y is automated test,
because some user who does not understand the questions is surely going
to run fsck -y to fix problems.
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +check_redirect "origin" $upperdir/testdir
> +check_opaque $upperdir/origin
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test invalid redirect xattr with lower same name directory exists,
> +# should remove invalid xattr, and ask user directory merge or not
> +# by default
> +echo "+ Invalid redirect(3)"
> +mkdir $lowerdir/testdir
> +make_redirect_dir $upperdir/testdir "origin"
> +
> +echo "y" > $tmp.input
> +echo "n" >> $tmp.input
Same here. suggest to validate results of fsck -y.
fsck -y to user means: I don't care how fix it, just fix it!
If user does not care, fsck can have its own arbitrary selections,
but they should be consistent, so we can test them.
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +$GETFATTR_PROG --absolute-names -d -m $OVL_REDIRECT_XATTR $upperdir/testdir
> +check_opaque $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test duplicate redirect directories point to one origin, should fail in
> +# auto mode, should remove the duplicate one in yes mode
> +echo "+ Duplicate redirect"
> +mkdir $lowerdir/origin
> +mknod $upperdir/origin c 0 0
> +make_redirect_dir $upperdir/testdir1 "origin"
> +make_redirect_dir $upperdir/testdir2 "origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 && \
> + _fail "fsck should fail"
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -y >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +redirect_1=`check_redirect "origin" $upperdir/testdir1 2>/dev/null`
> +redirect_2=`check_redirect "origin" $upperdir/testdir2 2>/dev/null`
> +[[ $redirect_1 == $redirect_2 ]] && echo "Redirect xattr incorrect"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/overlay/202.out b/tests/overlay/202.out
> new file mode 100644
> index 0000000..ae50d96
> --- /dev/null
> +++ b/tests/overlay/202.out
> @@ -0,0 +1,10 @@
> +QA output created by 202
> ++ Invalid redirect
> ++ Invalid redirect(2)
> ++ Valid redirect
> ++ Valid redirect(2)
> ++ Missing whiteout
> ++ Valid redirect(3)
> ++ Valid redirect(4)
> ++ Invalid redirect(3)
> ++ Duplicate redirect
> diff --git a/tests/overlay/group b/tests/overlay/group
> index 7c5fcbb..e39b5e0 100644
> --- a/tests/overlay/group
> +++ b/tests/overlay/group
> @@ -50,3 +50,4 @@
> 047 auto quick copyup hardlink
> 048 auto quick copyup hardlink
> 201 auto quick fsck
> +202 auto quick fsck
> --
> 2.5.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test
2017-12-28 12:37 ` Amir Goldstein
@ 2017-12-29 1:49 ` zhangyi (F)
2017-12-30 4:06 ` zhangyi (F)
1 sibling, 0 replies; 18+ messages in thread
From: zhangyi (F) @ 2017-12-29 1:49 UTC (permalink / raw)
To: Amir Goldstein
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On 2017/12/28 20:37, Amir Goldstein Wrote:
> On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> Add fsck.overlay test case to test it how to deal with invalid/valid/
>> duplicate redirect xattr in underlying directories of overlayfs.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>
> Nice. See suggestions to improve.
Thanks for your suggestions!
[..]
>> +# Check redirect xattr
>> +check_redirect()
>> +{
>> + local expect=$1
>> + local target=$2
>
> Suggest to flip target/expect order to match target/value order in
> make_redirect_dir
> Then for the tests that expect empty redirect call check_redirect with
> empty expect,
> e.g. check_redirect $upperdir/testdir ""
> of course "" is not strictly needed, but may be more readable
> Even better use helper check_not_redirect() to do that
I prefer check_not_redirect(), will change it.
>> +
>> + value=$($GETFATTR_PROG --absolute-names --only-values \
>> + -n $OVL_REDIRECT_XATTR $target)
>> +
>> + [[ $value == $expect ]] || echo "Redirect xattr incorrect"
>> +}
>> +
>> +# Check opaque xattr
>> +check_opaque()
>> +{
>> + local target=$1
>
> Here too, I suggest:
> local expect=${2-$OVL_OPAQUE_XATTR_VAL}
>
> And then tests can either call check_opaque $target "" or add a helper
> check_not_opaque() to do that
>
[..]
>> +# Create test directories
>> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
>> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
>> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
>> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
>> +
>> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
>
> Again, suggest make_dirs helper with rm -rf before and drop
> all individual rm -rf after tests.
>
Will change.
[..]
>> +# Test valid redirect xattr with general directory cover lower origin
>> +# target, should ask user this directory is merged or not by default
>> +# and do nothing in auto mode
>> +echo "+ Valid redirect(4)"
>> +mkdir $lowerdir/origin
>> +mkdir $upperdir/origin
>> +make_redirect_dir $upperdir/testdir "origin"
>> +
>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
>> + _fail "fsck should not fail"
>> +check_redirect "origin" $upperdir/testdir
>> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR $upperdir/testdir
>> +
>> +echo "n" > $tmp.input
>
> This approach seems a bit fragile to me.
> Maybe you will want to add some questions in the future.
> To me it makes better sense to check behavior of fsck -y is automated test,
> because some user who does not understand the questions is surely going
> to run fsck -y to fix problems.
>
Yes, for ordinary users, they are more likely to use -p or -y, this input
will also increase workload in future, is not really necessary, will remove.
>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
>> + _fail "fsck should not fail"
>> +check_redirect "origin" $upperdir/testdir
>> +check_opaque $upperdir/origin
>> +rm -rf $lowerdir/* $upperdir/*
>> +
>> +# Test invalid redirect xattr with lower same name directory exists,
>> +# should remove invalid xattr, and ask user directory merge or not
>> +# by default
>> +echo "+ Invalid redirect(3)"
>> +mkdir $lowerdir/testdir
>> +make_redirect_dir $upperdir/testdir "origin"
>> +
>> +echo "y" > $tmp.input
>> +echo "n" >> $tmp.input
>
> Same here. suggest to validate results of fsck -y.
> fsck -y to user means: I don't care how fix it, just fix it!
> If user does not care, fsck can have its own arbitrary selections,
> but they should be consistent, so we can test them.
>
Will remove, too.
Thanks,
Yi.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test
2017-12-28 12:37 ` Amir Goldstein
2017-12-29 1:49 ` zhangyi (F)
@ 2017-12-30 4:06 ` zhangyi (F)
2017-12-30 9:35 ` Amir Goldstein
1 sibling, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-30 4:06 UTC (permalink / raw)
To: Amir Goldstein, zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On 2017/12/28 20:37, Amir Goldstein Wrote:
>
>> +# Test valid redirect xattr with general directory cover lower origin
>> +# target, should ask user this directory is merged or not by default
>> +# and do nothing in auto mode
>> +echo "+ Valid redirect(4)"
>> +mkdir $lowerdir/origin
>> +mkdir $upperdir/origin
>> +make_redirect_dir $upperdir/testdir "origin"
>> +
>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
>> + _fail "fsck should not fail"
>> +check_redirect "origin" $upperdir/testdir
>> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR $upperdir/testdir
>> +
>> +echo "n" > $tmp.input
> This approach seems a bit fragile to me.
> Maybe you will want to add some questions in the future.
> To me it makes better sense to check behavior of fsck -y is automated test,
> because some user who does not understand the questions is surely going
> to run fsck -y to fix problems.
>
>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >> $seqres.full 2>&1 || \
>> + _fail "fsck should not fail"
>> +check_redirect "origin" $upperdir/testdir
>> +check_opaque $upperdir/origin
>> +rm -rf $lowerdir/* $upperdir/*
>> +
Hi Amir：
Think about this again. IIUC, I think there is a consistency problem
about this case.
If we allow user to create a new directory in upper layer merge with
lower redirect origin target when overlay offline, we will see duplicate
d_ino in overlay filesystem after next mount, becasue current
ovl_getattr() will get the lower d_ino for both merged dir and redirect dir.
eg:
mnt: dira(ino=x) dirb(ino=x)
upper: *dira(ino=y) dirb(ino=z, redirect=dirb)
lower: dira(ino=x)
*) User remove whiteout or opaque dir and then newly created when
overlayfs is offline.
Furthermore, if user rename dira again, will lead to duplicate redirect
xattr directly.
So I think we should forbid user do this and check by fsck.overlay. Or
handle by overlay filesytem? thought?
Thanks，
Yi.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test
2017-12-30 4:06 ` zhangyi (F)
@ 2017-12-30 9:35 ` Amir Goldstein
2017-12-31 5:06 ` zhangyi
0 siblings, 1 reply; 18+ messages in thread
From: Amir Goldstein @ 2017-12-30 9:35 UTC (permalink / raw)
To: zhangyi (F)
Cc: zhangyi (F), overlayfs, fstests, Miklos Szeredi, Eryu Guan,
Miao Xie, yangerkun
On Sat, Dec 30, 2017 at 6:06 AM, zhangyi (F) <yizhang089@gmail.com> wrote:
> On 2017/12/28 20:37, Amir Goldstein Wrote:
>
>>
>>> +# Test valid redirect xattr with general directory cover lower origin
>>> +# target, should ask user this directory is merged or not by default
>>> +# and do nothing in auto mode
>>> +echo "+ Valid redirect(4)"
>>> +mkdir $lowerdir/origin
>>> +mkdir $upperdir/origin
>>> +make_redirect_dir $upperdir/testdir "origin"
>>> +
>>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1
>>> || \
>>> + _fail "fsck should not fail"
>>> +check_redirect "origin" $upperdir/testdir
>>> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR
>>> $upperdir/testdir
>>> +
>>> +echo "n" > $tmp.input
>>
>> This approach seems a bit fragile to me.
>> Maybe you will want to add some questions in the future.
>> To me it makes better sense to check behavior of fsck -y is automated
>> test,
>> because some user who does not understand the questions is surely going
>> to run fsck -y to fix problems.
>>
>>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >>
>>> $seqres.full 2>&1 || \
>>> + _fail "fsck should not fail"
>>> +check_redirect "origin" $upperdir/testdir
>>> +check_opaque $upperdir/origin
>>> +rm -rf $lowerdir/* $upperdir/*
>>> +
>
> Hi Amir：
>
> Think about this again. IIUC, I think there is a consistency problem about
> this case.
> If we allow user to create a new directory in upper layer merge with lower
> redirect origin target when overlay offline, we will see duplicate d_ino in
> overlay filesystem after next mount, becasue current ovl_getattr() will get
> the lower d_ino for both merged dir and redirect dir.
>
> eg:
> mnt: dira(ino=x) dirb(ino=x)
> upper: *dira(ino=y) dirb(ino=z, redirect=dirb)
> lower: dira(ino=x)
>
> *) User remove whiteout or opaque dir and then newly created when overlayfs
> is offline.
>
> Furthermore, if user rename dira again, will lead to duplicate redirect
> xattr directly.
> So I think we should forbid user do this and check by fsck.overlay. Or
> handle by overlay filesytem? thought?
>
I am not sure I follow. Of course there is a consistency problem with
allowing merge
and redirect to same target. merge dir is just a private case of
"redirect to .".
merge+redirect is not different in any way than multiple redirect.
All our discussion w.r.t the "Is merge dir?" question was revolving around the
fact that either the merge dir or the redirected dir needs to be fixed.
And duplicate d_ino is the *least* of the problem.
With duplicate redirect, there can be many objects (all descendants)
in the file system
with the same st_dev;st_ino, which may have different data.
This work in my queue is verifying multiple redirect/merge at lookup time,
but *only* if directories where indexed to begin with.
fsck.overlay will be needed to find and correct duplicate redirects
and even merge
dirs that were copied up without an index and fix the index (if
-overify=on is given).
There is a limitation of verify=on that it cannot use index to verify
mulitple redirect
from mid layers. This is where only fsck.overlay can be used.
Cheers,
Amir.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test
2017-12-30 9:35 ` Amir Goldstein
@ 2017-12-31 5:06 ` zhangyi
0 siblings, 0 replies; 18+ messages in thread
From: zhangyi @ 2017-12-31 5:06 UTC (permalink / raw)
To: Amir Goldstein
Cc: zhangyi (F), overlayfs, fstests, Miklos Szeredi, Eryu Guan,
Miao Xie, yangerkun
On 2017/12/30 17:35, Amir Goldstein Wrote:
> On Sat, Dec 30, 2017 at 6:06 AM, zhangyi (F) <yizhang089@gmail.com> wrote:
>> On 2017/12/28 20:37, Amir Goldstein Wrote:
>>
>>>> +# Test valid redirect xattr with general directory cover lower origin
>>>> +# target, should ask user this directory is merged or not by default
>>>> +# and do nothing in auto mode
>>>> +echo "+ Valid redirect(4)"
>>>> +mkdir $lowerdir/origin
>>>> +mkdir $upperdir/origin
>>>> +make_redirect_dir $upperdir/testdir "origin"
>>>> +
>>>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1
>>>> || \
>>>> + _fail "fsck should not fail"
>>>> +check_redirect "origin" $upperdir/testdir
>>>> +$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR
>>>> $upperdir/testdir
>>>> +
>>>> +echo "n" > $tmp.input
>>> This approach seems a bit fragile to me.
>>> Maybe you will want to add some questions in the future.
>>> To me it makes better sense to check behavior of fsck -y is automated
>>> test,
>>> because some user who does not understand the questions is surely going
>>> to run fsck -y to fix problems.
>>>
>>>> +_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >>
>>>> $seqres.full 2>&1 || \
>>>> + _fail "fsck should not fail"
>>>> +check_redirect "origin" $upperdir/testdir
>>>> +check_opaque $upperdir/origin
>>>> +rm -rf $lowerdir/* $upperdir/*
>>>> +
>> Hi Amir：
>>
>> Think about this again. IIUC, I think there is a consistency problem about
>> this case.
>> If we allow user to create a new directory in upper layer merge with lower
>> redirect origin target when overlay offline, we will see duplicate d_ino in
>> overlay filesystem after next mount, becasue current ovl_getattr() will get
>> the lower d_ino for both merged dir and redirect dir.
>>
>> eg:
>> mnt: dira(ino=x) dirb(ino=x)
>> upper: *dira(ino=y) dirb(ino=z, redirect=dirb)
>> lower: dira(ino=x)
>>
>> *) User remove whiteout or opaque dir and then newly created when overlayfs
>> is offline.
>>
>> Furthermore, if user rename dira again, will lead to duplicate redirect
>> xattr directly.
>> So I think we should forbid user do this and check by fsck.overlay. Or
>> handle by overlay filesytem? thought?
>>
> I am not sure I follow. Of course there is a consistency problem with
> allowing merge
> and redirect to same target. merge dir is just a private case of
> "redirect to .".
> merge+redirect is not different in any way than multiple redirect.
> All our discussion w.r.t the "Is merge dir?" question was revolving around the
> fact that either the merge dir or the redirected dir needs to be fixed.
Exactly，If user selsect the "merge" option(not opaque), we also should
fix the
redirected dir which point to the same origin. Thanks!
> And duplicate d_ino is the *least* of the problem.
> With duplicate redirect, there can be many objects (all descendants)
> in the file system
> with the same st_dev;st_ino, which may have different data.
>
> This work in my queue is verifying multiple redirect/merge at lookup time,
> but *only* if directories where indexed to begin with.
> fsck.overlay will be needed to find and correct duplicate redirects
> and even merge
> dirs that were copied up without an index and fix the index (if
> -overify=on is given).
>
> There is a limitation of verify=on that it cannot use index to verify
> mulitple redirect
> from mid layers. This is where only fsck.overlay can be used.
OK, I can add this check and fix latter. :）
Cheers,
Yi.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH xfstest v2 4/4] overlay: add fsck.overlay impure xattr test
2017-12-28 11:49 [PATCH xfstest v2 0/4] overlay: add fsck.overlay basic tests zhangyi (F)
` (2 preceding siblings ...)
2017-12-28 11:49 ` [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test zhangyi (F)
@ 2017-12-28 11:49 ` zhangyi (F)
2017-12-28 12:43 ` Amir Goldstein
3 siblings, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-28 11:49 UTC (permalink / raw)
To: linux-unionfs, fstests
Cc: miklos, amir73il, eguan, yi.zhang, miaoxie, yangerkun
Add fsck.overlay test case to test it how to deal with impure
xattr in underlying directories of overlayfs.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
tests/overlay/203 | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/overlay/203.out | 3 ++
tests/overlay/group | 1 +
3 files changed, 128 insertions(+)
create mode 100755 tests/overlay/203
create mode 100644 tests/overlay/203.out
diff --git a/tests/overlay/203 b/tests/overlay/203
new file mode 100755
index 0000000..f21dc6d
--- /dev/null
+++ b/tests/overlay/203
@@ -0,0 +1,124 @@
+#! /bin/bash
+# FS QA Test 203
+#
+# Test fsck.overlay how to deal with impure xattr in overlayfs.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Huawei. All Rights Reserved.
+# Author: zhangyi (F) <yi.zhang@huawei.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
+
+# remove all files from previous tests
+_scratch_mkfs
+
+OVL_REDIRECT_XATTR="trusted.overlay.redirect"
+OVL_IMPURE_XATTR="trusted.overlay.impure"
+OVL_IMPURE_XATTR_VAL="y"
+
+# Create a redirect directory
+make_redirect_dir()
+{
+ local target=$1
+ local value=$2
+
+ mkdir -p $target
+ $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
+}
+
+# Remove impure xattr
+remove_impure()
+{
+ local target=$1
+
+ $SETFATTR_PROG -x $OVL_IMPURE_XATTR $target
+}
+
+# Check impure xattr
+check_impure()
+{
+ local target=$1
+
+ value=$($GETFATTR_PROG --absolute-names --only-values \
+ -n $OVL_IMPURE_XATTR $target)
+
+ [[ $value == $OVL_IMPURE_XATTR_VAL ]] || echo "Missing impure xattr"
+}
+
+# Create test directories
+lowerdir=$OVL_BASE_SCRATCH_MNT/lower
+lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
+upperdir=$OVL_BASE_SCRATCH_MNT/upper
+workdir=$OVL_BASE_SCRATCH_MNT/workdir
+
+mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
+
+# Test missing impure xattr in directory which has origin files, should fix
+echo "+ Missing impure"
+mkdir $lowerdir/testdir $upperdir/testdir
+touch $lowerdir/testdir/foo
+_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
+touch $SCRATCH_MNT/testdir/foo
+$UMOUNT_PROG $SCRATCH_MNT
+remove_impure $upperdir/testdir
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+check_impure $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# Test missing impure xattr in directory which has redirect directories,
+# should fix
+echo "+ Missing impure(2)"
+mkdir -p $lowerdir/origin
+make_redirect_dir $upperdir/testdir/subdir "/origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
+ _fail "fsck should not fail"
+check_impure $upperdir/testdir
+rm -rf $lowerdir/* $upperdir/*
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/203.out b/tests/overlay/203.out
new file mode 100644
index 0000000..3bedf2a
--- /dev/null
+++ b/tests/overlay/203.out
@@ -0,0 +1,3 @@
+QA output created by 203
++ Missing impure
++ Missing impure(2)
diff --git a/tests/overlay/group b/tests/overlay/group
index e39b5e0..f99d89e 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -51,3 +51,4 @@
048 auto quick copyup hardlink
201 auto quick fsck
202 auto quick fsck
+203 auto quick fsck
--
2.5.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH xfstest v2 4/4] overlay: add fsck.overlay impure xattr test
2017-12-28 11:49 ` [PATCH xfstest v2 4/4] overlay: add fsck.overlay impure xattr test zhangyi (F)
@ 2017-12-28 12:43 ` Amir Goldstein
0 siblings, 0 replies; 18+ messages in thread
From: Amir Goldstein @ 2017-12-28 12:43 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On Thu, Dec 28, 2017 at 1:49 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add fsck.overlay test case to test it how to deal with impure
> xattr in underlying directories of overlayfs.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Looks good, besides the usual make_dirs() suggestion.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
> tests/overlay/203 | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/overlay/203.out | 3 ++
> tests/overlay/group | 1 +
> 3 files changed, 128 insertions(+)
> create mode 100755 tests/overlay/203
> create mode 100644 tests/overlay/203.out
>
> diff --git a/tests/overlay/203 b/tests/overlay/203
> new file mode 100755
> index 0000000..f21dc6d
> --- /dev/null
> +++ b/tests/overlay/203
> @@ -0,0 +1,124 @@
> +#! /bin/bash
> +# FS QA Test 203
> +#
> +# Test fsck.overlay how to deal with impure xattr in overlayfs.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Huawei. All Rights Reserved.
> +# Author: zhangyi (F) <yi.zhang@huawei.com>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs overlay
> +_supported_os Linux
> +_require_scratch
> +_require_command "$FSCK_OVERLAY_PROG" fsck.overlay
> +
> +# remove all files from previous tests
> +_scratch_mkfs
> +
> +OVL_REDIRECT_XATTR="trusted.overlay.redirect"
> +OVL_IMPURE_XATTR="trusted.overlay.impure"
> +OVL_IMPURE_XATTR_VAL="y"
> +
> +# Create a redirect directory
> +make_redirect_dir()
> +{
> + local target=$1
> + local value=$2
> +
> + mkdir -p $target
> + $SETFATTR_PROG -n $OVL_REDIRECT_XATTR -v $value $target
> +}
> +
> +# Remove impure xattr
> +remove_impure()
> +{
> + local target=$1
> +
> + $SETFATTR_PROG -x $OVL_IMPURE_XATTR $target
> +}
> +
> +# Check impure xattr
> +check_impure()
> +{
> + local target=$1
> +
> + value=$($GETFATTR_PROG --absolute-names --only-values \
> + -n $OVL_IMPURE_XATTR $target)
> +
> + [[ $value == $OVL_IMPURE_XATTR_VAL ]] || echo "Missing impure xattr"
> +}
> +
> +# Create test directories
> +lowerdir=$OVL_BASE_SCRATCH_MNT/lower
> +lowerdir2=$OVL_BASE_SCRATCH_MNT/lower2
> +upperdir=$OVL_BASE_SCRATCH_MNT/upper
> +workdir=$OVL_BASE_SCRATCH_MNT/workdir
> +
> +mkdir -p $lowerdir $lowerdir2 $upperdir $workdir $workdir
> +
> +# Test missing impure xattr in directory which has origin files, should fix
> +echo "+ Missing impure"
> +mkdir $lowerdir/testdir $upperdir/testdir
> +touch $lowerdir/testdir/foo
> +_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
> +touch $SCRATCH_MNT/testdir/foo
> +$UMOUNT_PROG $SCRATCH_MNT
> +remove_impure $upperdir/testdir
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +check_impure $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# Test missing impure xattr in directory which has redirect directories,
> +# should fix
> +echo "+ Missing impure(2)"
> +mkdir -p $lowerdir/origin
> +make_redirect_dir $upperdir/testdir/subdir "/origin"
> +
> +_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1 || \
> + _fail "fsck should not fail"
> +check_impure $upperdir/testdir
> +rm -rf $lowerdir/* $upperdir/*
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/overlay/203.out b/tests/overlay/203.out
> new file mode 100644
> index 0000000..3bedf2a
> --- /dev/null
> +++ b/tests/overlay/203.out
> @@ -0,0 +1,3 @@
> +QA output created by 203
> ++ Missing impure
> ++ Missing impure(2)
> diff --git a/tests/overlay/group b/tests/overlay/group
> index e39b5e0..f99d89e 100644
> --- a/tests/overlay/group
> +++ b/tests/overlay/group
> @@ -51,3 +51,4 @@
> 048 auto quick copyup hardlink
> 201 auto quick fsck
> 202 auto quick fsck
> +203 auto quick fsck
> --
> 2.5.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread