FS/XFS testing framework
 help / color / mirror / Atom feed
* [PATCH v2] fstests: add support for hfsplus
@ 2018-09-24 21:43 Ernesto A. Fernández
  2018-09-25  5:57 ` Amir Goldstein
  0 siblings, 1 reply; 5+ messages in thread
From: Ernesto A. Fernández @ 2018-09-24 21:43 UTC (permalink / raw)
  To: fstests; +Cc: Eryu Guan, Eric Sandeen, Dave Chinner

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 <ernesto.mnd.fernandez@gmail.com>
---
 common/config | 10 ++++++++++
 common/rc     | 15 ++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/common/config b/common/config
index 1ba8d96c..315f8b1e 100644
--- a/common/config
+++ b/common/config
@@ -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"
 		;;
diff --git a/common/rc b/common/rc
index d5bb1fee..a843e9f6 100644
--- a/common/rc
+++ b/common/rc
@@ -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)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] fstests: add support for hfsplus
  2018-09-24 21:43 [PATCH v2] fstests: add support for hfsplus Ernesto A. Fernández
@ 2018-09-25  5:57 ` Amir Goldstein
  2018-09-25 17:58   ` Ernesto A. Fernández
  0 siblings, 1 reply; 5+ messages in thread
From: Amir Goldstein @ 2018-09-25  5:57 UTC (permalink / raw)
  To: ernesto.mnd.fernandez; +Cc: fstests, Eryu Guan, Eric Sandeen, Dave Chinner

On Tue, Sep 25, 2018 at 12:44 AM Ernesto A. Fernández
<ernesto.mnd.fernandez@gmail.com> wrote:
>
> 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?

>
> [1] https://github.com/eafer/hfsprogs-linux.git
>
> Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
> ---
>  common/config | 10 ++++++++++
>  common/rc     | 15 ++++++++++++++-
>  2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/common/config b/common/config
> index 1ba8d96c..315f8b1e 100644
> --- a/common/config
> +++ b/common/config
> @@ -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"
>                 ;;
> diff --git a/common/rc b/common/rc
> index d5bb1fee..a843e9f6 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -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"

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??

> +       ;;
>  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?

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] fstests: add support for hfsplus
  2018-09-25  5:57 ` Amir Goldstein
@ 2018-09-25 17:58   ` Ernesto A. Fernández
  2018-09-25 18:12     ` Amir Goldstein
  0 siblings, 1 reply; 5+ messages in thread
From: Ernesto A. Fernández @ 2018-09-25 17:58 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: fstests, Eryu Guan, Eric Sandeen, Dave Chinner

On Tue, Sep 25, 2018 at 08:57:58AM +0300, Amir Goldstein wrote:
> On Tue, Sep 25, 2018 at 12:44 AM Ernesto A. Fernández
> <ernesto.mnd.fernandez@gmail.com> wrote:
> >
> > 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.

> 
> >
> > [1] https://github.com/eafer/hfsprogs-linux.git
> >
> > Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
> > ---
> >  common/config | 10 ++++++++++
> >  common/rc     | 15 ++++++++++++++-
> >  2 files changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/config b/common/config
> > index 1ba8d96c..315f8b1e 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -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"
> >                 ;;
> > diff --git a/common/rc b/common/rc
> > index d5bb1fee..a843e9f6 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -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"
> 
> 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.

> 
> > +       ;;
> >  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?

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

> 
> Thanks,
> Amir.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] fstests: add support for hfsplus
  2018-09-25 17:58   ` Ernesto A. Fernández
@ 2018-09-25 18:12     ` Amir Goldstein
  2018-09-25 18:35       ` Ernesto A. Fernández
  0 siblings, 1 reply; 5+ messages in thread
From: Amir Goldstein @ 2018-09-25 18:12 UTC (permalink / raw)
  To: ernesto.mnd.fernandez; +Cc: fstests, Eryu Guan, Eric Sandeen, Dave Chinner

On Tue, Sep 25, 2018 at 8:58 PM Ernesto A. Fernández
<ernesto.mnd.fernandez@gmail.com> wrote:
>
> On Tue, Sep 25, 2018 at 08:57:58AM +0300, Amir Goldstein wrote:
> > On Tue, Sep 25, 2018 at 12:44 AM Ernesto A. Fernández
> > <ernesto.mnd.fernandez@gmail.com> wrote:
> > >
> > > 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.
>
> >
> > >
> > > [1] https://github.com/eafer/hfsprogs-linux.git
> > >
> > > Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
> > > ---
> > >  common/config | 10 ++++++++++
> > >  common/rc     | 15 ++++++++++++++-
> > >  2 files changed, 24 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/common/config b/common/config
> > > index 1ba8d96c..315f8b1e 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -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"
> > >                 ;;
> > > diff --git a/common/rc b/common/rc
> > > index d5bb1fee..a843e9f6 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -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"
> >
> > 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.

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

>
> >
> > > +       ;;
> > >  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?
>
> 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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] fstests: add support for hfsplus
  2018-09-25 18:12     ` Amir Goldstein
@ 2018-09-25 18:35       ` Ernesto A. Fernández
  0 siblings, 0 replies; 5+ messages in thread
From: Ernesto A. Fernández @ 2018-09-25 18:35 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: fstests, Eryu Guan, Eric Sandeen, Dave Chinner

On Tue, Sep 25, 2018 at 09:12:03PM +0300, Amir Goldstein wrote:
> On Tue, Sep 25, 2018 at 8:58 PM Ernesto A. Fernández
> <ernesto.mnd.fernandez@gmail.com> wrote:
> >
> > On Tue, Sep 25, 2018 at 08:57:58AM +0300, Amir Goldstein wrote:
> > > On Tue, Sep 25, 2018 at 12:44 AM Ernesto A. Fernández
> > > <ernesto.mnd.fernandez@gmail.com> wrote:
> > > >
> > > > 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.
> >
> > >
> > > >
> > > > [1] https://github.com/eafer/hfsprogs-linux.git
> > > >
> > > > Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
> > > > ---
> > > >  common/config | 10 ++++++++++
> > > >  common/rc     | 15 ++++++++++++++-
> > > >  2 files changed, 24 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/common/config b/common/config
> > > > index 1ba8d96c..315f8b1e 100644
> > > > --- a/common/config
> > > > +++ b/common/config
> > > > @@ -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"
> > > >                 ;;
> > > > diff --git a/common/rc b/common/rc
> > > > index d5bb1fee..a843e9f6 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -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"
> > >
> > > 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.
> 
> I don't mind myself. If nobody has objections to support hfsplus only with
> patched tools that is a valid option.
> 
> >
> > >
> > > > +       ;;
> > > >  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?
> >
> > 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.

> 
> Thanks,
> Amir.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-09-26  0:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-24 21:43 [PATCH v2] fstests: add support for hfsplus Ernesto A. Fernández
2018-09-25  5:57 ` Amir Goldstein
2018-09-25 17:58   ` Ernesto A. Fernández
2018-09-25 18:12     ` Amir Goldstein
2018-09-25 18:35       ` Ernesto A. Fernández

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox