Re: [PATCH 4/4] fstests: btrfs/126,127,128: test feature ioctl and sysfs interfaces

2016-06-27 Thread Eryu Guan
On Fri, Jun 24, 2016 at 11:08:34AM -0400, je...@suse.com wrote:
> From: Jeff Mahoney 
> 
> This tests the exporting of feature information from the kernel via
> sysfs and ioctl. The first test works whether the sysfs permissions
> are correct, if the information exported via sysfs matches
> what the ioctls are reporting, and if they both match the on-disk
> superblock's version of the feature sets. The second and third tests
> test online setting and clearing of feature bits via the sysfs and
> ioctl interfaces, checking whether they match the on-disk super on
> each cycle.
> 
> In every case, if the features are not present, it is not considered
> a failure and a message indicating that will be dumped to the $num.full
> file.
> 
> Signed-off-by: Jeff Mahoney 
> ---
>  tests/btrfs/126 | 269 
> 
>  tests/btrfs/126.out |   2 +
>  tests/btrfs/127 | 185 
>  tests/btrfs/127.out |   2 +
>  tests/btrfs/128 | 178 ++
>  tests/btrfs/128.out |   2 +
>  tests/btrfs/group   |   3 +
>  7 files changed, 641 insertions(+)
>  create mode 100755 tests/btrfs/126
>  create mode 100644 tests/btrfs/126.out
>  create mode 100755 tests/btrfs/127
>  create mode 100644 tests/btrfs/127.out
>  create mode 100755 tests/btrfs/128
>  create mode 100644 tests/btrfs/128.out
> 
> diff --git a/tests/btrfs/126 b/tests/btrfs/126
> new file mode 100755
> index 000..3d660c5
> --- /dev/null
> +++ b/tests/btrfs/126
> @@ -0,0 +1,269 @@
> +#!/bin/bash
> +# FA QA Test No. 126
> +#
> +# Test online feature publishing
> +#
> +# This test doesn't test the changing of features. It does test that
> +# the proper publishing bits and permissions match up with
> +# the expected values.
> +#
> +#---
> +# Copyright (c) 2013 SUSE, All Rights Reserved.

Copyright year 2016.

> +#
> +# 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

Missing _cleanup() and trap, use './new btrfs' to create new btrfs
tests.

> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter.btrfs
> +
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +_require_command $BTRFS_SHOW_SUPER_PROG

_require_command "$BTRFS_SHOW_SUPER_PROG" btrfs-show-super

> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +
> +check_features() {

"{" on a seperate line

> + reserved="$2"
> + method="$3"
> + if [ "$1" != 0 ]; then
> + echo "$method: failed: $reserved"
> + exit 1
> + fi

No need to check return value.

> +if [ "$reserved" = "Not implemented." ]; then
> +echo "Skipping ioctl test. Not implemented." >> $seqres.full
> +return
> +fi

Call _notrun if ioctl not implemented. Do the check before actual test
starts.

And you're mixing spaces and tabs for indention in this function.

> +}
> +
> +error=false

All the checks around error can be omitted.

> +
> +# test -w will always return true if root is making the call.
> +# This would be true in most cases, but for sysfs files, the permissions
> +# are enforced even for root.
> +is_writeable() {

"{" on a seperate line

> + local file=$1
> + mode=$(stat -c "0%a" "$file")
> + mode=$(( $mode & 0200 ))
> +
> + [ "$mode" -eq 0 ] && return 1
> + return 0
> +}
> +
> +# ioctl
> +read -a features < <(src/btrfs_ioctl_helper $SCRATCH_MNT GET_FEATURES 2>&1)
> +check_features $? "$features" "GET_FEATURES"
> +
> +test_ioctl=true
> +[ "${features[*]}" = "Not implemented." ] && test_ioctl=false
> +
> +read -a supp_features < <(src/btrfs_ioctl_helper $SCRATCH_MNT 
> GET_SUPPORTED_FEATURES 2>&1)
> +check_features $? "$supp_features" "GET_SUPPORTED_FEATURES"
> +[ "${supp_features[*]}" = "Not implemented." ] && test_ioctl=false

These checks are not needed if the test was checked and _notrun properly
before test.

> +
> +# Sysfs checks
> +fsid=$(_btrfs_get_fsid $SCRATCH_DEV)
> +sysfs_base="/sys/fs/btrfs"
> +
> +# TODO Add tool to enable and test unknown feature bits
> +get_feature_mask() {
> + class

[PATCH 4/4] fstests: btrfs/126,127,128: test feature ioctl and sysfs interfaces

2016-06-24 Thread jeffm
From: Jeff Mahoney 

This tests the exporting of feature information from the kernel via
sysfs and ioctl. The first test works whether the sysfs permissions
are correct, if the information exported via sysfs matches
what the ioctls are reporting, and if they both match the on-disk
superblock's version of the feature sets. The second and third tests
test online setting and clearing of feature bits via the sysfs and
ioctl interfaces, checking whether they match the on-disk super on
each cycle.

In every case, if the features are not present, it is not considered
a failure and a message indicating that will be dumped to the $num.full
file.

Signed-off-by: Jeff Mahoney 
---
 tests/btrfs/126 | 269 
 tests/btrfs/126.out |   2 +
 tests/btrfs/127 | 185 
 tests/btrfs/127.out |   2 +
 tests/btrfs/128 | 178 ++
 tests/btrfs/128.out |   2 +
 tests/btrfs/group   |   3 +
 7 files changed, 641 insertions(+)
 create mode 100755 tests/btrfs/126
 create mode 100644 tests/btrfs/126.out
 create mode 100755 tests/btrfs/127
 create mode 100644 tests/btrfs/127.out
 create mode 100755 tests/btrfs/128
 create mode 100644 tests/btrfs/128.out

diff --git a/tests/btrfs/126 b/tests/btrfs/126
new file mode 100755
index 000..3d660c5
--- /dev/null
+++ b/tests/btrfs/126
@@ -0,0 +1,269 @@
+#!/bin/bash
+# FA QA Test No. 126
+#
+# Test online feature publishing
+#
+# This test doesn't test the changing of features. It does test that
+# the proper publishing bits and permissions match up with
+# the expected values.
+#
+#---
+# Copyright (c) 2013 SUSE, All Rights Reserved.
+#
+# 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
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter.btrfs
+
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+_require_command $BTRFS_SHOW_SUPER_PROG
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+
+check_features() {
+   reserved="$2"
+   method="$3"
+   if [ "$1" != 0 ]; then
+   echo "$method: failed: $reserved"
+   exit 1
+   fi
+if [ "$reserved" = "Not implemented." ]; then
+echo "Skipping ioctl test. Not implemented." >> $seqres.full
+return
+fi
+}
+
+error=false
+
+# test -w will always return true if root is making the call.
+# This would be true in most cases, but for sysfs files, the permissions
+# are enforced even for root.
+is_writeable() {
+   local file=$1
+   mode=$(stat -c "0%a" "$file")
+   mode=$(( $mode & 0200 ))
+
+   [ "$mode" -eq 0 ] && return 1
+   return 0
+}
+
+# ioctl
+read -a features < <(src/btrfs_ioctl_helper $SCRATCH_MNT GET_FEATURES 2>&1)
+check_features $? "$features" "GET_FEATURES"
+
+test_ioctl=true
+[ "${features[*]}" = "Not implemented." ] && test_ioctl=false
+
+read -a supp_features < <(src/btrfs_ioctl_helper $SCRATCH_MNT 
GET_SUPPORTED_FEATURES 2>&1)
+check_features $? "$supp_features" "GET_SUPPORTED_FEATURES"
+[ "${supp_features[*]}" = "Not implemented." ] && test_ioctl=false
+
+# Sysfs checks
+fsid=$(_btrfs_get_fsid $SCRATCH_DEV)
+sysfs_base="/sys/fs/btrfs"
+
+# TODO Add tool to enable and test unknown feature bits
+get_feature_mask() {
+   class=""
+   case "$attr" in
+   mixed_backref)  class=incompat; bit=0x1 ;;
+   default_subvol) class=incompat; bit=0x2 ;;
+   mixed_groups)   class=incompat; bit=0x4 ;;
+   compress_lzo)   class=incompat; bit=0x8 ;;
+   compress_lsov2) class=incompat; bit=0x10 ;;
+   big_metadata)   class=incompat; bit=0x20 ;;
+   extended_iref)  class=incompat; bit=0x40 ;;
+   raid56) class=incompat; bit=0x80 ;;
+   skinny_metadata)class=incompat; bit=0x100 ;;
+   compat:*)   class=compat; bit=${attr##compat:} ;;
+   compat_ro:*)class=compat_ro; bit=${attr##compat_ro:} ;;
+   incompat:*) class=incompat; bit=${attr##incompat:} ;;
+   esac
+   if [ -z "$class" ]; then
+   echo "Unknown feature name $attr. xfstests needs updating." \
+