[PATCH 0/4] some random small fixes

[PATCH 0/4] some random small fixes

[Eryu Guan]

Eryu Guan (4):
  new: fix template for new case to use a single tab not four spaces
  generic/027: discard mkdir error message
  common: append -d option to XFS_COPY_PROG when testing v5 xfs
  xfs/073: be quiet about mouting with nouuid option

 common/rc         | 5 +++++
 new               | 4 ++--
 tests/generic/027 | 6 +++---
 tests/xfs/073     | 2 +-
 tests/xfs/073.out | 1 -
 5 files changed, 11 insertions(+), 7 deletions(-)

-- 
2.1.0

[PATCH 1/4] new: fix template for new case to use a single tab not four spaces

[Eryu Guan]

So we have preferred code style for all new test cases.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 new | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/new b/new
index 86f9075..d1f8939 100755
--- a/new
+++ b/new
@@ -148,8 +148,8 @@ trap "_cleanup; exit \\\$status" 0 1 2 3 15
 
 _cleanup()
 {
-    cd /
-    rm -f \$tmp.*
+	cd /
+	rm -f \$tmp.*
 }
 
 # get standard environment, filters and checks
-- 
2.1.0

[PATCH 2/4] generic/027: discard mkdir error message

[Eryu Guan]

mkdir fails due to ENOSPC occasionally and will fail the whole test.
Redirect stdout and stderr to /dev/null.

Also fix the code style in _cleanup to use single tab.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 tests/generic/027 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/generic/027 b/tests/generic/027
index 8dab117..d2e59d6 100755
--- a/tests/generic/027
+++ b/tests/generic/027
@@ -36,8 +36,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
-    cd /
-    rm -f $tmp.*
+	cd /
+	rm -f $tmp.*
 }
 
 create_file()
@@ -46,7 +46,7 @@ create_file()
 	local direct=$2
 	local i=0
 
-	mkdir -p $dir
+	mkdir -p $dir >/dev/null 2>&1
 	while $XFS_IO_PROG -f $direct -c "pwrite 0 1k" $dir/file_$i >/dev/null 2>&1; do
 		let i=$i+1
 	done
-- 
2.1.0

[PATCH 3/4] common: append -d option to XFS_COPY_PROG when testing v5 xfs

[Eryu Guan]

xfs_copy doesn't work on v5 xfs without -d option, this fails xfs/073
when testing xfs with MKFS_OPTIONS="-m crc=1" set.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 common/rc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/common/rc b/common/rc
index 1ed9df5..ad87a18 100644
--- a/common/rc
+++ b/common/rc
@@ -2926,6 +2926,11 @@ init_rc()
 	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
 	xfs_io -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
 	export XFS_IO_PROG="$XFS_IO_PROG -F"
+
+	# xfs_copy doesn't work on v5 xfs yet without -d option
+	if [ "$FSTYP" == "xfs" ] && [[ $MKFS_OPTIONS =~ crc=1 ]]; then
+		export XFS_COPY_PROG="$XFS_COPY_PROG -d"
+	fi
 }
 
 # get real device path name by following link
-- 
2.1.0

[PATCH 4/4] xfs/073: be quiet about mouting with nouuid option

[Eryu Guan]

v4 and v5 xfs generate different outputs because v4 xfs only retries
mount with nouuid option once in the test, v5 xfs tries more times(with
xfs_copy running with -d option).

Just be quiet about mounting with nouuid option, it's much easier than
preparing two different 073.outs and selecting the proper one at runtime

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 tests/xfs/073     | 2 +-
 tests/xfs/073.out | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/xfs/073 b/tests/xfs/073
index f955771..508725f 100755
--- a/tests/xfs/073
+++ b/tests/xfs/073
@@ -88,7 +88,7 @@ _verify_copy()
 
 	mount -t xfs -o loop $target $target_dir 2>/dev/null
 	if [ $? -ne 0 ]; then
-		echo retrying mount with nouuid option
+		echo retrying mount with nouuid option >>$seqres.full
 		mount -t xfs -o loop -o nouuid $target $target_dir
 		if [ $? -ne 0 ]; then
 			echo mount failed - evil!
diff --git a/tests/xfs/073.out b/tests/xfs/073.out
index f3b147e..3f27467 100644
--- a/tests/xfs/073.out
+++ b/tests/xfs/073.out
@@ -29,7 +29,6 @@ Creating file <FSIMAGE1>
 All copies completed.
 checking new image
 mounting new image on loopback
-retrying mount with nouuid option
 comparing new image files to old
 comparing new image directories to old
 comparing new image geometry to old
-- 
2.1.0

Re: [PATCH 1/4] new: fix template for new case to use a single tab not four spaces

[Eric Sandeen]

On 3/11/15 7:15 AM, Eryu Guan wrote:
> So we have preferred code style for all new test cases.

Sure, makes sense.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
>  new | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/new b/new
> index 86f9075..d1f8939 100755
> --- a/new
> +++ b/new
> @@ -148,8 +148,8 @@ trap "_cleanup; exit \\\$status" 0 1 2 3 15
>  
>  _cleanup()
>  {
> -    cd /
> -    rm -f \$tmp.*
> +	cd /
> +	rm -f \$tmp.*
>  }
>  
>  # get standard environment, filters and checks
> 

Re: [PATCH 2/4] generic/027: discard mkdir error message

[Eric Sandeen]

On 3/11/15 7:15 AM, Eryu Guan wrote:
> mkdir fails due to ENOSPC occasionally and will fail the whole test.
> Redirect stdout and stderr to /dev/null.
> 
> Also fix the code style in _cleanup to use single tab.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  tests/generic/027 | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/generic/027 b/tests/generic/027
> index 8dab117..d2e59d6 100755
> --- a/tests/generic/027
> +++ b/tests/generic/027
> @@ -36,8 +36,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>  
>  _cleanup()
>  {
> -    cd /
> -    rm -f $tmp.*
> +	cd /
> +	rm -f $tmp.*
>  }
>  
>  create_file()
> @@ -46,7 +46,7 @@ create_file()
>  	local direct=$2
>  	local i=0
>  
> -	mkdir -p $dir
> +	mkdir -p $dir >/dev/null 2>&1
>  	while $XFS_IO_PROG -f $direct -c "pwrite 0 1k" $dir/file_$i >/dev/null 2>&1; do
>  		let i=$i+1
>  	done
> 

Re: [PATCH 3/4] common: append -d option to XFS_COPY_PROG when testing v5 xfs

[Eric Sandeen]

On 3/11/15 7:15 AM, Eryu Guan wrote:
> xfs_copy doesn't work on v5 xfs without -d option, this fails xfs/073
> when testing xfs with MKFS_OPTIONS="-m crc=1" set.

Neat, I did not know about the "=~" trick ;)

Test xfs/073 adds -d already, but two -d's are accepted...

... but the xfs/073 fails due to the fallback to nouuid, forced by
the duplicate ID which is found:

@@ -17,6 +17,7 @@
 All copies completed.
 checking new image
 mounting new image on loopback
+retrying mount with nouuid option

Also:

* This will only work until crcs are made default
* If $TEST_DEV was made with CRCs then "crc=1" might not be in MKFS_OPTIONS,
  but xfs_copy of $TEST_DEV would still fail

But we only ever use it on $SCRATCH_DEV in tests today, and if/when
crcs are made default we can figure out how to test for that later...

Still, need some solution that doesn't fail the existing test.

We could just _notrun this test with crcs ... talking with hch, I wonder
if we even have a use for xfs_copy anymore.   A changed-uuid xfs_copy
will be very hard to do w/ v5 filesystems.

-Eric

> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
>  common/rc | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 1ed9df5..ad87a18 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2926,6 +2926,11 @@ init_rc()
>  	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
>  	xfs_io -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
>  	export XFS_IO_PROG="$XFS_IO_PROG -F"
> +
> +	# xfs_copy doesn't work on v5 xfs yet without -d option
> +	if [ "$FSTYP" == "xfs" ] && [[ $MKFS_OPTIONS =~ crc=1 ]]; then
> +		export XFS_COPY_PROG="$XFS_COPY_PROG -d"
> +	fi
>  }
>  
>  # get real device path name by following link
> 

Re: [PATCH 4/4] xfs/073: be quiet about mouting with nouuid option

[Eric Sandeen]

On 3/11/15 7:15 AM, Eryu Guan wrote:
> v4 and v5 xfs generate different outputs because v4 xfs only retries
> mount with nouuid option once in the test, v5 xfs tries more times(with
> xfs_copy running with -d option).
> 
> Just be quiet about mounting with nouuid option, it's much easier than
> preparing two different 073.outs and selecting the proper one at runtime

Ah, ok, I see now.  Might have been better to swap the order of your patches
3 and 4 so things work after each patch application.

This does make the test slightly less useful, as we don't know whether we've
properly copied the UUID (if for some strange reason it changed when it
shouldn't) but that doesn't seem too likely or important, so:

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
>  tests/xfs/073     | 2 +-
>  tests/xfs/073.out | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tests/xfs/073 b/tests/xfs/073
> index f955771..508725f 100755
> --- a/tests/xfs/073
> +++ b/tests/xfs/073
> @@ -88,7 +88,7 @@ _verify_copy()
>  
>  	mount -t xfs -o loop $target $target_dir 2>/dev/null
>  	if [ $? -ne 0 ]; then
> -		echo retrying mount with nouuid option
> +		echo retrying mount with nouuid option >>$seqres.full
>  		mount -t xfs -o loop -o nouuid $target $target_dir
>  		if [ $? -ne 0 ]; then
>  			echo mount failed - evil!
> diff --git a/tests/xfs/073.out b/tests/xfs/073.out
> index f3b147e..3f27467 100644
> --- a/tests/xfs/073.out
> +++ b/tests/xfs/073.out
> @@ -29,7 +29,6 @@ Creating file <FSIMAGE1>
>  All copies completed.
>  checking new image
>  mounting new image on loopback
> -retrying mount with nouuid option
>  comparing new image files to old
>  comparing new image directories to old
>  comparing new image geometry to old
> 

Re: [PATCH 3/4] common: append -d option to XFS_COPY_PROG when testing v5 xfs

[Eric Sandeen]

On 3/11/15 10:48 AM, Eric Sandeen wrote:
> On 3/11/15 7:15 AM, Eryu Guan wrote:
>> xfs_copy doesn't work on v5 xfs without -d option, this fails xfs/073
>> when testing xfs with MKFS_OPTIONS="-m crc=1" set.
> 
> Neat, I did not know about the "=~" trick ;)
> 
> Test xfs/073 adds -d already, but two -d's are accepted...
> 
> ... but the xfs/073 fails due to the fallback to nouuid, forced by
> the duplicate ID which is found:
> 
> @@ -17,6 +17,7 @@
>  All copies completed.
>  checking new image
>  mounting new image on loopback
> +retrying mount with nouuid option

Oh, patch 4 fixes this.

Dave, you might want to apply 3 & 4 in swapped order.

I think this is ok for now,

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Re: [PATCH 3/4] common: append -d option to XFS_COPY_PROG when testing v5 xfs

[Eryu Guan]

On Wed, Mar 11, 2015 at 10:48:18AM -0400, Eric Sandeen wrote:
> On 3/11/15 7:15 AM, Eryu Guan wrote:
> > xfs_copy doesn't work on v5 xfs without -d option, this fails xfs/073
> > when testing xfs with MKFS_OPTIONS="-m crc=1" set.
> 
> Neat, I did not know about the "=~" trick ;)
> 
> Test xfs/073 adds -d already, but two -d's are accepted...
> 
> ... but the xfs/073 fails due to the fallback to nouuid, forced by
> the duplicate ID which is found:
> 
> @@ -17,6 +17,7 @@
>  All copies completed.
>  checking new image
>  mounting new image on loopback
> +retrying mount with nouuid option
> 
> Also:
> 
> * This will only work until crcs are made default
> * If $TEST_DEV was made with CRCs then "crc=1" might not be in MKFS_OPTIONS,
>   but xfs_copy of $TEST_DEV would still fail
> 
> But we only ever use it on $SCRATCH_DEV in tests today, and if/when
> crcs are made default we can figure out how to test for that later...

I think the right way to detect crc is to check $_fs_has_crcs var, like
what xfs/033 does. But still, this only works for test on SCRATCH_DEV.

And xfs/073 tests nouuid explicitly, I should not just hide the test and
make the test less useful. The two-out-files way is probably better.

I'll send v2 for patch 3 and 4.

As for the order of the patches, I fixed the xfs_copy on v5 xfs issue
and hit the extra "retrying mount with nouuid option" failure, so fixed
that later :)

Thank you so much for the detailed review!

Eryu
> 
> Still, need some solution that doesn't fail the existing test.
> 
> We could just _notrun this test with crcs ... talking with hch, I wonder
> if we even have a use for xfs_copy anymore.   A changed-uuid xfs_copy
> will be very hard to do w/ v5 filesystems.
> 
> -Eric
> 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> >  common/rc | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/common/rc b/common/rc
> > index 1ed9df5..ad87a18 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -2926,6 +2926,11 @@ init_rc()
> >  	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
> >  	xfs_io -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
> >  	export XFS_IO_PROG="$XFS_IO_PROG -F"
> > +
> > +	# xfs_copy doesn't work on v5 xfs yet without -d option
> > +	if [ "$FSTYP" == "xfs" ] && [[ $MKFS_OPTIONS =~ crc=1 ]]; then
> > +		export XFS_COPY_PROG="$XFS_COPY_PROG -d"
> > +	fi
> >  }
> >  
> >  # get real device path name by following link
> > 
> 
> --
> 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