| Message ID | 20180901020503.5i4hz4inqtfd3frg@eaf (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] fstests: add support for hfsplus | expand |
On Fri, Aug 31, 2018 at 11:05:03PM -0300, Ernesto A. Fernández wrote: > It is not possible to set file system size when running mkfs.hfsplus, > so use the device mapper as a workaround. I'd prefer _notrun the test in _scratch_mkfs_sized() instead of this workaround, as the harness is expecting to operate $SCRATCH_DEV but this workaround may break this assumption in subtle ways, e.g. now _check_scratch_fs fails to create hfsplus-tmp device because $SCRATCH_DEV may still be mounted. Otherwise this patch looks fine to me. Thanks, Eryu > > Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> > --- > I've been using this to test hfsplus patches. If there's a better way > please let me know. > > common/config | 7 +++++++ > common/rc | 38 +++++++++++++++++++++++++++++++++++++- > 2 files changed, 44 insertions(+), 1 deletion(-) > > diff --git a/common/config b/common/config > index 2f1f2720..32a1ccb8 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 > diff --git a/common/rc b/common/rc > index ec631ad9..38f61944 100644 > --- a/common/rc > +++ b/common/rc > @@ -158,6 +158,9 @@ case "$FSTYP" in > ubifs) > [ "$UBIUPDATEVOL_PROG" = "" ] && _fatal "ubiupdatevol not found" > ;; > + hfsplus) > + [ "$MKFS_HFSPLUS_PROG" = "" ] && _fatal "mkfs.hfsplus not found" > + ;; > esac > > if [ ! -z "$REPORT_LIST" ]; then > @@ -746,6 +749,12 @@ _scratch_mkfs() > mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" > mkfs_filter="grep -v -e ^mkfs\.ocfs2" > ;; > + hfsplus) > + mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" > + mkfs_filter="cat" > + # a test may have been interrupted after _scratch_mkfs_sized() > + rm -f ${RESULT_DIR}/hfsplus_sectors > + ;; > *) > mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" > mkfs_filter="cat" > @@ -995,6 +1004,17 @@ _scratch_mkfs_sized() > fi > export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS" > ;; > + hfsplus) > + # mkfs.hfsplus does not allow setting the size of the filesystem > + _require_dm_target linear > + local sectors=`expr $fssize / 512` > + local table="0 $sectors linear $SCRATCH_DEV 0" > + _dmsetup_create hfsplus-tmp --table "$table" || _fatal "dmsetup failed" > + ${MKFS_PROG}.$FSTYP $MKFS_OPTIONS -b $blocksize /dev/mapper/hfsplus-tmp > + _dmsetup_remove hfsplus-tmp > + # remember the true size of the filesystem for fsck.hfsplus > + echo -n $sectors > ${RESULT_DIR}/hfsplus_sectors > + ;; > *) > _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized" > ;; > @@ -2661,6 +2681,22 @@ _check_scratch_fs() > ubifs) > # there is no fsck program for ubifs yet > ;; > + hfsplus) > + # fsck.hfsplus will always assume the filesystem fills the device > + if [ -f ${RESULT_DIR}/hfsplus_sectors ]; then > + local sectors=`cat ${RESULT_DIR}/hfsplus_sectors` > + rm -f ${RESULT_DIR}/hfsplus_sectors > + local table="0 $sectors linear $SCRATCH_DEV 0" > + _dmsetup_create hfsplus-tmp --table "$table" || \ > + _fatal "dmsetup failed" > + _check_generic_filesystem /dev/mapper/hfsplus-tmp > + local fsck_status=$? > + _dmsetup_remove hfsplus-tmp > + return $fsck_status > + else > + _check_generic_filesystem $device > + fi > + ;; > *) > _check_generic_filesystem $device > ;; > @@ -3055,7 +3091,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 >
On Sun, Sep 02, 2018 at 01:09:57AM +0800, Eryu Guan wrote: > On Fri, Aug 31, 2018 at 11:05:03PM -0300, Ernesto A. Fernández wrote: > > It is not possible to set file system size when running mkfs.hfsplus, > > so use the device mapper as a workaround. > > I'd prefer _notrun the test in _scratch_mkfs_sized() instead of this > workaround, as the harness is expecting to operate $SCRATCH_DEV but this > workaround may break this assumption in subtle ways, e.g. now > _check_scratch_fs fails to create hfsplus-tmp device because > $SCRATCH_DEV may still be mounted. I didn't realize $SCRATCH_DEV could still be mounted, sorry. Maybe I can unmount it before _dmsetup_create(), and remount after _dmsetup_remove()? Like _check_generic_filesystem() does. Or would that bring other problems? I know it's ugly, but one test that uses _scratch_mkfs_sized() has helped me find a number of bugs already. It would be really useful to get it to work. Thanks, Ernest > > Otherwise this patch looks fine to me. > > Thanks, > Eryu > > > > > Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> > > --- > > I've been using this to test hfsplus patches. If there's a better way > > please let me know. > > > > common/config | 7 +++++++ > > common/rc | 38 +++++++++++++++++++++++++++++++++++++- > > 2 files changed, 44 insertions(+), 1 deletion(-) > > > > diff --git a/common/config b/common/config > > index 2f1f2720..32a1ccb8 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 > > diff --git a/common/rc b/common/rc > > index ec631ad9..38f61944 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -158,6 +158,9 @@ case "$FSTYP" in > > ubifs) > > [ "$UBIUPDATEVOL_PROG" = "" ] && _fatal "ubiupdatevol not found" > > ;; > > + hfsplus) > > + [ "$MKFS_HFSPLUS_PROG" = "" ] && _fatal "mkfs.hfsplus not found" > > + ;; > > esac > > > > if [ ! -z "$REPORT_LIST" ]; then > > @@ -746,6 +749,12 @@ _scratch_mkfs() > > mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" > > mkfs_filter="grep -v -e ^mkfs\.ocfs2" > > ;; > > + hfsplus) > > + mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" > > + mkfs_filter="cat" > > + # a test may have been interrupted after _scratch_mkfs_sized() > > + rm -f ${RESULT_DIR}/hfsplus_sectors > > + ;; > > *) > > mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" > > mkfs_filter="cat" > > @@ -995,6 +1004,17 @@ _scratch_mkfs_sized() > > fi > > export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS" > > ;; > > + hfsplus) > > + # mkfs.hfsplus does not allow setting the size of the filesystem > > + _require_dm_target linear > > + local sectors=`expr $fssize / 512` > > + local table="0 $sectors linear $SCRATCH_DEV 0" > > + _dmsetup_create hfsplus-tmp --table "$table" || _fatal "dmsetup failed" > > + ${MKFS_PROG}.$FSTYP $MKFS_OPTIONS -b $blocksize /dev/mapper/hfsplus-tmp > > + _dmsetup_remove hfsplus-tmp > > + # remember the true size of the filesystem for fsck.hfsplus > > + echo -n $sectors > ${RESULT_DIR}/hfsplus_sectors > > + ;; > > *) > > _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized" > > ;; > > @@ -2661,6 +2681,22 @@ _check_scratch_fs() > > ubifs) > > # there is no fsck program for ubifs yet > > ;; > > + hfsplus) > > + # fsck.hfsplus will always assume the filesystem fills the device > > + if [ -f ${RESULT_DIR}/hfsplus_sectors ]; then > > + local sectors=`cat ${RESULT_DIR}/hfsplus_sectors` > > + rm -f ${RESULT_DIR}/hfsplus_sectors > > + local table="0 $sectors linear $SCRATCH_DEV 0" > > + _dmsetup_create hfsplus-tmp --table "$table" || \ > > + _fatal "dmsetup failed" > > + _check_generic_filesystem /dev/mapper/hfsplus-tmp > > + local fsck_status=$? > > + _dmsetup_remove hfsplus-tmp > > + return $fsck_status > > + else > > + _check_generic_filesystem $device > > + fi > > + ;; > > *) > > _check_generic_filesystem $device > > ;; > > @@ -3055,7 +3091,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 > >
On Sun, Sep 02, 2018 at 12:13:26AM -0500, Eric Sandeen wrote: > > > On 9/1/18 11:03 PM, Ernesto A. Fernández wrote: > > On Sun, Sep 02, 2018 at 01:09:57AM +0800, Eryu Guan wrote: > >> On Fri, Aug 31, 2018 at 11:05:03PM -0300, Ernesto A. Fernández wrote: > >>> It is not possible to set file system size when running mkfs.hfsplus, > >>> so use the device mapper as a workaround. > >> > >> I'd prefer _notrun the test in _scratch_mkfs_sized() instead of this > >> workaround, as the harness is expecting to operate $SCRATCH_DEV but this > >> workaround may break this assumption in subtle ways, e.g. now > >> _check_scratch_fs fails to create hfsplus-tmp device because > >> $SCRATCH_DEV may still be mounted. > > > > I didn't realize $SCRATCH_DEV could still be mounted, sorry. Maybe I can > > unmount it before _dmsetup_create(), and remount after _dmsetup_remove()? > > Like _check_generic_filesystem() does. Or would that bring other problems? I haven't seen other problems yet (after just a few simple test runs), but I suspect it may break tests in more subtle ways, the _check_scratch_fs failure is just an example. And even it works for now, and we have to worry about making it continue to work when adding new features in the future and that's a maintaining burden we'd better to avoid. > > > > I know it's ugly, but one test that uses _scratch_mkfs_sized() has helped > > me find a number of bugs already. It would be really useful to get it to > > work. > > Has anyone proposed a patch to mkfs.hfsplus to accept a filesystem size? > I'd expect that might be less complicated than this sort of devicemapper > setup ;) Yeah, this would be the best way to let _scratch_mkfs_sized() support hfsplus :) Thanks, Eryu
On Sun, Sep 02, 2018 at 10:05:29PM +0800, Eryu Guan wrote: > On Sun, Sep 02, 2018 at 12:13:26AM -0500, Eric Sandeen wrote: > > > > > > On 9/1/18 11:03 PM, Ernesto A. Fernández wrote: > > > On Sun, Sep 02, 2018 at 01:09:57AM +0800, Eryu Guan wrote: > > >> On Fri, Aug 31, 2018 at 11:05:03PM -0300, Ernesto A. Fernández wrote: > > >>> It is not possible to set file system size when running mkfs.hfsplus, > > >>> so use the device mapper as a workaround. > > >> > > >> I'd prefer _notrun the test in _scratch_mkfs_sized() instead of this > > >> workaround, as the harness is expecting to operate $SCRATCH_DEV but this > > >> workaround may break this assumption in subtle ways, e.g. now > > >> _check_scratch_fs fails to create hfsplus-tmp device because > > >> $SCRATCH_DEV may still be mounted. > > > > > > I didn't realize $SCRATCH_DEV could still be mounted, sorry. Maybe I can > > > unmount it before _dmsetup_create(), and remount after _dmsetup_remove()? > > > Like _check_generic_filesystem() does. Or would that bring other problems? > > I haven't seen other problems yet (after just a few simple test runs), > but I suspect it may break tests in more subtle ways, the > _check_scratch_fs failure is just an example. And even it works for now, > and we have to worry about making it continue to work when adding new > features in the future and that's a maintaining burden we'd better to > avoid. > > > > > > > I know it's ugly, but one test that uses _scratch_mkfs_sized() has helped > > > me find a number of bugs already. It would be really useful to get it to > > > work. > > > > Has anyone proposed a patch to mkfs.hfsplus to accept a filesystem size? > > I'd expect that might be less complicated than this sort of devicemapper > > setup ;) > > Yeah, this would be the best way to let _scratch_mkfs_sized() support > hfsplus :) mkfs.hfsplus and fsck.hfsplus both come from Apple. Do you think they would pick up a patch? I'm not sure how to reach them. I could also just patch these tools for myself. When somebody calls _scratch_mkfs_sized() on hfsplus with an unpatched version, the test could _notrun and tell them where to get mine. Is that acceptable? > > Thanks, > Eryu
On Sun, Sep 02, 2018 at 10:05:29PM +0800, Eryu Guan wrote: > On Sun, Sep 02, 2018 at 12:13:26AM -0500, Eric Sandeen wrote: > > > > > > On 9/1/18 11:03 PM, Ernesto A. Fernández wrote: > > > On Sun, Sep 02, 2018 at 01:09:57AM +0800, Eryu Guan wrote: > > >> On Fri, Aug 31, 2018 at 11:05:03PM -0300, Ernesto A. Fernández wrote: > > >>> It is not possible to set file system size when running mkfs.hfsplus, > > >>> so use the device mapper as a workaround. > > >> > > >> I'd prefer _notrun the test in _scratch_mkfs_sized() instead of this > > >> workaround, as the harness is expecting to operate $SCRATCH_DEV but this > > >> workaround may break this assumption in subtle ways, e.g. now > > >> _check_scratch_fs fails to create hfsplus-tmp device because > > >> $SCRATCH_DEV may still be mounted. > > > > > > I didn't realize $SCRATCH_DEV could still be mounted, sorry. Maybe I can > > > unmount it before _dmsetup_create(), and remount after _dmsetup_remove()? > > > Like _check_generic_filesystem() does. Or would that bring other problems? > > I haven't seen other problems yet (after just a few simple test runs), > but I suspect it may break tests in more subtle ways, the > _check_scratch_fs failure is just an example. And even it works for now, > and we have to worry about making it continue to work when adding new > features in the future and that's a maintaining burden we'd better to > avoid. Ah, I just replied on the -fsdevel thread saying exactly this.... > > > I know it's ugly, but one test that uses _scratch_mkfs_sized() has helped > > > me find a number of bugs already. It would be really useful to get it to > > > work. > > > > Has anyone proposed a patch to mkfs.hfsplus to accept a filesystem size? > > I'd expect that might be less complicated than this sort of devicemapper > > setup ;) > > Yeah, this would be the best way to let _scratch_mkfs_sized() support > hfsplus :) .... and this, too. :) It'll be easy to add a mkfs option check in _scratch_mkfs_sized() for hfsplus. That should make it work (or not run, as the case may be) reliably into the future, too. Cheers, Dave.
diff --git a/common/config b/common/config index 2f1f2720..32a1ccb8 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 diff --git a/common/rc b/common/rc index ec631ad9..38f61944 100644 --- a/common/rc +++ b/common/rc @@ -158,6 +158,9 @@ case "$FSTYP" in ubifs) [ "$UBIUPDATEVOL_PROG" = "" ] && _fatal "ubiupdatevol not found" ;; + hfsplus) + [ "$MKFS_HFSPLUS_PROG" = "" ] && _fatal "mkfs.hfsplus not found" + ;; esac if [ ! -z "$REPORT_LIST" ]; then @@ -746,6 +749,12 @@ _scratch_mkfs() mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" mkfs_filter="grep -v -e ^mkfs\.ocfs2" ;; + hfsplus) + mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" + mkfs_filter="cat" + # a test may have been interrupted after _scratch_mkfs_sized() + rm -f ${RESULT_DIR}/hfsplus_sectors + ;; *) mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" mkfs_filter="cat" @@ -995,6 +1004,17 @@ _scratch_mkfs_sized() fi export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS" ;; + hfsplus) + # mkfs.hfsplus does not allow setting the size of the filesystem + _require_dm_target linear + local sectors=`expr $fssize / 512` + local table="0 $sectors linear $SCRATCH_DEV 0" + _dmsetup_create hfsplus-tmp --table "$table" || _fatal "dmsetup failed" + ${MKFS_PROG}.$FSTYP $MKFS_OPTIONS -b $blocksize /dev/mapper/hfsplus-tmp + _dmsetup_remove hfsplus-tmp + # remember the true size of the filesystem for fsck.hfsplus + echo -n $sectors > ${RESULT_DIR}/hfsplus_sectors + ;; *) _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized" ;; @@ -2661,6 +2681,22 @@ _check_scratch_fs() ubifs) # there is no fsck program for ubifs yet ;; + hfsplus) + # fsck.hfsplus will always assume the filesystem fills the device + if [ -f ${RESULT_DIR}/hfsplus_sectors ]; then + local sectors=`cat ${RESULT_DIR}/hfsplus_sectors` + rm -f ${RESULT_DIR}/hfsplus_sectors + local table="0 $sectors linear $SCRATCH_DEV 0" + _dmsetup_create hfsplus-tmp --table "$table" || \ + _fatal "dmsetup failed" + _check_generic_filesystem /dev/mapper/hfsplus-tmp + local fsck_status=$? + _dmsetup_remove hfsplus-tmp + return $fsck_status + else + _check_generic_filesystem $device + fi + ;; *) _check_generic_filesystem $device ;; @@ -3055,7 +3091,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)
It is not possible to set file system size when running mkfs.hfsplus, so use the device mapper as a workaround. Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> --- I've been using this to test hfsplus patches. If there's a better way please let me know. common/config | 7 +++++++ common/rc | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-)