From: Eryu Guan <eguan@redhat.com>
To: Zorro Lang <zlang@redhat.com>
Cc: fstests@vger.kernel.org, xfs@oss.sgi.com
Subject: Re: [PATCH v2] xfs/187: fix new sb_features2 stop case running
Date: Fri, 2 Sep 2016 16:38:26 +0800 [thread overview]
Message-ID: <20160902083826.GK27776@eguan.usersys.redhat.com> (raw)
In-Reply-To: <1472642336-28112-1-git-send-email-zlang@redhat.com>
On Wed, Aug 31, 2016 at 07:18:56PM +0800, Zorro Lang wrote:
> This case is too old, at that time there's no "ftype" feature for
> XFS. Due to this case need to clear features2 bits when mkfs.xfs,
> so ftype bit stop case running for long time.
>
> New common function _require_old_xfs_format() will help to fix
> this problem. Call it as:
>
> _require_old_xfs_format ATTR2 LAZYSBCOUNT
>
> Then it'll help to clear all features2 bits, besides ATTR2 and
> LAZYSBCOUNT which will be tested in case.
>
> Signed-off-by: Zorro Lang <zlang@redhat.com>
Looks good to me overall, but the commit log and comments seem not so
clear to me :)
Some comments inline.
> ---
>
> Hi,
>
> V2 add a new common function _require_old_xfs_format(), which help to
> to make sure no features2 xfs bits will be set.
>
> But mostly we still want to test some features2 bits, so I make
> this function won't deal with those features which are specified by
> arguments.
>
> For clear CRC feature, we can set MKFS_OPTIONS="$MKFS_OPTIONS -m crc=0"
> simply. But if the user specify crc=1/0 in local.config file, the test
> can't continue running. So I check if it has been set in the function.
I think this is because newer version of xfsprogs doesn't allow
specifying an option multiple times.
>
> Please tell me if you have better way to implement this function:)
>
> By the way, did I miss some features2?
>
> Thanks,
> Zorro
>
> common/rc | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/xfs/187 | 36 ++++++++++------------------
> 2 files changed, 87 insertions(+), 24 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 3fb0600..29e2987 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3923,6 +3923,81 @@ _require_xfs_mkfs_without_validation()
> fi
> }
>
> +# Make sure no features2 bits in XFS, besides those features are
By "besides" I think you mean "except" :)
> +# specified by arguments. All current features2 names as below:
> +# "CRC FINOBT PROJID32BIT ATTR2 LAZYSBCOUNT FTYPE"
> +_require_old_xfs_format()
> +{
> + local skiplist="$*"
> + local ftr2=""
> + local b
Seems not a good var name
> + local opts
"opts" is not used.
> +
> + _scratch_mkfs >/dev/null 2>&1
> + ftr2=`$XFS_DB_PROG -c version $SCRATCH_DEV | tr 'a-z' 'A-Z' |\
> + sed -n -e "s/,/ /g" -e "s/.*MOREBITS\(.*\)/\1/p"`
> +
> + for b in `echo $skiplist | tr 'a-z' 'A-Z'`; do
> + i=`echo $ftr2 | sed -n -e "s/\(.*\)$b\(.*\)/\1\2/p"`
> + if [ -n "$b" ]; then
> + ftr2="$i"
> + fi
> + done
> +
> + for b in $ftr2; do
> + case $b in
> + CRC)
> + if echo $MKFS_OPTIONS | grep -q "crc=1"; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/crc=1/crc=0/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -m crc=0"
> + fi
> + ;;
> + FINOBT)
> + if echo $MKFS_OPTIONS | grep -q "finobt=1"; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/finobt=1/finobt=0/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -m finobt=0"
> + fi
> + ;;
> + PROJID32BIT)
> + if echo $MKFS_OPTIONS | grep -q "projid32bit=1"; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/projid32bit=1/projid32bit=0/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -i projid32bit=0"
> + fi
> + ;;
> + ATTR2)
> + if echo $MKFS_OPTIONS | grep -q "attr="; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/attr=[0-9]/attr=1/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -i attr=1"
> + fi
> + ;;
> + LAZYSBCOUNT)
> + if echo $MKFS_OPTIONS | grep -q "lazy-count=1"; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/lazy-count=1/lazy-count=0/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -l lazy-count=0"
> + fi
> + ;;
> + FTYPE)
> + if echo $MKFS_OPTIONS | grep -q "ftype=1"; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/ftype=1/ftype=0/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -n ftype=0"
> + fi
> + ;;
> + esac
> + done
> +}
> +
> init_rc
>
> ################################################################################
> diff --git a/tests/xfs/187 b/tests/xfs/187
> index 836b924..ffc851c 100755
> --- a/tests/xfs/187
> +++ b/tests/xfs/187
> @@ -31,7 +31,6 @@ 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
> @@ -57,25 +56,16 @@ _supported_fs xfs
> _supported_os Linux
>
> _require_scratch
> +# clear features2 bits which we won't test
> +_require_old_xfs_format ATTR2 LAZYSBCOUNT
Need more comments to state that ATTR2 and LAZYSBCOUNT are features we
want to keep, otherwise this looks like only these two features are
cleared.
> _require_attrs
> -_require_attr_v1
> -_require_projid16bit
I'd keep above two requires to make sure mkfs supports such features.
>
> rm -f $seqres.full
> -
> -# Reset the options so that we can control what is going on here
> -export MKFS_OPTIONS=""
> -export MOUNT_OPTIONS=""
> -
> -# lazysb, attr2 and other feature bits are held in features2 and will require
> -# morebitsbit on So test with lazysb and without it to see if the morebitsbit is
> -# okay etc. If the mkfs defaults change, these need to change as well.
> -export MKFS_NO_LAZY="-m crc=0 -l lazy-count=0 -i projid32bit=0"
> -export MKFS_LAZY="-m crc=0 -l lazy-count=1 -i projid32bit=0"
> +_scratch_mkfs >/dev/null 2>&1
What does this _scratch_mkfs do?
Thanks,
Eryu
>
> # Make sure that when we think we are testing with morebits off
> # that we really are.
> -_scratch_mkfs -i attr=1 $MKFS_NO_LAZY >/dev/null 2>&1
> +_scratch_mkfs -i attr=1 -l lazy-count=0 >/dev/null 2>&1
> $XFS_DB_PROG -c version $SCRATCH_DEV 2>&1 >$tmp.db
> if grep -i morebits $tmp.db
> then
> @@ -90,13 +80,13 @@ echo "*** 1. test attr2 mkfs and then noattr2 mount ***"
> echo ""
> echo "attr2 fs"
> echo ""
> -_scratch_mkfs -i attr=2 $MKFS_NO_LAZY >/dev/null 2>&1
> +_scratch_mkfs -i attr=2 -l lazy-count=0 >/dev/null 2>&1
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
> echo ""
> echo "noattr2 fs"
> echo ""
> _scratch_mount -o noattr2
> -$UMOUNT_PROG $SCRATCH_MNT
> +_scratch_unmount
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
>
> # adding an EA will ensure the ATTR1 flag is turned on
> @@ -105,7 +95,7 @@ echo "*** 2. test attr2 mkfs and then noattr2 mount with 1 EA ***"
> echo ""
> echo "attr2 fs"
> echo ""
> -_scratch_mkfs -i attr=2 $MKFS_NO_LAZY >/dev/null 2>&1
> +_scratch_mkfs -i attr=2 -l lazy-count=0 >/dev/null 2>&1
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
> echo ""
> echo "noattr2 fs"
> @@ -115,8 +105,8 @@ cd $SCRATCH_MNT
> touch testfile
> $SETFATTR_PROG -n user.test -v 0xbabe testfile
> $GETFATTR_PROG testfile
> -cd $here
> -$UMOUNT_PROG $SCRATCH_MNT
> +cd - >/dev/null
> +_scratch_unmount
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
>
> echo ""
> @@ -125,16 +115,14 @@ echo ""
> echo ""
> echo "attr2 fs"
> echo ""
> -_scratch_mkfs -i attr=2 $MKFS_LAZY >/dev/null 2>&1
> +_scratch_mkfs -i attr=2 -l lazy-count=1 >/dev/null 2>&1
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
> echo ""
> echo "noattr2 fs"
> echo ""
> _scratch_mount -o noattr2
> -cd $SCRATCH_MNT
> -touch testfile
> -cd $here
> -$UMOUNT_PROG $SCRATCH_MNT
> +touch $SCRATCH_MNT/testfile
> +_scratch_unmount
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
>
> # success, all done
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Eryu Guan <eguan@redhat.com>
To: Zorro Lang <zlang@redhat.com>
Cc: fstests@vger.kernel.org, xfs@oss.sgi.com
Subject: Re: [PATCH v2] xfs/187: fix new sb_features2 stop case running
Date: Fri, 2 Sep 2016 16:38:26 +0800 [thread overview]
Message-ID: <20160902083826.GK27776@eguan.usersys.redhat.com> (raw)
In-Reply-To: <1472642336-28112-1-git-send-email-zlang@redhat.com>
On Wed, Aug 31, 2016 at 07:18:56PM +0800, Zorro Lang wrote:
> This case is too old, at that time there's no "ftype" feature for
> XFS. Due to this case need to clear features2 bits when mkfs.xfs,
> so ftype bit stop case running for long time.
>
> New common function _require_old_xfs_format() will help to fix
> this problem. Call it as:
>
> _require_old_xfs_format ATTR2 LAZYSBCOUNT
>
> Then it'll help to clear all features2 bits, besides ATTR2 and
> LAZYSBCOUNT which will be tested in case.
>
> Signed-off-by: Zorro Lang <zlang@redhat.com>
Looks good to me overall, but the commit log and comments seem not so
clear to me :)
Some comments inline.
> ---
>
> Hi,
>
> V2 add a new common function _require_old_xfs_format(), which help to
> to make sure no features2 xfs bits will be set.
>
> But mostly we still want to test some features2 bits, so I make
> this function won't deal with those features which are specified by
> arguments.
>
> For clear CRC feature, we can set MKFS_OPTIONS="$MKFS_OPTIONS -m crc=0"
> simply. But if the user specify crc=1/0 in local.config file, the test
> can't continue running. So I check if it has been set in the function.
I think this is because newer version of xfsprogs doesn't allow
specifying an option multiple times.
>
> Please tell me if you have better way to implement this function:)
>
> By the way, did I miss some features2?
>
> Thanks,
> Zorro
>
> common/rc | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/xfs/187 | 36 ++++++++++------------------
> 2 files changed, 87 insertions(+), 24 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 3fb0600..29e2987 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3923,6 +3923,81 @@ _require_xfs_mkfs_without_validation()
> fi
> }
>
> +# Make sure no features2 bits in XFS, besides those features are
By "besides" I think you mean "except" :)
> +# specified by arguments. All current features2 names as below:
> +# "CRC FINOBT PROJID32BIT ATTR2 LAZYSBCOUNT FTYPE"
> +_require_old_xfs_format()
> +{
> + local skiplist="$*"
> + local ftr2=""
> + local b
Seems not a good var name
> + local opts
"opts" is not used.
> +
> + _scratch_mkfs >/dev/null 2>&1
> + ftr2=`$XFS_DB_PROG -c version $SCRATCH_DEV | tr 'a-z' 'A-Z' |\
> + sed -n -e "s/,/ /g" -e "s/.*MOREBITS\(.*\)/\1/p"`
> +
> + for b in `echo $skiplist | tr 'a-z' 'A-Z'`; do
> + i=`echo $ftr2 | sed -n -e "s/\(.*\)$b\(.*\)/\1\2/p"`
> + if [ -n "$b" ]; then
> + ftr2="$i"
> + fi
> + done
> +
> + for b in $ftr2; do
> + case $b in
> + CRC)
> + if echo $MKFS_OPTIONS | grep -q "crc=1"; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/crc=1/crc=0/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -m crc=0"
> + fi
> + ;;
> + FINOBT)
> + if echo $MKFS_OPTIONS | grep -q "finobt=1"; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/finobt=1/finobt=0/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -m finobt=0"
> + fi
> + ;;
> + PROJID32BIT)
> + if echo $MKFS_OPTIONS | grep -q "projid32bit=1"; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/projid32bit=1/projid32bit=0/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -i projid32bit=0"
> + fi
> + ;;
> + ATTR2)
> + if echo $MKFS_OPTIONS | grep -q "attr="; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/attr=[0-9]/attr=1/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -i attr=1"
> + fi
> + ;;
> + LAZYSBCOUNT)
> + if echo $MKFS_OPTIONS | grep -q "lazy-count=1"; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/lazy-count=1/lazy-count=0/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -l lazy-count=0"
> + fi
> + ;;
> + FTYPE)
> + if echo $MKFS_OPTIONS | grep -q "ftype=1"; then
> + MKFS_OPTIONS=`echo $MKFS_OPTIONS | \
> + sed -e "s/ftype=1/ftype=0/g"`
> + else
> + MKFS_OPTIONS="$MKFS_OPTIONS -n ftype=0"
> + fi
> + ;;
> + esac
> + done
> +}
> +
> init_rc
>
> ################################################################################
> diff --git a/tests/xfs/187 b/tests/xfs/187
> index 836b924..ffc851c 100755
> --- a/tests/xfs/187
> +++ b/tests/xfs/187
> @@ -31,7 +31,6 @@ 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
> @@ -57,25 +56,16 @@ _supported_fs xfs
> _supported_os Linux
>
> _require_scratch
> +# clear features2 bits which we won't test
> +_require_old_xfs_format ATTR2 LAZYSBCOUNT
Need more comments to state that ATTR2 and LAZYSBCOUNT are features we
want to keep, otherwise this looks like only these two features are
cleared.
> _require_attrs
> -_require_attr_v1
> -_require_projid16bit
I'd keep above two requires to make sure mkfs supports such features.
>
> rm -f $seqres.full
> -
> -# Reset the options so that we can control what is going on here
> -export MKFS_OPTIONS=""
> -export MOUNT_OPTIONS=""
> -
> -# lazysb, attr2 and other feature bits are held in features2 and will require
> -# morebitsbit on So test with lazysb and without it to see if the morebitsbit is
> -# okay etc. If the mkfs defaults change, these need to change as well.
> -export MKFS_NO_LAZY="-m crc=0 -l lazy-count=0 -i projid32bit=0"
> -export MKFS_LAZY="-m crc=0 -l lazy-count=1 -i projid32bit=0"
> +_scratch_mkfs >/dev/null 2>&1
What does this _scratch_mkfs do?
Thanks,
Eryu
>
> # Make sure that when we think we are testing with morebits off
> # that we really are.
> -_scratch_mkfs -i attr=1 $MKFS_NO_LAZY >/dev/null 2>&1
> +_scratch_mkfs -i attr=1 -l lazy-count=0 >/dev/null 2>&1
> $XFS_DB_PROG -c version $SCRATCH_DEV 2>&1 >$tmp.db
> if grep -i morebits $tmp.db
> then
> @@ -90,13 +80,13 @@ echo "*** 1. test attr2 mkfs and then noattr2 mount ***"
> echo ""
> echo "attr2 fs"
> echo ""
> -_scratch_mkfs -i attr=2 $MKFS_NO_LAZY >/dev/null 2>&1
> +_scratch_mkfs -i attr=2 -l lazy-count=0 >/dev/null 2>&1
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
> echo ""
> echo "noattr2 fs"
> echo ""
> _scratch_mount -o noattr2
> -$UMOUNT_PROG $SCRATCH_MNT
> +_scratch_unmount
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
>
> # adding an EA will ensure the ATTR1 flag is turned on
> @@ -105,7 +95,7 @@ echo "*** 2. test attr2 mkfs and then noattr2 mount with 1 EA ***"
> echo ""
> echo "attr2 fs"
> echo ""
> -_scratch_mkfs -i attr=2 $MKFS_NO_LAZY >/dev/null 2>&1
> +_scratch_mkfs -i attr=2 -l lazy-count=0 >/dev/null 2>&1
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
> echo ""
> echo "noattr2 fs"
> @@ -115,8 +105,8 @@ cd $SCRATCH_MNT
> touch testfile
> $SETFATTR_PROG -n user.test -v 0xbabe testfile
> $GETFATTR_PROG testfile
> -cd $here
> -$UMOUNT_PROG $SCRATCH_MNT
> +cd - >/dev/null
> +_scratch_unmount
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
>
> echo ""
> @@ -125,16 +115,14 @@ echo ""
> echo ""
> echo "attr2 fs"
> echo ""
> -_scratch_mkfs -i attr=2 $MKFS_LAZY >/dev/null 2>&1
> +_scratch_mkfs -i attr=2 -l lazy-count=1 >/dev/null 2>&1
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
> echo ""
> echo "noattr2 fs"
> echo ""
> _scratch_mount -o noattr2
> -cd $SCRATCH_MNT
> -touch testfile
> -cd $here
> -$UMOUNT_PROG $SCRATCH_MNT
> +touch $SCRATCH_MNT/testfile
> +_scratch_unmount
> $XFS_DB_PROG -r -c version $SCRATCH_DEV 2>&1 | _filter_version
>
> # success, all done
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2016-09-02 8:38 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-31 11:18 [PATCH v2] xfs/187: fix new sb_features2 stop case running Zorro Lang
2016-08-31 11:18 ` Zorro Lang
2016-09-02 8:38 ` Eryu Guan [this message]
2016-09-02 8:38 ` Eryu Guan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160902083826.GK27776@eguan.usersys.redhat.com \
--to=eguan@redhat.com \
--cc=fstests@vger.kernel.org \
--cc=xfs@oss.sgi.com \
--cc=zlang@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.