diff mbox

generic/247: unmount test device, not scratch device

Message ID 20170705203754.92140-1-ebiggers3@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Biggers July 5, 2017, 8:37 p.m. UTC
From: Eric Biggers <ebiggers@google.com>

Commit d55123c080cb ("generic/247: filter out expected XFS warnings for
mixed mmap/direct I/O") made generic/247 start explicitly unmounting the
scratch device.  However, the test doesn't actually use the scratch
device, so it will fail if SCRATCH_DEV is undefined.  Fix it by
unmounting the test device instead.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 tests/generic/247 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Eryu Guan July 6, 2017, 2:50 a.m. UTC | #1
On Wed, Jul 05, 2017 at 01:37:54PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Commit d55123c080cb ("generic/247: filter out expected XFS warnings for
> mixed mmap/direct I/O") made generic/247 start explicitly unmounting the
> scratch device.  However, the test doesn't actually use the scratch
> device, so it will fail if SCRATCH_DEV is undefined.  Fix it by
> unmounting the test device instead.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks for the fix! I found this in my review and I remembered I removed
the _scratch_unmount and updated the comments accordingly at commit
time, but clearly I didn't.

Thanks,
Eryu

> ---
>  tests/generic/247 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/247 b/tests/generic/247
> index 509e8808..aac70780 100755
> --- a/tests/generic/247
> +++ b/tests/generic/247
> @@ -82,7 +82,7 @@ echo "Silence is golden."
>  
>  # unmount and check dmesg, filtering out expected XFS warnings about mixed
>  # mmap/dio
> -_scratch_unmount
> +_test_unmount
>  if [ "$FSTYP" == "xfs" ]; then
>  	_check_dmesg _filter_xfs_dmesg
>  else
> -- 
> 2.13.2.725.g09c95d1e9-goog
> 
> --
> 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
--
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
Eric Biggers July 6, 2017, 2:54 a.m. UTC | #2
On Thu, Jul 06, 2017 at 10:50:59AM +0800, Eryu Guan wrote:
> On Wed, Jul 05, 2017 at 01:37:54PM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Commit d55123c080cb ("generic/247: filter out expected XFS warnings for
> > mixed mmap/direct I/O") made generic/247 start explicitly unmounting the
> > scratch device.  However, the test doesn't actually use the scratch
> > device, so it will fail if SCRATCH_DEV is undefined.  Fix it by
> > unmounting the test device instead.
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> Thanks for the fix! I found this in my review and I remembered I removed
> the _scratch_unmount and updated the comments accordingly at commit
> time, but clearly I didn't.
> 

It was removed from generic/095 (maybe by accident?) but not generic/247.
Perhaps it needs to be added back to generic/095?

Eric
--
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
Eryu Guan July 6, 2017, 3:10 a.m. UTC | #3
On Wed, Jul 05, 2017 at 07:54:28PM -0700, Eric Biggers wrote:
> On Thu, Jul 06, 2017 at 10:50:59AM +0800, Eryu Guan wrote:
> > On Wed, Jul 05, 2017 at 01:37:54PM -0700, Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@google.com>
> > > 
> > > Commit d55123c080cb ("generic/247: filter out expected XFS warnings for
> > > mixed mmap/direct I/O") made generic/247 start explicitly unmounting the
> > > scratch device.  However, the test doesn't actually use the scratch
> > > device, so it will fail if SCRATCH_DEV is undefined.  Fix it by
> > > unmounting the test device instead.
> > > 
> > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > 
> > Thanks for the fix! I found this in my review and I remembered I removed
> > the _scratch_unmount and updated the comments accordingly at commit
> > time, but clearly I didn't.
> > 
> 
> It was removed from generic/095 (maybe by accident?) but not generic/247.
> Perhaps it needs to be added back to generic/095?

You're right, I updated the wrong test.. Yeah, it should be added back
to generic/095. Would you mind sending a v2 patch to do these edits all
together? i.e. add _scratch_unmount (and comments) back to generic/095
and remove them from generic/247?

Thanks,
Eryu
--
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
diff mbox

Patch

diff --git a/tests/generic/247 b/tests/generic/247
index 509e8808..aac70780 100755
--- a/tests/generic/247
+++ b/tests/generic/247
@@ -82,7 +82,7 @@  echo "Silence is golden."
 
 # unmount and check dmesg, filtering out expected XFS warnings about mixed
 # mmap/dio
-_scratch_unmount
+_test_unmount
 if [ "$FSTYP" == "xfs" ]; then
 	_check_dmesg _filter_xfs_dmesg
 else