diff mbox

[v2,1/5] common: re-enable tests that require scratch dev on NFS

Message ID 1414775040-4051-2-git-send-email-eguan@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eryu Guan Oct. 31, 2014, 5:03 p.m. UTC
This commit disables tests requires scratch dev running on NFS

c041421 xfstests: stop special casing nfs and udf

Now re-enable them to get a larger test coverage on NFS.

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

Comments

Dave Chinner Nov. 10, 2014, 2:12 a.m. UTC | #1
On Sat, Nov 01, 2014 at 01:03:56AM +0800, Eryu Guan wrote:
> This commit disables tests requires scratch dev running on NFS
> 
> c041421 xfstests: stop special casing nfs and udf
> 
> Now re-enable them to get a larger test coverage on NFS.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>

I'll commit this as is, but can you send another patch to wire this
same functionality up for CIFS?

Cheers,

Dave.
Eryu Guan Nov. 10, 2014, 4:05 a.m. UTC | #2
On Mon, Nov 10, 2014 at 01:12:48PM +1100, Dave Chinner wrote:
> On Sat, Nov 01, 2014 at 01:03:56AM +0800, Eryu Guan wrote:
> > This commit disables tests requires scratch dev running on NFS
> > 
> > c041421 xfstests: stop special casing nfs and udf
> > 
> > Now re-enable them to get a larger test coverage on NFS.
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> 
> I'll commit this as is, but can you send another patch to wire this
> same functionality up for CIFS?

Sure, will do that.

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
Steve French Nov. 12, 2014, 6:36 p.m. UTC | #3
There should be a check to make sure SCRATCH_MNT exists before you
wipe the whole disk ....

+# remove all files in $SCRATCH_MNT, useful when testing on NFS/CIFS
+_scratch_cleanup_files()
+{
+       _scratch_mount
+       rm -rf $SCRATCH_MNT/*
+       _scratch_unmount
+}
+
so if no SCRATCH_MNT then this does rm -rf/*
right ... (and wipes out your whole system ...)

On Fri, Oct 31, 2014 at 12:03 PM, Eryu Guan <eguan@redhat.com> wrote:
> This commit disables tests requires scratch dev running on NFS
>
> c041421 xfstests: stop special casing nfs and udf
>
> Now re-enable them to get a larger test coverage on NFS.
>
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
>  common/rc | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 747cf72..ae03712 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -551,6 +551,14 @@ _mkfs_dev()
>      rm -f $tmp_dir.mkfserr $tmp_dir.mkfsstd
>  }
>
> +# remove all files in $SCRATCH_MNT, useful when testing on NFS/CIFS
> +_scratch_cleanup_files()
> +{
> +       _scratch_mount
> +       rm -rf $SCRATCH_MNT/*
> +       _scratch_unmount
> +}
> +
>  _scratch_mkfs()
>  {
>      case $FSTYP in
> @@ -558,7 +566,9 @@ _scratch_mkfs()
>          _scratch_mkfs_xfs $*
>         ;;
>      nfs*)
> -       # do nothing for nfs
> +       # unable to re-create NFS, just remove all files in $SCRATCH_MNT to
> +       # avoid EEXIST caused by the leftover files created in previous runs
> +        _scratch_cleanup_files
>         ;;
>      cifs)
>         # do nothing for cifs
> @@ -1032,8 +1042,14 @@ _require_scratch_nocheck()
>  {
>      case "$FSTYP" in
>         nfs*)
> -                 _notrun "requires a scratch device"
> -                ;;
> +               echo $SCRATCH_DEV | grep -q ":/" > /dev/null 2>&1
> +               if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then
> +                       _notrun "this test requires a valid \$SCRATCH_DEV"
> +               fi
> +               if [ ! -d "$SCRATCH_MNT" ]; then
> +                       _notrun "this test requires a valid \$SCRATCH_MNT"
> +               fi
> +               ;;
>         cifs)
>                 _notrun "requires a scratch device"
>                 ;;
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Chinner Nov. 13, 2014, 3:33 a.m. UTC | #4
On Wed, Nov 12, 2014 at 12:36:13PM -0600, Steve French wrote:
> On Fri, Oct 31, 2014 at 12:03 PM, Eryu Guan <eguan@redhat.com> wrote:
> > This commit disables tests requires scratch dev running on NFS
> >
> > c041421 xfstests: stop special casing nfs and udf
> >
> > Now re-enable them to get a larger test coverage on NFS.
> >
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> >  common/rc | 22 +++++++++++++++++++---
> >  1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/common/rc b/common/rc
> > index 747cf72..ae03712 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -551,6 +551,14 @@ _mkfs_dev()
> >      rm -f $tmp_dir.mkfserr $tmp_dir.mkfsstd
> >  }
> >
> > +# remove all files in $SCRATCH_MNT, useful when testing on NFS/CIFS
> > +_scratch_cleanup_files()
> > +{
> > +       _scratch_mount
> > +       rm -rf $SCRATCH_MNT/*
> > +       _scratch_unmount
> > +}
>
> There should be a check to make sure SCRATCH_MNT exists before you
> wipe the whole disk ....
> 
> so if no SCRATCH_MNT then this does rm -rf/*
> right ... (and wipes out your whole system ...)

You can't get to that function until after all the checks that
SCRATCH_MNT exists. i.e. this happens during _scratch_mkfs, and that
is only called in tests after all the startup checks validate
devices and mounts exist. i.e. see common/config::get_next_config()

Cheers,

Dave.
diff mbox

Patch

diff --git a/common/rc b/common/rc
index 747cf72..ae03712 100644
--- a/common/rc
+++ b/common/rc
@@ -551,6 +551,14 @@  _mkfs_dev()
     rm -f $tmp_dir.mkfserr $tmp_dir.mkfsstd
 }
 
+# remove all files in $SCRATCH_MNT, useful when testing on NFS/CIFS
+_scratch_cleanup_files()
+{
+	_scratch_mount
+	rm -rf $SCRATCH_MNT/*
+	_scratch_unmount
+}
+
 _scratch_mkfs()
 {
     case $FSTYP in
@@ -558,7 +566,9 @@  _scratch_mkfs()
         _scratch_mkfs_xfs $*
 	;;
     nfs*)
-	# do nothing for nfs
+	# unable to re-create NFS, just remove all files in $SCRATCH_MNT to
+	# avoid EEXIST caused by the leftover files created in previous runs
+        _scratch_cleanup_files
 	;;
     cifs)
 	# do nothing for cifs
@@ -1032,8 +1042,14 @@  _require_scratch_nocheck()
 {
     case "$FSTYP" in
 	nfs*)
-                 _notrun "requires a scratch device"
-		 ;;
+		echo $SCRATCH_DEV | grep -q ":/" > /dev/null 2>&1
+		if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then
+			_notrun "this test requires a valid \$SCRATCH_DEV"
+		fi
+		if [ ! -d "$SCRATCH_MNT" ]; then
+			_notrun "this test requires a valid \$SCRATCH_MNT"
+		fi
+		;;
 	cifs)
 		_notrun "requires a scratch device"
 		;;