xfstests: add _require_freeze and minor cleanups

PATCH In review Eric Sandeen 3 participants 7 messages Sep 19, 2012 – Sep 25, 2012

Share & tools

Eric Sandeen

author

2012-09-19 22:52 UTC #0 lore raw

PATCH 1/1 xfstests: add _require_freeze and minor cleanups

More filesystems have grown freeze capability, so rather than hardcoding several in _supported_fs, make tests 068 and 280 generic and then add a new _require_freeze() which checks whether the fs under test can be frozen before beginning the test.

Minor other cleanups to 280:
- remove extra _supported_fs line
- clear $seq.full before beginning

Signed-off-by Eric Sandeen <[email protected]>

---

068 +2 -1

index b595d1d..617420c 100755
@@ -51,10 +51,11 @@ trap "_cleanup" 0 1 2 3 15
 . ./common.filter
 
 # real QA test starts here
-_supported_fs btrfs ext3 ext4 xfs
+_supported_fs generic
 _supported_os Linux IRIX
 
 _require_scratch
+_require_freeze
 
 echo "*** init FS"

280 +4 -2

index 55849ed..5e26173 100755
@@ -45,13 +45,15 @@ _cleanup()
 
 _require_scratch
 _require_quota
+_require_freeze
 
 # real QA test starts here
 
 # Modify as appropriate.
-_supported_fs generic
 _supported_os Linux
-_supported_fs ext3 ext4 xfs
+_supported_fs generic
+
+rm -f $seq.full
 
 umount $SCRATCH_DEV 2>/dev/null
 _scratch_mkfs >> $seq.full 2>&1

common.rc +9 -0

index 602513a..0e8a306 100644
@@ -1758,6 +1758,15 @@ _require_btrfs()
 	[ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd)"
 }
 
+# Does freeze work on this fs?
+_require_freeze()
+{
+	xfs_freeze -f "$TEST_DIR" >/dev/null 2>&1
+	result=$? 
+	xfs_freeze -u "$TEST_DIR" >/dev/null 2>&1
+	[ $result -eq 0 ] || _notrun "$FSTYP does not support freezing"
+}
+
 # arg 1 is dev to remove and is output of the below eg.
 # ls -l /sys/class/block/sdd | rev | cut -d "/" -f 3 | rev
 _devmgt_remove()

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Ben Myers

2012-09-21 16:37 UTC #1 lore raw

Hey Eric,

Quoted 58 lines · Eric Sandeen

More filesystems have grown freeze capability, so rather than hardcoding several in _supported_fs, make tests 068 and 280 generic and then add a new _require_freeze() which checks whether the fs under test can be frozen before beginning the test.
Minor other cleanups to 280:
- remove extra _supported_fs line
- clear $seq.full before beginning
Signed-off-by Eric Sandeen <[email protected]>
---
068 +2 -1
index b595d1d..617420c 100755
@@ -51,10 +51,11 @@ trap "_cleanup" 0 1 2 3 15
 . ./common.filter
 
 # real QA test starts here
-_supported_fs btrfs ext3 ext4 xfs
+_supported_fs generic
 _supported_os Linux IRIX
 
 _require_scratch
+_require_freeze
 
 echo "*** init FS"
 
280 +4 -2
index 55849ed..5e26173 100755
@@ -45,13 +45,15 @@ _cleanup()
 
 _require_scratch
 _require_quota
+_require_freeze
 
 # real QA test starts here
 
 # Modify as appropriate.
-_supported_fs generic
 _supported_os Linux
-_supported_fs ext3 ext4 xfs
+_supported_fs generic
+
+rm -f $seq.full
 
 umount $SCRATCH_DEV 2>/dev/null
 _scratch_mkfs >> $seq.full 2>&1
common.rc +0 -0
index 602513a..0e8a306 100644
@@ -1758,6 +1758,15 @@ _require_btrfs()
 	[ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd)"
 }

common.rc _require_btrfs() · line 1769

Quoted 10 lines

common.rc +9 -0

 
+# Does freeze work on this fs?
+_require_freeze()
+{
+	xfs_freeze -f "$TEST_DIR" >/dev/null 2>&1
+	result=$? 
+	xfs_freeze -u "$TEST_DIR" >/dev/null 2>&1
+	[ $result -eq 0 ] || _notrun "$FSTYP does not support freezing"
+}
+

Pretty good idea to generalize _require_freeze. It looks like xfs_freeze is a script that uses xfs_io which uses xfsctl XFS_IOC_FREEZE. So isn't what you have here xfs specific? It wouldn't work for the other filesystems that implement s_op.freeze_fs:

1 1502 btrfs/super.c <<GLOBAL>>
.freeze_fs = btrfs_freeze,
2 804 ext3/super.c <<GLOBAL>>
.freeze_fs = ext3_freeze,
3 1238 ext4/super.c <<GLOBAL>>
.freeze_fs = ext4_freeze,
4 1578 gfs2/super.c <<GLOBAL>>
.freeze_fs = gfs2_freeze,
5 760 jfs/super.c <<GLOBAL>>
.freeze_fs = jfs_freeze,
6 688 nilfs2/super.c <<GLOBAL>>
.freeze_fs = nilfs_freeze,
7 620 reiserfs/super.c <<GLOBAL>>
.freeze_fs = reiserfs_freeze,
8 1536 xfs/linux-2.6/xfs_super.c <<GLOBAL>>
.freeze_fs = xfs_fs_freeze,

Maybe it would be better if we had some kind of interface to test whether .freeze_fs is defined (if there isn't one already) rather than freeze and thaw to find out. Oddly freeze_super seems to just return 0 when .freeze_fs is not defined.

Regards,
Ben

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Eric Sandeen

author

2012-09-21 16:46 UTC #2 lore raw

Quoted 62 lines · Ben Myers

Hey Eric,
Quoted 59 lines · Eric Sandeen
More filesystems have grown freeze capability, so rather than hardcoding several in _supported_fs, make tests 068 and 280 generic and then add a new _require_freeze() which checks whether the fs under test can be frozen before beginning the test.
Minor other cleanups to 280:
- remove extra _supported_fs line
- clear $seq.full before beginning
Signed-off-by Eric Sandeen <[email protected]>
---
068 +2 -1
index b595d1d..617420c 100755
@@ -51,10 +51,11 @@ trap "_cleanup" 0 1 2 3 15
 . ./common.filter
 
 # real QA test starts here
-_supported_fs btrfs ext3 ext4 xfs
+_supported_fs generic
 _supported_os Linux IRIX
 
 _require_scratch
+_require_freeze
 
 echo "*** init FS"
 
280 +4 -2
index 55849ed..5e26173 100755
@@ -45,13 +45,15 @@ _cleanup()
 
 _require_scratch
 _require_quota
+_require_freeze
 
 # real QA test starts here
 
 # Modify as appropriate.
-_supported_fs generic
 _supported_os Linux
-_supported_fs ext3 ext4 xfs
+_supported_fs generic
+
+rm -f $seq.full
 
 umount $SCRATCH_DEV 2>/dev/null
 _scratch_mkfs >> $seq.full 2>&1
common.rc +0 -0
index 602513a..0e8a306 100644
@@ -1758,6 +1758,15 @@ _require_btrfs()
 	[ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd)"
 }
 

Quoted 14 lines

common.rc _require_btrfs() · line 1769
Quoted 9 lines
common.rc +9 -0
+# Does freeze work on this fs?
+_require_freeze()
+{
+	xfs_freeze -f "$TEST_DIR" >/dev/null 2>&1
+	result=$? 
+	xfs_freeze -u "$TEST_DIR" >/dev/null 2>&1
+	[ $result -eq 0 ] || _notrun "$FSTYP does not support freezing"
+}
+
Pretty good idea to generalize _require_freeze. It looks like xfs_freeze is a script that uses xfs_io which uses xfsctl XFS_IOC_FREEZE. So isn't what you have here xfs specific? It wouldn't work for the other filesystems that implement s_op.freeze_fs:

It got elevated to a generic ioctl:

fs/xfs/xfs_fs.h:

/*      XFS_IOC_FREEZE            -- FIFREEZE   119      */
/*      XFS_IOC_THAW              -- FITHAW     120      */

to:

include/linux/fs.h:

#define FIFREEZE        _IOWR('X', 119, int)    /* Freeze */
#define FITHAW          _IOWR('X', 120, int)    /* Thaw */

-Eric

Quoted 17 lines

1 1502 btrfs/super.c <<GLOBAL>>
.freeze_fs = btrfs_freeze,
2 804 ext3/super.c <<GLOBAL>>
.freeze_fs = ext3_freeze,
3 1238 ext4/super.c <<GLOBAL>>
.freeze_fs = ext4_freeze,
4 1578 gfs2/super.c <<GLOBAL>>
.freeze_fs = gfs2_freeze,
5 760 jfs/super.c <<GLOBAL>>
.freeze_fs = jfs_freeze,
6 688 nilfs2/super.c <<GLOBAL>>
.freeze_fs = nilfs_freeze,
7 620 reiserfs/super.c <<GLOBAL>>
.freeze_fs = reiserfs_freeze,
8 1536 xfs/linux-2.6/xfs_super.c <<GLOBAL>>
.freeze_fs = xfs_fs_freeze,

Quoted 8 lines

Maybe it would be better if we had some kind of interface to test whether .freeze_fs is defined (if there isn't one already) rather than freeze and thaw to find out. Oddly freeze_super seems to just return 0 when .freeze_fs is not defined.
Regards,
Ben

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Ben Myers

Reviewed-by

2012-09-21 19:58 UTC #3 lore raw

Quoted 81 lines · Eric Sandeen

Quoted 62 lines · Ben Myers
Hey Eric,
Quoted 59 lines · Eric Sandeen
More filesystems have grown freeze capability, so rather than hardcoding several in _supported_fs, make tests 068 and 280 generic and then add a new _require_freeze() which checks whether the fs under test can be frozen before beginning the test.
Minor other cleanups to 280:
- remove extra _supported_fs line
- clear $seq.full before beginning
Signed-off-by Eric Sandeen <[email protected]>
---
068 +2 -1
index b595d1d..617420c 100755
@@ -51,10 +51,11 @@ trap "_cleanup" 0 1 2 3 15
 . ./common.filter
 
 # real QA test starts here
-_supported_fs btrfs ext3 ext4 xfs
+_supported_fs generic
 _supported_os Linux IRIX
 
 _require_scratch
+_require_freeze
 
 echo "*** init FS"
 
280 +4 -2
index 55849ed..5e26173 100755
@@ -45,13 +45,15 @@ _cleanup()
 
 _require_scratch
 _require_quota
+_require_freeze
 
 # real QA test starts here
 
 # Modify as appropriate.
-_supported_fs generic
 _supported_os Linux
-_supported_fs ext3 ext4 xfs
+_supported_fs generic
+
+rm -f $seq.full
 
 umount $SCRATCH_DEV 2>/dev/null
 _scratch_mkfs >> $seq.full 2>&1
common.rc +0 -0
index 602513a..0e8a306 100644
@@ -1758,6 +1758,15 @@ _require_btrfs()
 	[ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd)"
 }
 
Quoted 14 lines
common.rc _require_btrfs() · line 1769
Quoted 9 lines
common.rc +9 -0
+# Does freeze work on this fs?
+_require_freeze()
+{
+	xfs_freeze -f "$TEST_DIR" >/dev/null 2>&1
+	result=$? 
+	xfs_freeze -u "$TEST_DIR" >/dev/null 2>&1
+	[ $result -eq 0 ] || _notrun "$FSTYP does not support freezing"
+}
+
Pretty good idea to generalize _require_freeze. It looks like xfs_freeze is a script that uses xfs_io which uses xfsctl XFS_IOC_FREEZE. So isn't what you have here xfs specific? It wouldn't work for the other filesystems that implement s_op.freeze_fs:
It got elevated to a generic ioctl:
fs/xfs/xfs_fs.h:

Quoted 8 lines

/*      XFS_IOC_FREEZE            -- FIFREEZE   119      */
/*      XFS_IOC_THAW              -- FITHAW     120      */

to:

include/linux/fs.h:

#define FIFREEZE        _IOWR('X', 119, int)    /* Freeze */
#define FITHAW          _IOWR('X', 120, int)    /* Thaw */

Ah, great. I see it now. It looks like test 119 is also using freeze. Should that one also _require_freeze?

Reviewed-by Ben Myers <[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Eric Sandeen

author

2012-09-21 20:19 UTC #4 lore raw

Quoted 4 lines · Ben Myers

Quoted 3 lines · Eric Sandeen
Quoted 2 lines · Ben Myers
Hey Eric,

...

Quoted 11 lines

Quoted 11 lines
Quoted 4 lines
Pretty good idea to generalize _require_freeze. It looks like xfs_freeze is a script that uses xfs_io which uses xfsctl XFS_IOC_FREEZE. So isn't what you have here xfs specific? It wouldn't work for the other filesystems that implement s_op.freeze_fs:
It got elevated to a generic ioctl:
fs/xfs/xfs_fs.h:
/*      XFS_IOC_FREEZE            -- FIFREEZE   119      */
/*      XFS_IOC_THAW              -- FITHAW     120      */

Quoted 8 lines

Quoted 5 lines
to:
include/linux/fs.h:
#define FIFREEZE        _IOWR('X', 119, int)    /* Freeze */
#define FITHAW          _IOWR('X', 120, int)    /* Thaw */
Ah, great. I see it now. It looks like test 119 is also using freeze. Should that one also _require_freeze?

Since it's xfs-specific, I didn't think it was necessary, but it could.

-Eric

Quoted 2 lines

Reviewed-by Ben Myers <[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

Reviewed-by on v1 1/1

2012-09-25 09:32 UTC #5 lore raw

In reply to Eric Sandeen

Looks good,

Reviewed-by Christoph Hellwig <[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Ben Myers

2012-09-25 15:27 UTC #6 lore raw

In reply to Eric Sandeen

Quoted 10 lines · Eric Sandeen

More filesystems have grown freeze capability, so rather than hardcoding several in _supported_fs, make tests 068 and 280 generic and then add a new _require_freeze() which checks whether the fs under test can be frozen before beginning the test.
Minor other cleanups to 280:
- remove extra _supported_fs line
- clear $seq.full before beginning
Signed-off-by Eric Sandeen <[email protected]>

committed to git://oss.sgi.com/xfs/cmds/xfstests.git, master branch.

Christoph, sorry I missed your reviewed-by.

-Ben

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs