fstests: add support for hfsplus

PATCH In review Ernesto A. Fernández 2 participants 5 messages Sep 25, 2018 – Sep 26, 2018

Share & tools

Ernesto A. Fernández

author

2018-09-25 03:48 UTC #0 lore raw

PATCH v2 1/1 fstests: add support for hfsplus

Running tests on hfsplus requires patched versions of the mkfs and fsck tools [1] that support filesystems smaller than the device.

[1] https://github.com/eafer/hfsprogs-linux.git

Signed-off-by Ernesto A. Fernández <[email protected]>

---
 common/config | 10 ++++++++++
 common/rc     | 15 ++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

common/config +10 -0

index 1ba8d96c..315f8b1e 100644
@@ -229,6 +229,7 @@ case "$HOSTOS" in
 	export MKFS_CIFS_PROG="false"
 	export MKFS_OVERLAY_PROG="false"
 	export MKFS_REISER4_PROG=$(type -P mkfs.reiser4)
+	export MKFS_HFSPLUS_PROG=$(type -P mkfs.hfsplus)
 	export E2FSCK_PROG=$(type -P e2fsck)
 	export TUNE2FS_PROG=$(type -P tune2fs)
 	export FSCK_OVERLAY_PROG=$(type -P fsck.overlay)

@@ -313,6 +314,9 @@ _mount_opts()
 	ubifs)
 		export MOUNT_OPTIONS=$UBIFS_MOUNT_OPTIONS
 		;;
+	hfsplus)
+		export MOUNT_OPTIONS=$HFSPLUS_MOUNT_OPTIONS
+		;;
 	*)
 		;;
 	esac

@@ -380,6 +384,9 @@ _mkfs_opts()
 	f2fs)
 		export MKFS_OPTIONS="$F2FS_MKFS_OPTIONS"
 		;;
+	hfsplus)
+		export MKFS_OPTIONS=$HFSPLUS_MKFS_OPTIONS
+		;;
 	*)
 		;;
 	esac

@@ -397,6 +404,9 @@ _fsck_opts()
 	f2fs)
 		export FSCK_OPTIONS=""
 		;;
+	hfsplus)
+		export FSCK_OPTIONS="-nX"
+		;;
 	*)
 		export FSCK_OPTIONS="-n"
 		;;

common/rc +14 -1

index d5bb1fee..a843e9f6 100644
@@ -158,6 +158,12 @@ case "$FSTYP" in
     ubifs)
 	[ "$UBIUPDATEVOL_PROG" = "" ] && _fatal "ubiupdatevol not found"
 	;;
+    hfsplus)
+	[ "$MKFS_HFSPLUS_PROG" = "" ] && _fatal "mkfs.hfsplus not found"
+	mkfs.hfsplus -X |& grep "invalid" &&
+		_fatal "A patched version of mkfs.hfsplus is required:" \
+		       "https://github.com/eafer/hfsprogs-linux.git"
+	;;
 esac
 
 if [ ! -z "$REPORT_LIST" ]; then

@@ -746,6 +752,10 @@ _scratch_mkfs()
 		mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --"
 		mkfs_filter="grep -v -e ^mkfs\.ocfs2"
 		;;
+	hfsplus)
+		mkfs_cmd="yes | $MKFS_PROG -t $FSTYP -- -X"
+		mkfs_filter="cat"
+		;;
 	*)
 		mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --"
 		mkfs_filter="cat"

@@ -995,6 +1005,9 @@ _scratch_mkfs_sized()
 	fi
 	export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS"
 	;;
+    hfsplus)
+	${MKFS_PROG}.$FSTYP $MKFS_OPTIONS -X $fssize -b $blocksize $SCRATCH_DEV
+	;;
     *)
 	_notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized"
 	;;

@@ -3055,7 +3068,7 @@ _require_metadata_journaling()
 	fi
 
 	case "$FSTYP" in
-	ext2|vfat|msdos|udf)
+	ext2|vfat|msdos|udf|hfsplus)
 		_notrun "$FSTYP does not support metadata journaling"
 		;;
 	ext4)

Signature · 1 line

2.11.0

Ernesto A. Fernández

author

2018-09-26 00:06 UTC #2 lore raw

Quoted 8 lines · Amir Goldstein

Quoted 3 lines · Ernesto A. Fernández
Running tests on hfsplus requires patched versions of the mkfs and fsck tools [1] that support filesystems smaller than the device.
That's not accurate, is it? Wouldn't it be more accurate to say that the patched tools are required to run the _scratch_mkfs_sized tests?

I set this up to call _fatal early if the tools are not the patched versions.

Using -X on mkfs.hfsplus forces the size to be a whole number of blocks, and using -X on fsck makes it work under that assumption. This is briefly explained in the patched man page for the tools.

It's a workaround needed because hfsplus only stores the number of allocation blocks in the volume header. If the device size is not a multiple of the block size, the backup header will be past the end, and fsck will take that as corruption.

Of course I could let the patched fsck use the -X flag by default. But that could be dangerous if somebody accidentally uses it to check an important filesystem.

Quoted 60 lines

[1] https://github.com/eafer/hfsprogs-linux.git

Signed-off-by Ernesto A. Fernández <[email protected]>

---
 common/config | 10 ++++++++++
 common/rc     | 15 ++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

common/config +10 -0

index 1ba8d96c..315f8b1e 100644
@@ -229,6 +229,7 @@ case "$HOSTOS" in
        export MKFS_CIFS_PROG="false"
        export MKFS_OVERLAY_PROG="false"
        export MKFS_REISER4_PROG=$(type -P mkfs.reiser4)
+       export MKFS_HFSPLUS_PROG=$(type -P mkfs.hfsplus)
        export E2FSCK_PROG=$(type -P e2fsck)
        export TUNE2FS_PROG=$(type -P tune2fs)
        export FSCK_OVERLAY_PROG=$(type -P fsck.overlay)

@@ -313,6 +314,9 @@ _mount_opts()
        ubifs)
                export MOUNT_OPTIONS=$UBIFS_MOUNT_OPTIONS
                ;;
+       hfsplus)
+               export MOUNT_OPTIONS=$HFSPLUS_MOUNT_OPTIONS
+               ;;
        *)
                ;;
        esac

@@ -380,6 +384,9 @@ _mkfs_opts()
        f2fs)
                export MKFS_OPTIONS="$F2FS_MKFS_OPTIONS"
                ;;
+       hfsplus)
+               export MKFS_OPTIONS=$HFSPLUS_MKFS_OPTIONS
+               ;;
        *)
                ;;
        esac

@@ -397,6 +404,9 @@ _fsck_opts()
        f2fs)
                export FSCK_OPTIONS=""
                ;;
+       hfsplus)
+               export FSCK_OPTIONS="-nX"
+               ;;
        *)
                export FSCK_OPTIONS="-n"
                ;;

common/rc +0 -0

index d5bb1fee..a843e9f6 100644
@@ -158,6 +158,12 @@ case "$FSTYP" in
     ubifs)
        [ "$UBIUPDATEVOL_PROG" = "" ] && _fatal "ubiupdatevol not found"
        ;;

Quoted 8 lines

common/rc case "$FSTYP" in · line 165
Quoted 5 lines
common/rc +5 -0
+    hfsplus)
+       [ "$MKFS_HFSPLUS_PROG" = "" ] && _fatal "mkfs.hfsplus not found"
+       mkfs.hfsplus -X |& grep "invalid" &&
+               _fatal "A patched version of mkfs.hfsplus is required:" \
+                      "https://github.com/eafer/hfsprogs-linux.git"
Why fatal and not just notrun the _scratch_mkfs_sized tests if no -X support? Must all tests run with mkfs -X and fsck -X??

I always use -X on mkfs so the tests will complain if the size of the device is not a whole number of blocks. Otherwise the tests would later fail on fsck.

Maybe I should check for -X support when setting the FSCK_OPTIONS, and in _scratch_mkfs()? Then _fatal would not be necessary.

Quoted 4 lines

Quoted 4 lines
common/rc +1 -0
+       ;;
 esac
 

Quoted 9 lines

common/rc _scratch_mkfs() · line 756

Quoted 7 lines

common/rc +2 -0

 if [ ! -z "$REPORT_LIST" ]; then

@@ -746,6 +752,10 @@ _scratch_mkfs()
                mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --"
                mkfs_filter="grep -v -e ^mkfs\.ocfs2"
                ;;
+       hfsplus)
+               mkfs_cmd="yes | $MKFS_PROG -t $FSTYP -- -X"

-X here seems like a typo?

This is so the tests will complain if the device size is not a multiple of the block size. I think I explained it in the patched man page for mkfs.hfsplus, but it may be better to put a comment here.

Thanks,
Ernest

Quoted 3 lines

Thanks,
Amir.

Amir Goldstein

2018-09-26 00:20 UTC #3 lore raw

Quoted 96 lines · Ernesto A. Fernández

Quoted 8 lines · Amir Goldstein
Quoted 3 lines · Ernesto A. Fernández
Running tests on hfsplus requires patched versions of the mkfs and fsck tools [1] that support filesystems smaller than the device.
That's not accurate, is it? Wouldn't it be more accurate to say that the patched tools are required to run the _scratch_mkfs_sized tests?
I set this up to call _fatal early if the tools are not the patched versions.
Using -X on mkfs.hfsplus forces the size to be a whole number of blocks, and using -X on fsck makes it work under that assumption. This is briefly explained in the patched man page for the tools.
It's a workaround needed because hfsplus only stores the number of allocation blocks in the volume header. If the device size is not a multiple of the block size, the backup header will be past the end, and fsck will take that as corruption.
Of course I could let the patched fsck use the -X flag by default. But that could be dangerous if somebody accidentally uses it to check an important filesystem.
Quoted 60 lines
Quoted 60 lines
[1] https://github.com/eafer/hfsprogs-linux.git
Signed-off-by Ernesto A. Fernández <[email protected]>
---
 common/config | 10 ++++++++++
 common/rc     | 15 ++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)
common/config +10 -0
index 1ba8d96c..315f8b1e 100644
@@ -229,6 +229,7 @@ case "$HOSTOS" in
        export MKFS_CIFS_PROG="false"
        export MKFS_OVERLAY_PROG="false"
        export MKFS_REISER4_PROG=$(type -P mkfs.reiser4)
+       export MKFS_HFSPLUS_PROG=$(type -P mkfs.hfsplus)
        export E2FSCK_PROG=$(type -P e2fsck)
        export TUNE2FS_PROG=$(type -P tune2fs)
        export FSCK_OVERLAY_PROG=$(type -P fsck.overlay)
@@ -313,6 +314,9 @@ _mount_opts()
        ubifs)
                export MOUNT_OPTIONS=$UBIFS_MOUNT_OPTIONS
                ;;
+       hfsplus)
+               export MOUNT_OPTIONS=$HFSPLUS_MOUNT_OPTIONS
+               ;;
        *)
                ;;
        esac
@@ -380,6 +384,9 @@ _mkfs_opts()
        f2fs)
                export MKFS_OPTIONS="$F2FS_MKFS_OPTIONS"
                ;;
+       hfsplus)
+               export MKFS_OPTIONS=$HFSPLUS_MKFS_OPTIONS
+               ;;
        *)
                ;;
        esac
@@ -397,6 +404,9 @@ _fsck_opts()
        f2fs)
                export FSCK_OPTIONS=""
                ;;
+       hfsplus)
+               export FSCK_OPTIONS="-nX"
+               ;;
        *)
                export FSCK_OPTIONS="-n"
                ;;
common/rc +0 -0
index d5bb1fee..a843e9f6 100644
@@ -158,6 +158,12 @@ case "$FSTYP" in
     ubifs)
        [ "$UBIUPDATEVOL_PROG" = "" ] && _fatal "ubiupdatevol not found"
        ;;
Quoted 8 lines
common/rc case "$FSTYP" in · line 165
Quoted 5 lines
common/rc +5 -0
+    hfsplus)
+       [ "$MKFS_HFSPLUS_PROG" = "" ] && _fatal "mkfs.hfsplus not found"
+       mkfs.hfsplus -X |& grep "invalid" &&
+               _fatal "A patched version of mkfs.hfsplus is required:" \
+                      "https://github.com/eafer/hfsprogs-linux.git"
Why fatal and not just notrun the _scratch_mkfs_sized tests if no -X support? Must all tests run with mkfs -X and fsck -X??

Quoted 6 lines

I always use -X on mkfs so the tests will complain if the size of the device is not a whole number of blocks. Otherwise the tests would later fail on fsck.
Maybe I should check for -X support when setting the FSCK_OPTIONS, and in _scratch_mkfs()? Then _fatal would not be necessary.

I don't mind myself. If nobody has objections to support hfsplus only with patched tools that is a valid option.

Quoted 15 lines

Quoted 5 lines

Quoted 5 lines
common/rc +1 -0
+       ;;
 esac
 

Quoted 9 lines

common/rc _scratch_mkfs() · line 756

Quoted 7 lines

common/rc +2 -0

 if [ ! -z "$REPORT_LIST" ]; then

@@ -746,6 +752,10 @@ _scratch_mkfs()
                mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --"
                mkfs_filter="grep -v -e ^mkfs\.ocfs2"
                ;;
+       hfsplus)
+               mkfs_cmd="yes | $MKFS_PROG -t $FSTYP -- -X"

-X here seems like a typo?

Quoted 4 lines

This is so the tests will complain if the device size is not a multiple of the block size. I think I explained it in the patched man page for mkfs.hfsplus, but it may be better to put a comment here.

Comment is a good idea, but -X after -- ? shouldn't all options come before --?

Thanks,
Amir.

Ernesto A. Fernández

author

2018-09-26 00:44 UTC #4 lore raw

Quoted 123 lines · Amir Goldstein

Quoted 96 lines · Ernesto A. Fernández
Quoted 8 lines · Amir Goldstein
Quoted 3 lines · Ernesto A. Fernández
Running tests on hfsplus requires patched versions of the mkfs and fsck tools [1] that support filesystems smaller than the device.
That's not accurate, is it? Wouldn't it be more accurate to say that the patched tools are required to run the _scratch_mkfs_sized tests?
I set this up to call _fatal early if the tools are not the patched versions.
Using -X on mkfs.hfsplus forces the size to be a whole number of blocks, and using -X on fsck makes it work under that assumption. This is briefly explained in the patched man page for the tools.
It's a workaround needed because hfsplus only stores the number of allocation blocks in the volume header. If the device size is not a multiple of the block size, the backup header will be past the end, and fsck will take that as corruption.
Of course I could let the patched fsck use the -X flag by default. But that could be dangerous if somebody accidentally uses it to check an important filesystem.
Quoted 60 lines
Quoted 60 lines
[1] https://github.com/eafer/hfsprogs-linux.git
Signed-off-by Ernesto A. Fernández <[email protected]>
---
 common/config | 10 ++++++++++
 common/rc     | 15 ++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)
common/config +10 -0
index 1ba8d96c..315f8b1e 100644
@@ -229,6 +229,7 @@ case "$HOSTOS" in
        export MKFS_CIFS_PROG="false"
        export MKFS_OVERLAY_PROG="false"
        export MKFS_REISER4_PROG=$(type -P mkfs.reiser4)
+       export MKFS_HFSPLUS_PROG=$(type -P mkfs.hfsplus)
        export E2FSCK_PROG=$(type -P e2fsck)
        export TUNE2FS_PROG=$(type -P tune2fs)
        export FSCK_OVERLAY_PROG=$(type -P fsck.overlay)
@@ -313,6 +314,9 @@ _mount_opts()
        ubifs)
                export MOUNT_OPTIONS=$UBIFS_MOUNT_OPTIONS
                ;;
+       hfsplus)
+               export MOUNT_OPTIONS=$HFSPLUS_MOUNT_OPTIONS
+               ;;
        *)
                ;;
        esac
@@ -380,6 +384,9 @@ _mkfs_opts()
        f2fs)
                export MKFS_OPTIONS="$F2FS_MKFS_OPTIONS"
                ;;
+       hfsplus)
+               export MKFS_OPTIONS=$HFSPLUS_MKFS_OPTIONS
+               ;;
        *)
                ;;
        esac
@@ -397,6 +404,9 @@ _fsck_opts()
        f2fs)
                export FSCK_OPTIONS=""
                ;;
+       hfsplus)
+               export FSCK_OPTIONS="-nX"
+               ;;
        *)
                export FSCK_OPTIONS="-n"
                ;;
common/rc +0 -0
index d5bb1fee..a843e9f6 100644
@@ -158,6 +158,12 @@ case "$FSTYP" in
     ubifs)
        [ "$UBIUPDATEVOL_PROG" = "" ] && _fatal "ubiupdatevol not found"
        ;;
Quoted 8 lines
common/rc case "$FSTYP" in · line 165
Quoted 5 lines
common/rc +5 -0
+    hfsplus)
+       [ "$MKFS_HFSPLUS_PROG" = "" ] && _fatal "mkfs.hfsplus not found"
+       mkfs.hfsplus -X |& grep "invalid" &&
+               _fatal "A patched version of mkfs.hfsplus is required:" \
+                      "https://github.com/eafer/hfsprogs-linux.git"
Why fatal and not just notrun the _scratch_mkfs_sized tests if no -X support? Must all tests run with mkfs -X and fsck -X??
Quoted 6 lines
I always use -X on mkfs so the tests will complain if the size of the device is not a whole number of blocks. Otherwise the tests would later fail on fsck.
Maybe I should check for -X support when setting the FSCK_OPTIONS, and in _scratch_mkfs()? Then _fatal would not be necessary.
I don't mind myself. If nobody has objections to support hfsplus only with patched tools that is a valid option.
Quoted 15 lines
Quoted 14 lines
common/rc _scratch_mkfs() · line 756
Quoted 12 lines
common/rc +3 -0
+       ;;
 esac
 
 if [ ! -z "$REPORT_LIST" ]; then
@@ -746,6 +752,10 @@ _scratch_mkfs()
                mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --"
                mkfs_filter="grep -v -e ^mkfs\.ocfs2"
                ;;
+       hfsplus)
+               mkfs_cmd="yes | $MKFS_PROG -t $FSTYP -- -X"
-X here seems like a typo?

Quoted 7 lines

Quoted 4 lines
This is so the tests will complain if the device size is not a multiple of the block size. I think I explained it in the patched man page for mkfs.hfsplus, but it may be better to put a comment here.
Comment is a good idea, but -X after -- ? shouldn't all options come before --?

I think the generic mkfs program passes everything after -- to mkfs.fstype. I was following what ext2 does above.

Quoted 3 lines

Thanks,
Amir.