xfs
[Top] [All Lists]

Re: [PATCH] xfstests: f2fs support

To: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
Subject: Re: [PATCH] xfstests: f2fs support
From: Lukáš Czerner <lczerner@xxxxxxxxxx>
Date: Tue, 3 Jun 2014 13:32:30 +0200 (CEST)
Cc: Dave Chinner <david@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1401717572-21731-1-git-send-email-jaegeuk@xxxxxxxxxx>
References: <1401717572-21731-1-git-send-email-jaegeuk@xxxxxxxxxx>
User-agent: Alpine 2.00 (LFD 1167 2008-08-23)
On Mon, 2 Jun 2014, jaegeuk@xxxxxxxxxx wrote:

> Date: Mon,  2 Jun 2014 22:59:32 +0900
> From: jaegeuk@xxxxxxxxxx
> To: Dave Chinner <david@xxxxxxxxxxxxx>
> Cc: xfs@xxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx,
>     linux-fsdevel@xxxxxxxxxxxxxxx, linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxxx,
>     Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
> Subject: [PATCH] xfstests: f2fs support
> 
> From: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
> 
> This patch adds to support f2fs file system.

Hi,

Looks good.

Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx>

Btw dry run option in fsck is quite useful here because when the
file system gets corrupted you do not want it to get fixed, but
rather investigate the corrupted file system to figure out what
happened. But of course it's not required :)

However after a quick look at you fsck code it seems that you do not
attempt to fix anything at all...

Also having mkfs to check for existing signatures on the device
before attempting to create the file system is a good thing to do
(at least xfs, btrfs extN are doing so) to avoid data loss by
mistake. You can use libblkid from util-linux and it's very simple
to use.

Thanks!
-Lukas

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
> ---
>  common/config | 7 +++++++
>  common/rc     | 6 ++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/common/config b/common/config
> index 0dbf0b9..0607294 100644
> --- a/common/config
> +++ b/common/config
> @@ -210,6 +210,7 @@ case "$HOSTOS" in
>          export MKFS_XFS_PROG="`set_prog_path mkfs.xfs`"
>          export MKFS_UDF_PROG="`set_prog_path mkudffs`"
>          export MKFS_BTRFS_PROG="`set_btrfs_mkfs_prog_path_with_opts`"
> +        export MKFS_F2FS_PROG="`set_prog_path mkfs.f2fs`"
>          export BTRFS_UTIL_PROG="`set_prog_path btrfs`"
>          export BTRFS_SHOW_SUPER_PROG="`set_prog_path btrfs-show-super`"
>          export XFS_FSR_PROG="`set_prog_path xfs_fsr`"
> @@ -241,6 +242,9 @@ _mount_opts()
>               # acls & xattrs aren't turned on by default on ext$FOO
>               export MOUNT_OPTIONS="-o acl,user_xattr $EXT_MOUNT_OPTIONS"
>               ;;
> +     f2fs)
> +             export MOUNT_OPTIONS="-o acl,user_xattr $F2FS_MOUNT_OPTIONS"
> +             ;;
>       reiserfs)
>               # acls & xattrs aren't turned on by default on reiserfs
>               export MOUNT_OPTIONS="-o acl,user_xattr $REISERFS_MOUNT_OPTIONS"
> @@ -295,6 +299,9 @@ _fsck_opts()
>       reiserfs)
>               export FSCK_OPTIONS="--yes"
>               ;;
> +     f2fs)
> +             export FSCK_OPTIONS=""
> +             ;;
>       *)
>               export FSCK_OPTIONS="-n"
>               ;;
> diff --git a/common/rc b/common/rc
> index f27ee53..fcdabfe 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -105,6 +105,9 @@ case "$FSTYP" in
>      btrfs)
>        [ "$MKFS_BTRFS_PROG" = "" ] && _fatal "mkfs.btrfs not found"
>        ;;
> +    f2fs)
> +      [ "$MKFS_F2FS_PROG" = "" ] && _fatal "mkfs.f2fs not found"
> +      ;;
>      nfs)
>        ;;
>  esac
> @@ -511,6 +514,9 @@ _scratch_mkfs()
>      tmpfs)
>       # do nothing for tmpfs
>       ;;
> +    f2fs)
> +        $MKFS_F2FS_PROG $MKFS_OPTIONS $* $SCRATCH_DEV > /dev/null
> +     ;;
>      *)
>       yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $SCRATCH_DEV
>       ;;
> 

<Prev in Thread] Current Thread [Next in Thread>