[PATCH V2] test online label ioctl

[PATCH V2] 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 <sandeen@redhat.com>
---

(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


diff --git a/common/rc b/common/rc
index 9ffab7fd..88a99cff 100644
--- a/common/rc
+++ b/common/rc
@@ -2158,6 +2158,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.
@@ -2196,7 +2199,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?)"
@@ -3802,6 +3805,31 @@ _require_scratch_feature()
 	esac
 }
 
+# The maximum filesystem label length, not including terminating NULL
+_label_get_max()
+{
+	case $FSTYP in
+	xfs)
+		MAXLEN=12
+		;;
+	btrfs)
+		MAXLEN=255
+		;;
+	*)
+		MAXLEN=0
+		;;
+	esac
+
+	echo $MAXLEN
+}
+
+_require_label_get_max()
+{
+	if [ $(_label_get_max) -eq 0 ]; then
+		_notrun "$FSTYP does not define maximum label length"
+	fi
+}
+
 init_rc
 
 ################################################################################
diff --git a/tests/generic/479 b/tests/generic/479
old mode 100644
new mode 100755
diff --git a/tests/generic/485 b/tests/generic/485
new file mode 100755
index 00000000..941afd3d
--- /dev/null
+++ b/tests/generic/485
@@ -0,0 +1,90 @@
+#! /bin/bash
+# FS QA Test 485
+#
+# Test the online filesystem label set/get ioctls
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2018 Red Hat, Inc.  All Rights Reserved.
+# Author: Eric Sandeen <sandeen@redhat.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 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 the interface max:
+LABEL=$(perl -e "print 'l' x 257;")
+$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT
+
+# And it succeeds right at the filesystem max:
+MAXLEN=$(_label_get_max)
+LABEL=$(perl -e "print 'o' x $MAXLEN;")
+$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT | sed -e 's/o\+/MAXLABEL/'
+
+# And that it fails past the filesystem max:
+let TOOLONG=MAXLEN+1
+LABEL=$(perl -e "print 'o' x $TOOLONG;")
+$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/485.out b/tests/generic/485.out
new file mode 100644
index 00000000..24efca8c
--- /dev/null
+++ b/tests/generic/485.out
@@ -0,0 +1,9 @@
+QA output created by 485
+label = "label.485"
+label = "label.485"
+SCRATCH_DEV: LABEL="label.485"
+SCRATCH_DEV: LABEL="label.485"
+label = "label.485"
+label: Invalid argument
+label = "MAXLABEL"
+label: Invalid argument
diff --git a/tests/generic/group b/tests/generic/group
index 19be9267..9ac5bbd1 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -487,3 +487,4 @@
 482 auto metadata replay
 483 auto quick log metadata
 484 auto quick
+485 auto quick

Re: [PATCH V2] test online label ioctl

[Dave Chinner]

On Mon, May 14, 2018 at 12:09:16PM -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 <sandeen@redhat.com>
> ---
> 
> (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
> 
> 
> diff --git a/common/rc b/common/rc
> index 9ffab7fd..88a99cff 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2158,6 +2158,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.
> @@ -2196,7 +2199,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?)"
> @@ -3802,6 +3805,31 @@ _require_scratch_feature()
>  	esac
>  }
>  
> +# The maximum filesystem label length, not including terminating NULL
> +_label_get_max()
> +{
> +	case $FSTYP in
> +	xfs)
> +		MAXLEN=12
> +		;;
> +	btrfs)
> +		MAXLEN=255
> +		;;
> +	*)
> +		MAXLEN=0

Why not just _notrun here?

> +		;;
> +	esac
> +
> +	echo $MAXLEN
> +}
> +
> +_require_label_get_max()
> +{
> +	if [ $(_label_get_max) -eq 0 ]; then
> +		_notrun "$FSTYP does not define maximum label length"
> +	fi

And this check can go away?

Also, shouldn't it be _require_online_label_change() ? And then
maybe you can move the xfs_io label command check inside it?

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

Ok, so "LABEL" here is a special blkid match token....

> +# 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 the interface max:
> +LABEL=$(perl -e "print 'l' x 257;")

And now you use it as a variable. Nasty and confusing. Using lower
case for local variables is the norm, right? I thought we were only
supposed to use upper case for global test harness variables...

But even making it "label" is problematic:

> +$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT

because "label" is an xfs_io command. Perhaps call it "fs_label"?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH V2] test online label ioctl

[Eric Sandeen]

On 5/14/18 6:11 PM, Dave Chinner wrote:
> On Mon, May 14, 2018 at 12:09:16PM -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 <sandeen@redhat.com>
>> ---

<snip>
 
>> +# The maximum filesystem label length, not including terminating NULL
>> +_label_get_max()
>> +{
>> +	case $FSTYP in
>> +	xfs)
>> +		MAXLEN=12
>> +		;;
>> +	btrfs)
>> +		MAXLEN=255
>> +		;;
>> +	*)
>> +		MAXLEN=0
> 
> Why not just _notrun here?

do we want to go through the other steps only to get here and notrun
halfway through?

>> +		;;
>> +	esac
>> +
>> +	echo $MAXLEN
>> +}
>> +
>> +_require_label_get_max()
>> +{
>> +	if [ $(_label_get_max) -eq 0 ]; then
>> +		_notrun "$FSTYP does not define maximum label length"
>> +	fi
> 
> And this check can go away?

We'd like to know if all the validations in this can complete before we
get started, right?  That's why I did it this way.

> Also, shouldn't it be _require_online_label_change() ? And then
> maybe you can move the xfs_io label command check inside it?

Well, we want to know a lot of things:

1) can the kernel code for this filesystem support online label
2) does xfs_io support the label command
3) does this test know the maximum label length to test for this fs

the above stuff is for 3)

>> +# 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"
> 
> Ok, so "LABEL" here is a special blkid match token....

yep

>> +# 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 the interface max:
>> +LABEL=$(perl -e "print 'l' x 257;")
> 
> And now you use it as a variable. Nasty and confusing. Using lower
> case for local variables is the norm, right? I thought we were only
> supposed to use upper case for global test harness variables...

I guess I didn't know about that convention (or forgot)

ok, yeah, that's a little confusing I guess.  *shrug*  I can change it
if you prefer.  Obviously the "blkid -s LABEL" must remain "LABEL"

> But even making it "label" is problematic:
> 
>> +$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT
> 
> because "label" is an xfs_io command. Perhaps call it "fs_label"?

ok

-Eric

Re: [PATCH V2] test online label ioctl

[Dave Chinner]

On Mon, May 14, 2018 at 06:26:07PM -0500, Eric Sandeen wrote:
> On 5/14/18 6:11 PM, Dave Chinner wrote:
> > On Mon, May 14, 2018 at 12:09:16PM -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 <sandeen@redhat.com>
> >> ---
> 
> <snip>
>  
> >> +# The maximum filesystem label length, not including terminating NULL
> >> +_label_get_max()
> >> +{
> >> +	case $FSTYP in
> >> +	xfs)
> >> +		MAXLEN=12
> >> +		;;
> >> +	btrfs)
> >> +		MAXLEN=255
> >> +		;;
> >> +	*)
> >> +		MAXLEN=0
> > 
> > Why not just _notrun here?
> 
> do we want to go through the other steps only to get here and notrun
> halfway through?
> 
> >> +		;;
> >> +	esac
> >> +
> >> +	echo $MAXLEN
> >> +}
> >> +
> >> +_require_label_get_max()
> >> +{
> >> +	if [ $(_label_get_max) -eq 0 ]; then
> >> +		_notrun "$FSTYP does not define maximum label length"
> >> +	fi
> > 
> > And this check can go away?
> 
> We'd like to know if all the validations in this can complete before we
> get started, right?  That's why I did it this way.

Sure, just trying to be a bit defensive as people often forget
_requires directives when writing new tests....

> > Also, shouldn't it be _require_online_label_change() ? And then
> > maybe you can move the xfs_io label command check inside it?
> 
> Well, we want to know a lot of things:
> 
> 1) can the kernel code for this filesystem support online label
> 2) does xfs_io support the label command
> 3) does this test know the maximum label length to test for this fs
> 
> the above stuff is for 3)

What I was suggesting was doing all of these in one function similar
to _require_xfs_sparse_inodes, _require_meta_uuid, etc:

_require_online_label_change()
{
	# need xfs_io support
	_require_xfs_io_command "label"

	# need fstests knowledge of label size
	[ $(_label_get_max) -eq 0 ] && _notrun "$FSTYP does not define maximum label length"

	# need kernel support
	$XFS_IO_PROG -c label $TEST_DIR > /dev/null 2>&1
	[ $? -ne 0 ] && _notrun "Kernel does not support FS_IOC_GETLABEL"
}

Which also means that the label_f command in xfs_io needs to set the
exitcode to non-zero when the ioctl fails so that xfs_io returns
non-zero on failure...

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com

[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 <sandeen@redhat.com>
---

(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 00000000..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 <sandeen@redhat.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 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 the interface max:
+fs_label=$(perl -e "print 'l' x 257;")
+$XFS_IO_PROG -c "label $fs_label" $SCRATCH_MNT
+
+# And it succeeds right at the filesystem max:
+max_label_len=$(_label_get_max)
+fs_label=$(perl -e "print 'o' x $max_label_len;")
+$XFS_IO_PROG -c "label $fs_label" $SCRATCH_MNT | sed -e 's/o\+/MAXLABEL/'
+
+# And that it fails past the filesystem max:
+let toolong_label_len=max_label_len+1
+fs_label=$(perl -e "print 'o' x $toolong_label_len;")
+$XFS_IO_PROG -c "label $fs_label " $SCRATCH_MNT
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/488.out b/tests/generic/488.out
new file mode 100644
index 00000000..3c70a15f
--- /dev/null
+++ b/tests/generic/488.out
@@ -0,0 +1,9 @@
+QA output created by 488
+label = "label.488"
+label = "label.488"
+SCRATCH_DEV: LABEL="label.488"
+SCRATCH_DEV: LABEL="label.488"
+label = "label.488"
+label: Invalid argument
+label = "MAXLABEL"
+label: Invalid argument
diff --git a/tests/generic/group b/tests/generic/group
index dc637c96..6266213e 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -490,3 +490,4 @@
  485 auto quick insert
  486 auto quick attr
  487 auto quick
+488 auto quick

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 <sandeen@redhat.com>
> ---
> 
> (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
david@fromorbit.com

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<sandeen@redhat.com>
>> ---
>>
>> (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

[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 <sandeen@redhat.com>
---

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: use/test new -s / -c options to set and clear


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/488 b/tests/generic/488
new file mode 100755
index 00000000..3616522d
--- /dev/null
+++ b/tests/generic/488
@@ -0,0 +1,95 @@
+#! /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 <sandeen@redhat.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 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 -s label.$seq" $SCRATCH_MNT
+$XFS_IO_PROG -c "label" $SCRATCH_MNT
+
+$XFS_IO_PROG -c "label -c" $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
+$XFS_IO_PROG -c "label -s label.$seq" $SCRATCH_MNT
+$XFS_IO_PROG -c "label" $SCRATCH_MNT
+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 the interface max:
+fs_label=$(perl -e "print 'l' x 257;")
+$XFS_IO_PROG -c "label -s $fs_label" $SCRATCH_MNT
+
+# And it succeeds right at the filesystem max:
+max_label_len=$(_label_get_max)
+fs_label=$(perl -e "print 'o' x $max_label_len;")
+$XFS_IO_PROG -c "label -s $fs_label" $SCRATCH_MNT | sed -e 's/o\+/MAXLABEL/'
+
+# And that it fails past the filesystem max:
+let toolong_label_len=max_label_len+1
+fs_label=$(perl -e "print 'o' x $toolong_label_len;")
+$XFS_IO_PROG -c "label -s $fs_label " $SCRATCH_MNT
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/488.out b/tests/generic/488.out
new file mode 100644
index 00000000..70691af4
--- /dev/null
+++ b/tests/generic/488.out
@@ -0,0 +1,13 @@
+QA output created by 488
+label = "label.488"
+label = "label.488"
+label = ""
+label = ""
+label = "label.488"
+label = "label.488"
+SCRATCH_DEV: LABEL="label.488"
+SCRATCH_DEV: LABEL="label.488"
+label = "label.488"
+label: Invalid argument
+label = "MAXLABEL"
+label: Invalid argument
diff --git a/tests/generic/group b/tests/generic/group
index dc637c96..6266213e 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -490,3 +490,4 @@
  485 auto quick insert
  486 auto quick attr
  487 auto quick
+488 auto quick

[PATCH V4] 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 <sandeen@redhat.com>
---

Now with new and improved sequential V4 versioning!

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

V4: use/test new -s / -c options to set and clear

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/488 b/tests/generic/488
new file mode 100755
index 00000000..3616522d
--- /dev/null
+++ b/tests/generic/488
@@ -0,0 +1,95 @@
+#! /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 <sandeen@redhat.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 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 -s label.$seq" $SCRATCH_MNT
+$XFS_IO_PROG -c "label" $SCRATCH_MNT
+
+$XFS_IO_PROG -c "label -c" $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
+$XFS_IO_PROG -c "label -s label.$seq" $SCRATCH_MNT
+$XFS_IO_PROG -c "label" $SCRATCH_MNT
+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 the interface max:
+fs_label=$(perl -e "print 'l' x 257;")
+$XFS_IO_PROG -c "label -s $fs_label" $SCRATCH_MNT
+
+# And it succeeds right at the filesystem max:
+max_label_len=$(_label_get_max)
+fs_label=$(perl -e "print 'o' x $max_label_len;")
+$XFS_IO_PROG -c "label -s $fs_label" $SCRATCH_MNT | sed -e 's/o\+/MAXLABEL/'
+
+# And that it fails past the filesystem max:
+let toolong_label_len=max_label_len+1
+fs_label=$(perl -e "print 'o' x $toolong_label_len;")
+$XFS_IO_PROG -c "label -s $fs_label " $SCRATCH_MNT
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/488.out b/tests/generic/488.out
new file mode 100644
index 00000000..70691af4
--- /dev/null
+++ b/tests/generic/488.out
@@ -0,0 +1,13 @@
+QA output created by 488
+label = "label.488"
+label = "label.488"
+label = ""
+label = ""
+label = "label.488"
+label = "label.488"
+SCRATCH_DEV: LABEL="label.488"
+SCRATCH_DEV: LABEL="label.488"
+label = "label.488"
+label: Invalid argument
+label = "MAXLABEL"
+label: Invalid argument
diff --git a/tests/generic/group b/tests/generic/group
index dc637c96..6266213e 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -490,3 +490,4 @@
  485 auto quick insert
  486 auto quick attr
  487 auto quick
+488 auto quick

Re: [PATCH V4] test online label ioctl

[Dave Chinner]

On Thu, May 17, 2018 at 10:28:26AM -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 <sandeen@redhat.com>
> ---
> 
> Now with new and improved sequential V4 versioning!

Looks good now.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com