Re: [PATCH V3] test online label ioctl

[Eric Sandeen]

On 5/15/18 7:51 PM, Dave Chinner wrote:

On Tue, May 15, 2018 at 10:22:37AM -0500, Eric Sandeen wrote:

This tests the online label ioctl that btrfs has, which has been
recently proposed for XFS.

To run, it requires an updated xfs_io with the label command and a
filesystem that supports it

A slight change here to _require_xfs_io_command as well, so that tests
which simply fail with "Inappropriate ioctl" can be caught in the
common case.

Signed-off-by: Eric Sandeen
---

(urgh send as proper new thread, sorry)

This passes on btrfs, _notruns on xfs/ext4 of yore, and passes
on xfs w/ my online label patchset (as long as xfs_io has the new
capability)

V2: Add a max label length helper
 Set the proper btrfs max label length o_O oops
 Filter trailing whitespace from blkid output

V3: lowercase local vars, simplify max label len function

Looks good now, but I wondered about one thing the test doesn't
cover: can you clear the label by setting it to a null string?
i.e you check max length bounds, but don't check empty string
behaviour...


hohum, yes.  I'll fix that, which will also require a change to xfs_io
to be able to set a null string.

Will send a V3 in a bit.

-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3] test online label ioctl

[Dave Chinner]

On Tue, May 15, 2018 at 10:22:37AM -0500, Eric Sandeen wrote:
> This tests the online label ioctl that btrfs has, which has been
> recently proposed for XFS.
> 
> To run, it requires an updated xfs_io with the label command and a
> filesystem that supports it
> 
> A slight change here to _require_xfs_io_command as well, so that tests
> which simply fail with "Inappropriate ioctl" can be caught in the
> common case.
> 
> Signed-off-by: Eric Sandeen 
> ---
> 
> (urgh send as proper new thread, sorry)
> 
> This passes on btrfs, _notruns on xfs/ext4 of yore, and passes
> on xfs w/ my online label patchset (as long as xfs_io has the new
> capability)
> 
> V2: Add a max label length helper
> Set the proper btrfs max label length o_O oops
> Filter trailing whitespace from blkid output
> 
> V3: lowercase local vars, simplify max label len function

Looks good now, but I wondered about one thing the test doesn't
cover: can you clear the label by setting it to a null string?
i.e you check max length bounds, but don't check empty string
behaviour...

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V3] test online label ioctl

[Eric Sandeen]

This tests the online label ioctl that btrfs has, which has been
recently proposed for XFS.

To run, it requires an updated xfs_io with the label command and a
filesystem that supports it

A slight change here to _require_xfs_io_command as well, so that tests
which simply fail with "Inappropriate ioctl" can be caught in the
common case.

Signed-off-by: Eric Sandeen 
---

(urgh send as proper new thread, sorry)

This passes on btrfs, _notruns on xfs/ext4 of yore, and passes
on xfs w/ my online label patchset (as long as xfs_io has the new
capability)

V2: Add a max label length helper
Set the proper btrfs max label length o_O oops
Filter trailing whitespace from blkid output

V3: lowercase local vars, simplify max label len function



diff --git a/common/rc b/common/rc
index ffe53236..333cfb82 100644
--- a/common/rc
+++ b/common/rc
@@ -2144,6 +2144,9 @@ _require_xfs_io_command()
echo $testio | grep -q "Inappropriate ioctl" && \
_notrun "xfs_io $command support is missing"
;;
+   "label")
+   testio=`$XFS_IO_PROG -c "label" $TEST_DIR 2>&1`
+   ;;
"open")
# -c "open $f" is broken in xfs_io <= 4.8. Along with the fix,
# a new -C flag was introduced to execute one shot commands.
@@ -2182,7 +2185,7 @@ _require_xfs_io_command()
rm -f $testfile 2>&1 > /dev/null
echo $testio | grep -q "not found" && \
_notrun "xfs_io $command support is missing"
-   echo $testio | grep -q "Operation not supported" && \
+   echo $testio | grep -q "Operation not supported\|Inappropriate ioctl" 
&& \
_notrun "xfs_io $command failed (old kernel/wrong fs?)"
echo $testio | grep -q "Invalid" && \
_notrun "xfs_io $command failed (old kernel/wrong fs/bad args?)"
@@ -3788,6 +3791,29 @@ _require_scratch_feature()
esac
 }
 
+# The maximum filesystem label length, /not/ including terminating NULL

+_label_get_max()
+{
+   case $FSTYP in
+   xfs)
+   echo 12
+   ;;
+   btrfs)
+   echo 255
+   ;;
+   *)
+   _notrun "$FSTYP does not define maximum label length"
+   ;;
+   esac
+}
+
+# Helper to check above early in a script
+_require_label_get_max()
+{
+   # Just call _label_get_max which will notrun if appropriate
+   dummy=$(_label_get_max)
+}
+
 init_rc
 
 

diff --git a/tests/generic/479 b/tests/generic/479
old mode 100644
new mode 100755
diff --git a/tests/generic/488 b/tests/generic/488
new file mode 100755
index ..9521c5ba
--- /dev/null
+++ b/tests/generic/488
@@ -0,0 +1,90 @@
+#! /bin/bash
+# FS QA Test 488
+#
+# Test the online filesystem label set/get ioctls
+#
+#---
+# Copyright (c) 2018 Red Hat, Inc.  All Rights Reserved.
+# Author: Eric Sandeen 
+#
+# 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 generic
+_supported_os Linux
+_require_scratch
+_require_xfs_io_command "label"
+_require_label_get_max
+
+_scratch_mkfs > $seqres.full 2>&1
+_scratch_mount
+
+# Make sure we can set & clear the label
+$XFS_IO_PROG -c "label label.$seq" $SCRATCH_MNT
+$XFS_IO_PROG -c "label" $SCRATCH_MNT
+
+# And that userspace can see it now, while mounted
+# NB: some blkid has trailing whitespace, filter it out here
+blkid -s LABEL $SCRATCH_DEV | _filter_scratch | sed -e "s/ $//g"
+
+# And that the it is still there when it's unmounted
+_scratch_unmount
+blkid -s LABEL $SCRATCH_DEV | _filter_scratch | sed -e "s/ $//g"
+
+# And that it persists after a remount
+_scratch_mount
+$XFS_IO_PROG -c "label" $SCRATCH_MNT
+
+# And that a too-long label is rejected, beyond