generic/258 questions (mount issue)...

generic/258 questions (mount issue)...

[Michael L. Semon]

Hi!  When using xfstests generic/258 with along with $TEST_RTDEV 
$TEST_LOGDEV, it tends to scream bloody murder about corrupted 
partitions and such.  In fact, the commands in the test seem to do the 
right thing when executed by hand.  So once again, I grasped for straws 
and came up with this:

--- xfstests/tests/generic/258.orig	2013-05-21 20:19:38.430754829 -0400
+++ xfstests/tests/generic/258	2013-05-21 20:10:11.509021368 -0400
@@ -62,7 +62,7 @@
  # unmount, remount, and check the timestamp
  echo "Remounting to flush cache"
  umount $TEST_DEV
-mount $TEST_DEV $TEST_DIR
+_test_mount

  # Should yield -315593940 (prior to epoch)
  echo "Testing for negative seconds since epoch"

My questions are these:

1) Was there a better way to do this?

2) Not knowing the policy on umounting $TEST_DEV, could this have been a 
test for $SCRATCH_DEV?

This was tested only on XFS with rtdev and logdev partitions enabled.

Thanks!

Michael

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: generic/258 questions (mount issue)...

[Dave Chinner]

On Tue, May 21, 2013 at 08:43:13PM -0400, Michael L. Semon wrote:
> Hi!  When using xfstests generic/258 with along with $TEST_RTDEV
> $TEST_LOGDEV, it tends to scream bloody murder about corrupted
> partitions and such.  In fact, the commands in the test seem to do
> the right thing when executed by hand.  So once again, I grasped for
> straws and came up with this:
> 
> --- xfstests/tests/generic/258.orig	2013-05-21 20:19:38.430754829 -0400
> +++ xfstests/tests/generic/258	2013-05-21 20:10:11.509021368 -0400
> @@ -62,7 +62,7 @@
>  # unmount, remount, and check the timestamp
>  echo "Remounting to flush cache"
>  umount $TEST_DEV
> -mount $TEST_DEV $TEST_DIR
> +_test_mount
> 
>  # Should yield -315593940 (prior to epoch)
>  echo "Testing for negative seconds since epoch"
> 
> My questions are these:
> 
> 1) Was there a better way to do this?

No, your change is correct. Can you clean up the description of the
problem you had and add a Signed-off-by?

> 2) Not knowing the policy on umounting $TEST_DEV, could this have
> been a test for $SCRATCH_DEV?

There are a handful of other tests that also unmount the TEST_DEV.
Perhaps adding a _test_umount() wrapper to common/rc (similar to
_scratch_umount) would be best. At least shared/243 needs the same
_test_mount treatment as this test.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: generic/258 questions (mount issue)...

[Michael L. Semon]

On 05/21/2013 09:26 PM, Dave Chinner wrote:
> On Tue, May 21, 2013 at 08:43:13PM -0400, Michael L. Semon wrote:
>> Hi!  When using xfstests generic/258 with along with $TEST_RTDEV
>> $TEST_LOGDEV, it tends to scream bloody murder about corrupted
>> partitions and such.  In fact, the commands in the test seem to do
>> the right thing when executed by hand.  So once again, I grasped for
>> straws and came up with this:
>>
>> --- xfstests/tests/generic/258.orig	2013-05-21 20:19:38.430754829 -0400
>> +++ xfstests/tests/generic/258	2013-05-21 20:10:11.509021368 -0400
>> @@ -62,7 +62,7 @@
>>  # unmount, remount, and check the timestamp
>>  echo "Remounting to flush cache"
>>  umount $TEST_DEV
>> -mount $TEST_DEV $TEST_DIR
>> +_test_mount
>>
>>  # Should yield -315593940 (prior to epoch)
>>  echo "Testing for negative seconds since epoch"
>>
>> My questions are these:
>>
>> 1) Was there a better way to do this?
> 
> No, your change is correct. Can you clean up the description of the
> problem you had and add a Signed-off-by?
> 
>> 2) Not knowing the policy on umounting $TEST_DEV, could this have
>> been a test for $SCRATCH_DEV?
> 
> There are a handful of other tests that also unmount the TEST_DEV.
> Perhaps adding a _test_umount() wrapper to common/rc (similar to
> _scratch_umount) would be best. At least shared/243 needs the same
> _test_mount treatment as this test.
> 
> Cheers,
> 
> Dave.
> 

This is my first attempt at doing a `git format-patch --stdout`.  
Hopefully, this is somewhat more presentable to git.  Comments and 
brow-beating are welcome.

Thanks!

Michael

>From 7dc0667e1f8cd1c98f15ebf412dd0f038b55306e Mon Sep 17 00:00:00 2001
From: "Michael L. Semon" <mlsemon35@gmail.com>
Date: Tue, 21 May 2013 22:50:59 -0400
Subject: [PATCH] Change mount method for generic/258

Use the built-in _test_mount function from xfstests so it will use
the correct mount options for xfstests.  The script used a simple
umount-and-mount sequence, which caused a test failure on an XFS
filesystem that used both realtime and external log devices.

Signed-off-by: Michael L. Semon <mlsemon35@gmail.com>
---
 tests/generic/258 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/generic/258 b/tests/generic/258
index fb091ae..3aeb5ce 100755
--- a/tests/generic/258
+++ b/tests/generic/258
@@ -62,7 +62,7 @@ fi
 # unmount, remount, and check the timestamp
 echo "Remounting to flush cache"
 umount $TEST_DEV
-mount $TEST_DEV $TEST_DIR
+_test_mount
 
 # Should yield -315593940 (prior to epoch)
 echo "Testing for negative seconds since epoch"
-- 
1.8.2


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: generic/258 questions (mount issue)...

[Eric Sandeen]

On 5/21/13 10:03 PM, Michael L. Semon wrote:

...

> From 7dc0667e1f8cd1c98f15ebf412dd0f038b55306e Mon Sep 17 00:00:00 2001
> From: "Michael L. Semon" <mlsemon35@gmail.com>
> Date: Tue, 21 May 2013 22:50:59 -0400
> Subject: [PATCH] Change mount method for generic/258
> 
> Use the built-in _test_mount function from xfstests so it will use
> the correct mount options for xfstests.  The script used a simple
> umount-and-mount sequence, which caused a test failure on an XFS
> filesystem that used both realtime and external log devices.
> 
> Signed-off-by: Michael L. Semon <mlsemon35@gmail.com>

Thanks, this was my fault  :(

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

Want to consider dave's other suggestions in another patch?

> There are a handful of other tests that also unmount the TEST_DEV.
> Perhaps adding a _test_umount() wrapper to common/rc (similar to
> _scratch_umount) would be best. At least shared/243 needs the same
> _test_mount treatment as this test.

Eternal git-log fame would be yours.  ;)

-Eric

> ---
>  tests/generic/258 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/258 b/tests/generic/258
> index fb091ae..3aeb5ce 100755
> --- a/tests/generic/258
> +++ b/tests/generic/258
> @@ -62,7 +62,7 @@ fi
>  # unmount, remount, and check the timestamp
>  echo "Remounting to flush cache"
>  umount $TEST_DEV
> -mount $TEST_DEV $TEST_DIR
> +_test_mount
>  
>  # Should yield -315593940 (prior to epoch)
>  echo "Testing for negative seconds since epoch"
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: generic/258 questions (mount issue)...

[Michael L. Semon]

On 05/22/2013 10:10 AM, Eric Sandeen wrote:
> On 5/21/13 10:03 PM, Michael L. Semon wrote:
> 
> ...
> 
>> From 7dc0667e1f8cd1c98f15ebf412dd0f038b55306e Mon Sep 17 00:00:00 2001
>> From: "Michael L. Semon" <mlsemon35@gmail.com>
>> Date: Tue, 21 May 2013 22:50:59 -0400
>> Subject: [PATCH] Change mount method for generic/258
>>
>> Use the built-in _test_mount function from xfstests so it will use
>> the correct mount options for xfstests.  The script used a simple
>> umount-and-mount sequence, which caused a test failure on an XFS
>> filesystem that used both realtime and external log devices.
>>
>> Signed-off-by: Michael L. Semon <mlsemon35@gmail.com>
> 
> Thanks, this was my fault  :(
> 
> Reviewed-by: Eric Sandeen <sandeen@rehat.com>
> 
> Want to consider dave's other suggestions in another patch?
> 
>> There are a handful of other tests that also unmount the TEST_DEV.
>> Perhaps adding a _test_umount() wrapper to common/rc (similar to
>> _scratch_umount) would be best. At least shared/243 needs the same
>> _test_mount treatment as this test.
> 
> Eternal git-log fame would be yours.  ;)
> 
> -Eric

Will do.  Let me get my feet back under me.  The initial 
`grep -l _test_mount [0-9][0-9][0-9]` returned 014, 097, 192; so 
the thread was started in the spirit of "there's no way this can 
possibly be the correct function to use."

I'll look at shared/243.

One day, I would like to earn git-log fame by incorporating better 
support for JFS and especially NILFS2 into xfstests.  However, 
that's a topic for another day.  I know that I have uses for both 
filesystems, but that doesn't mean anybody else does.  Their 
mailing lists don't give much hint of user community or progress.

Thanks!

Michael

>> ---
>>  tests/generic/258 | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/generic/258 b/tests/generic/258
>> index fb091ae..3aeb5ce 100755
>> --- a/tests/generic/258
>> +++ b/tests/generic/258
>> @@ -62,7 +62,7 @@ fi
>>  # unmount, remount, and check the timestamp
>>  echo "Remounting to flush cache"
>>  umount $TEST_DEV
>> -mount $TEST_DEV $TEST_DIR
>> +_test_mount
>>  
>>  # Should yield -315593940 (prior to epoch)
>>  echo "Testing for negative seconds since epoch"
>>
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: generic/258 questions (mount issue)...

[Eric Sandeen]

On 5/22/13 11:15 AM, Michael L. Semon wrote:
> On 05/22/2013 10:10 AM, Eric Sandeen wrote:
>> On 5/21/13 10:03 PM, Michael L. Semon wrote:
>>
>> ...
>>
>>> From 7dc0667e1f8cd1c98f15ebf412dd0f038b55306e Mon Sep 17 00:00:00 2001
>>> From: "Michael L. Semon" <mlsemon35@gmail.com>
>>> Date: Tue, 21 May 2013 22:50:59 -0400
>>> Subject: [PATCH] Change mount method for generic/258
>>>
>>> Use the built-in _test_mount function from xfstests so it will use
>>> the correct mount options for xfstests.  The script used a simple
>>> umount-and-mount sequence, which caused a test failure on an XFS
>>> filesystem that used both realtime and external log devices.
>>>
>>> Signed-off-by: Michael L. Semon <mlsemon35@gmail.com>
>>
>> Thanks, this was my fault  :(
>>
>> Reviewed-by: Eric Sandeen <sandeen@rehat.com>
>>
>> Want to consider dave's other suggestions in another patch?
>>
>>> There are a handful of other tests that also unmount the TEST_DEV.
>>> Perhaps adding a _test_umount() wrapper to common/rc (similar to
>>> _scratch_umount) would be best. At least shared/243 needs the same
>>> _test_mount treatment as this test.
>>
>> Eternal git-log fame would be yours.  ;)
>>
>> -Eric
> 
> Will do.  Let me get my feet back under me.  The initial 
> `grep -l _test_mount [0-9][0-9][0-9]` returned 014, 097, 192; so 
> the thread was started in the spirit of "there's no way this can 
> possibly be the correct function to use."
> 
> I'll look at shared/243.
> 
> One day, I would like to earn git-log fame by incorporating better 
> support for JFS and especially NILFS2 into xfstests.  However, 
> that's a topic for another day.  I know that I have uses for both 
> filesystems, but that doesn't mean anybody else does.  Their 
> mailing lists don't give much hint of user community or progress.

The generic tests hopefully work; if not, it should be a fairly
simple fixup.  And you're free to add fs-specific tests :)

(I'm not sure if we fall back by default to mkfs.$FSTYP and fsck.$FSTYP;
if so, it might just work)

-Eric

> Thanks!
> 
> Michael
> 
>>> ---
>>>  tests/generic/258 | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/generic/258 b/tests/generic/258
>>> index fb091ae..3aeb5ce 100755
>>> --- a/tests/generic/258
>>> +++ b/tests/generic/258
>>> @@ -62,7 +62,7 @@ fi
>>>  # unmount, remount, and check the timestamp
>>>  echo "Remounting to flush cache"
>>>  umount $TEST_DEV
>>> -mount $TEST_DEV $TEST_DIR
>>> +_test_mount
>>>  
>>>  # Should yield -315593940 (prior to epoch)
>>>  echo "Testing for negative seconds since epoch"
>>>
>>
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: generic/258 questions (mount issue)...

[Michael L. Semon]

On 05/22/2013 12:19 PM, Eric Sandeen wrote:
> On 5/22/13 11:15 AM, Michael L. Semon wrote:
>> On 05/22/2013 10:10 AM, Eric Sandeen wrote:
>>> On 5/21/13 10:03 PM, Michael L. Semon wrote:

>> One day, I would like to earn git-log fame by incorporating better 
>> support for JFS and especially NILFS2 into xfstests.  However, 
>> that's a topic for another day.  I know that I have uses for both 
>> filesystems, but that doesn't mean anybody else does.  Their 
>> mailing lists don't give much hint of user community or progress.
> 
> The generic tests hopefully work; if not, it should be a fairly
> simple fixup.  And you're free to add fs-specific tests :)
> 
> (I'm not sure if we fall back by default to mkfs.$FSTYP and fsck.$FSTYP;
> if so, it might just work)
> 
> -Eric

They work fine--JFS currently survives all of generic/* intact--but...

JFS: The syntax for a lot of things is very close to that of ReiserFS, 
ext3, and ext4 filesystems.  However, before just inserting "jfs" into 
the common/rc case switches, test results need to be audited for JFS.  
Additionally, I don't know if the current tests use external journals 
for non-XFS filesystems:  My first attempt stepped all over the XFS 
journal creation script, looked messy, and was probably buggy as well.

NILFS2: fsstress/fsx seems to slowly rip NILFS2 to bits, and I have 
to come up with a narrower test and report bugs to those guys.  
common/rc could be revised to better target filesystems with no 
fsck whatsoever, such as NILFS2 and F2FS.  I've been getting by with 
a simple `ln -s /bin/true /sbin/fsck.nilfs2`, and really, that is the 
best long-term solution from an administrative standpoint.

So you see how this might be an easy fix but still take up hours in 
debugging...

Thanks again!

Michael

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: generic/258 questions (mount issue)...

[Eric Sandeen]

On 5/22/13 11:48 AM, Michael L. Semon wrote:
> On 05/22/2013 12:19 PM, Eric Sandeen wrote:
>> On 5/22/13 11:15 AM, Michael L. Semon wrote:
>>> On 05/22/2013 10:10 AM, Eric Sandeen wrote:
>>>> On 5/21/13 10:03 PM, Michael L. Semon wrote:
> 
>>> One day, I would like to earn git-log fame by incorporating better 
>>> support for JFS and especially NILFS2 into xfstests.  However, 
>>> that's a topic for another day.  I know that I have uses for both 
>>> filesystems, but that doesn't mean anybody else does.  Their 
>>> mailing lists don't give much hint of user community or progress.
>>
>> The generic tests hopefully work; if not, it should be a fairly
>> simple fixup.  And you're free to add fs-specific tests :)
>>
>> (I'm not sure if we fall back by default to mkfs.$FSTYP and fsck.$FSTYP;
>> if so, it might just work)
>>
>> -Eric
> 
> They work fine--JFS currently survives all of generic/* intact--but...
> 
> JFS: The syntax for a lot of things is very close to that of ReiserFS, 
> ext3, and ext4 filesystems.  However, before just inserting "jfs" into 
> the common/rc case switches, test results need to be audited for JFS.  
> Additionally, I don't know if the current tests use external journals 
> for non-XFS filesystems:  My first attempt stepped all over the XFS 
> journal creation script, looked messy, and was probably buggy as well.

xfs doesn't use external journal by default, it needs to have an extra
couple env. vars set (I think README covers this)

To test JFS that way it'd need more tweaking I suppose.  It could
be done, certainly.

> NILFS2: fsstress/fsx seems to slowly rip NILFS2 to bits, and I have 
> to come up with a narrower test and report bugs to those guys.

:)  not too surprising.  (first time I looked at ecryptfs vs.
fsx, fsx won the fight in about 3 ops.  Much better now.)

I'd just use fsx directly to narrow down failures to the smallest
set of operations.

> common/rc could be revised to better target filesystems with no 
> fsck whatsoever, such as NILFS2 and F2FS.  I've been getting by with 
> a simple `ln -s /bin/true /sbin/fsck.nilfs2`, and really, that is the 
> best long-term solution from an administrative standpoint.

Yeah I'd suggest that maybe nilfs2-utils should just ship something like that.

> So you see how this might be an easy fix but still take up hours in 
> debugging...

Yup.

-Eric

> Thanks again!
> 
> Michael
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: generic/258 questions (mount issue)...

[Rich Johnston]

On 05/21/2013 10:03 PM, Michael L. Semon wrote:

>
> This is my first attempt at doing a `git format-patch --stdout`.
> Hopefully, this is somewhat more presentable to git.  Comments and
> brow-beating are welcome.
>
> Thanks!
>
> Michael

Thanks for the patch Michael, it has been committed:

commit f4ba26dcdc64a206d73d47cf054ecb2742867c55
Author: Michael L. Semon <mlsemon35@gmail.com>
Date:   Wed May 22 13:50:59 2013 -0500

     xfstests: Change mount method for generic/258

The only other suggestion would be to make your email subject match the 
patch name and starting with which part of xfs it is submitted for 
(xfs:, xfstests:, xfsprogs:, xfsdump:, xfsdmapi:. etc) i.e.

Subject: [PATCH]  xfstests: Change mount method for generic/258

--Rich

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: generic/258 questions (mount issue)...

[Michael L. Semon]

On 05/21/2013 09:26 PM, Dave Chinner wrote:
> On Tue, May 21, 2013 at 08:43:13PM -0400, Michael L. Semon wrote:
>> Hi!  When using xfstests generic/258 with along with $TEST_RTDEV
>> $TEST_LOGDEV, it tends to scream bloody murder about corrupted
>> partitions and such.  In fact, the commands in the test seem to do
>> the right thing when executed by hand.  So once again, I grasped for
>> straws and came up with this:
>>
>> --- xfstests/tests/generic/258.orig	2013-05-21 20:19:38.430754829 -0400
>> +++ xfstests/tests/generic/258	2013-05-21 20:10:11.509021368 -0400
>> @@ -62,7 +62,7 @@
>>  # unmount, remount, and check the timestamp
>>  echo "Remounting to flush cache"
>>  umount $TEST_DEV
>> -mount $TEST_DEV $TEST_DIR
>> +_test_mount
>>
>>  # Should yield -315593940 (prior to epoch)
>>  echo "Testing for negative seconds since epoch"
>>
>> My questions are these:
>>
>> 1) Was there a better way to do this?
> 
> No, your change is correct. Can you clean up the description of the
> problem you had and add a Signed-off-by?
> 
>> 2) Not knowing the policy on umounting $TEST_DEV, could this have
>> been a test for $SCRATCH_DEV?
> 
> There are a handful of other tests that also unmount the TEST_DEV.
> Perhaps adding a _test_umount() wrapper to common/rc (similar to
> _scratch_umount) would be best. At least shared/243 needs the same
> _test_mount treatment as this test.
> 
> Cheers,
> 
> Dave.
> 

I have a simple _test_mount change in shared/243, tested and ready to 
post.  However, it may work the same either way.  AFAIK, the test seems 
to go like this:

xfs_io this;
if ( ext4 )
{
  umount;
  do some unmounted ext4 stuff;
  mount -t ${FSTYP} ...;
}
xfs_io that;
if ( ext4 )
{
  ...
}
...

So if it isn't ext4 being tested, $TEST_DEV is not umounted at all.
Until I got a closer look at this, it was a surprise to see the 
XFS+rtdev+logdev pass without changes.

The rest of your comments will be followed when I get home.  To use the 
external logdev means a 20%-30% improvement in some test results on XFS.
To get the other file systems to do this will certainly have me looking
at the $TEST_DEV mount and umount code anyway.

Thanks!

Michael

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] xfstests: Change mount method for shared/243

[Michael L. Semon]

Hi!  This patch updates the mount method in xfstests shared/243 
without seeming to change the behavior of the test.  Should this 
patch get a NACK for not being useful, no worries!  There will be 
other opportunities.

The patch was tested with ext4, with and without external journals. 
It was also tested on XFS with an external logdev.  The MOUNT_OPTIONS 
for the ext4 tests included '-o barrier=1' and '-o barrier=0'.  The 
test results varied on ext4 with an external journal, but the 
results shown by /proc/mounts was OK.  [barrier=1 causes a failed 
test, barrier=0 succeeds, both in the original and patched test.]

>From 8762f621a5fb9556fa96ec289bceffc4fcc844f3 Mon Sep 17 00:00:00 2001
From: "Michael L. Semon" <mlsemon35@gmail.com>
Date: Wed, 22 May 2013 20:57:56 -0400
Subject: [PATCH] xfstests: Change mount method for shared/243

Change the mount invocations in shared/243 to use the built-in
_test_mount function of xfstests.  This was done as part of a
clean-up recommendation.  Currently, only ext4 file systems are
affected by this change.

Signed-off-by: Michael L. Semon <mlsemon35@gmail.com>
---
 tests/shared/243 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/shared/243 b/tests/shared/243
index e0f2db2..b3eb025 100755
--- a/tests/shared/243
+++ b/tests/shared/243
@@ -108,7 +108,7 @@ _check_ext4_eof_flag()
     fi
 
     # Mount the ${TEST_DEV}
-    mount ${TEST_DEV} -t ${FSTYP} ${TEST_DIR}
+    _test_mount
   fi
 }
 
-- 
1.8.2


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs