2014-09-07 12:29:21

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH] ext4: use mkfs.ext4 -F instead of piping in yes

Using "yes | mkfs.ext4 ..." results in the error message results in
the test failing, at least for some versions of e2fsprogs:

+yes: standard output: Broken pipe
+yes: write error

It better to use the -F option, which will eliminate the questions.

Signed-off-by: Theodore Ts'o <[email protected]>
Cc: Xiaoguang Wang <[email protected]>
Cc: Eryu Guan <[email protected]>
---
tests/ext4/003 | 2 +-
tests/ext4/306 | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/ext4/003 b/tests/ext4/003
index 572e685..a2e9d75 100755
--- a/tests/ext4/003
+++ b/tests/ext4/003
@@ -43,7 +43,7 @@ _require_ext4_bigalloc

rm -f $seqres.full

-yes | mkfs.ext4 -O bigalloc -C 65536 -g 256 $SCRATCH_DEV 512m \
+mkfs.ext4 -F -O bigalloc -C 65536 -g 256 $SCRATCH_DEV 512m \
>> $seqres.full 2>&1
_scratch_mount || _fail "couldn't mount fs"

diff --git a/tests/ext4/306 b/tests/ext4/306
index fd50b0e..fd5c3a2 100755
--- a/tests/ext4/306
+++ b/tests/ext4/306
@@ -48,7 +48,7 @@ _require_scratch
rm -f $seqres.full

# Make a small ext4 fs with extents disabled & mount it
-yes | mkfs.ext4 -O ^extents,^64bit $SCRATCH_DEV 512m >> $seqres.full 2>&1
+mkfs.ext4 -F -O ^extents,^64bit $SCRATCH_DEV 512m >> $seqres.full 2>&1
_scratch_mount || _fail "couldn't mount fs"

# Create a small non-extent-based file
--
2.1.0



2014-09-07 15:49:28

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] ext4: use mkfs.ext4 -F instead of piping in yes

On Sun, Sep 07, 2014 at 08:29:02AM -0400, Theodore Ts'o wrote:
> Using "yes | mkfs.ext4 ..." results in the error message results in
> the test failing, at least for some versions of e2fsprogs:
>
> +yes: standard output: Broken pipe
> +yes: write error
>
> It better to use the -F option, which will eliminate the questions.

Looks good, but maybe we should have a $MKFS_EXT4_PROG variable similar
to $MKFS_XFS_PROG so that we can set this in one place?

2014-09-07 19:21:23

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: use mkfs.ext4 -F instead of piping in yes

On Sun, Sep 07, 2014 at 08:49:27AM -0700, Christoph Hellwig wrote:
> On Sun, Sep 07, 2014 at 08:29:02AM -0400, Theodore Ts'o wrote:
> > Using "yes | mkfs.ext4 ..." results in the error message results in
> > the test failing, at least for some versions of e2fsprogs:
> >
> > +yes: standard output: Broken pipe
> > +yes: write error
> >
> > It better to use the -F option, which will eliminate the questions.
>
> Looks good, but maybe we should have a $MKFS_EXT4_PROG variable similar
> to $MKFS_XFS_PROG so that we can set this in one place?

Agreed, I'll send a separate patch to fix that. One amusing thing:

% git grep mkfs.ext4
common/rc:_scratch_mkfs_ext4()
common/rc: _scratch_mkfs_ext4 $*
common/rc:# this test requires the bigalloc feature to be available in mkfs.ext4
common/rc: _scratch_mkfs_ext4 -O bigalloc >/dev/null 2>&1 \
common/rc: || _notrun "mkfs.ext4 doesn't have bigalloc feature"
common/rc: _scratch_mkfs_ext4 -O bigalloc >/dev/null 2>&1
tests/btrfs/012:MKFS_EXT4_PROG="`set_prog_path mkfs.ext4`"
tests/btrfs/012:_require_command $MKFS_EXT4_PROG mkfs.ext4
tests/ext4/003:mkfs.ext4 -F -O bigalloc -C 65536 -g 256 $SCRATCH_DEV 512m \
tests/ext4/306:mkfs.ext4 -F -O ^extents,^64bit $SCRATCH_DEV 512m >> $seqres.full 2>&1

(note the btrfs test is the one place using and defining MKFS_EXT4_PROG :-)

- Ted


2014-09-08 02:06:21

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH 2/2] ext4: speed up _require_ext4_bigalloc and _require_ext4_mkfs_bigalloc

We don't need to make a full-sized file system in order to test
whether "mkfs.ext4 -O bigalloc" works.

Signed-off-by: Theodore Ts'o <[email protected]>
---
common/rc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index 285bd7c..56e1847 100644
--- a/common/rc
+++ b/common/rc
@@ -1189,7 +1189,7 @@ _require_xfs_crc()
#
_require_ext4_mkfs_bigalloc()
{
- _scratch_mkfs_ext4 -O bigalloc >/dev/null 2>&1 \
+ $MKFS_EXT4_PROG -F -O bigalloc $SCRATCH_DEV 200k >/dev/null 2>&1 \
|| _notrun "mkfs.ext4 doesn't have bigalloc feature"
}

@@ -1197,7 +1197,7 @@ _require_ext4_mkfs_bigalloc()
#
_require_ext4_bigalloc()
{
- _scratch_mkfs_ext4 -O bigalloc >/dev/null 2>&1
+ $MKFS_EXT4_PROG -F -O bigalloc $SCRATCH_DEV 200k >/dev/null 2>&1
_scratch_mount >/dev/null 2>&1 \
|| _notrun "Ext4 kernel doesn't support bigalloc feature"
umount $SCRATCH_MNT
--
2.1.0


2014-09-08 02:06:21

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH 1/2] ext4: define MKFS_EXT4_PROG and use it instead of "mkfs.ext4" / "mkfs -t ext4"

Signed-off-by: Theodore Ts'o <[email protected]>
---
common/config | 1 +
common/rc | 7 +++++--
tests/btrfs/012 | 1 -
tests/ext4/003 | 2 +-
tests/ext4/306 | 2 +-
5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/common/config b/common/config
index 10cc6fe..f038813 100644
--- a/common/config
+++ b/common/config
@@ -209,6 +209,7 @@ case "$HOSTOS" in
;;
Linux)
export MKFS_XFS_PROG="`set_prog_path mkfs.xfs`"
+ export MKFS_EXT4_PROG="`set_prog_path mkfs.ext4`"
export MKFS_UDF_PROG="`set_prog_path mkudffs`"
export MKFS_BTRFS_PROG="`set_btrfs_mkfs_prog_path_with_opts`"
export BTRFS_UTIL_PROG="`set_prog_path btrfs`"
diff --git a/common/rc b/common/rc
index 16da898..285bd7c 100644
--- a/common/rc
+++ b/common/rc
@@ -105,6 +105,9 @@ case "$FSTYP" in
btrfs)
[ "$MKFS_BTRFS_PROG" = "" ] && _fatal "mkfs.btrfs not found"
;;
+ ext4)
+ [ "$MKFS_EXT4_PROG" = "" ] && _fatal "mkfs.ext4 not found"
+ ;;
nfs)
;;
esac
@@ -451,7 +454,7 @@ _scratch_mkfs_ext4()

local tmp_dir=/tmp/

- /sbin/mkfs -t ext4 -- -F $MKFS_OPTIONS $extra_mkfs_options $SCRATCH_DEV \
+ $MKFS_EXT4_PROG -F $MKFS_OPTIONS $extra_mkfs_options $SCRATCH_DEV \
2>$tmp_dir.mkfserr 1>$tmp_dir.mkfsstd
local mkfs_status=$?

@@ -466,7 +469,7 @@ _scratch_mkfs_ext4()
) >> $seqres.full

# running mkfs again. overwrite previous mkfs output files
- /sbin/mkfs -t ext4 -- -F $extra_mkfs_options $SCRATCH_DEV \
+ $MKFS_EXT4_PROG -F $extra_mkfs_options $SCRATCH_DEV \
2>$tmp_dir.mkfserr 1>$tmp_dir.mkfsstd
local mkfs_status=$?
fi
diff --git a/tests/btrfs/012 b/tests/btrfs/012
index f7e5da5..124c8ae 100755
--- a/tests/btrfs/012
+++ b/tests/btrfs/012
@@ -55,7 +55,6 @@ _supported_os Linux
_require_scratch

BTRFS_CONVERT_PROG="`set_prog_path btrfs-convert`"
-MKFS_EXT4_PROG="`set_prog_path mkfs.ext4`"
E2FSCK_PROG="`set_prog_path e2fsck`"

_require_command $BTRFS_CONVERT_PROG btrfs-convert
diff --git a/tests/ext4/003 b/tests/ext4/003
index a2e9d75..53875a9 100755
--- a/tests/ext4/003
+++ b/tests/ext4/003
@@ -43,7 +43,7 @@ _require_ext4_bigalloc

rm -f $seqres.full

-mkfs.ext4 -F -O bigalloc -C 65536 -g 256 $SCRATCH_DEV 512m \
+$MKFS_EXT4_PROG -F -O bigalloc -C 65536 -g 256 $SCRATCH_DEV 512m \
>> $seqres.full 2>&1
_scratch_mount || _fail "couldn't mount fs"

diff --git a/tests/ext4/306 b/tests/ext4/306
index fd5c3a2..edc0204 100755
--- a/tests/ext4/306
+++ b/tests/ext4/306
@@ -48,7 +48,7 @@ _require_scratch
rm -f $seqres.full

# Make a small ext4 fs with extents disabled & mount it
-mkfs.ext4 -F -O ^extents,^64bit $SCRATCH_DEV 512m >> $seqres.full 2>&1
+$MKFS_EXT4_PROG -F -O ^extents,^64bit $SCRATCH_DEV 512m >> $seqres.full 2>&1
_scratch_mount || _fail "couldn't mount fs"

# Create a small non-extent-based file
--
2.1.0


2014-09-08 09:50:15

by Dave Chinner

[permalink] [raw]
Subject: Re: [PATCH 2/2] ext4: speed up _require_ext4_bigalloc and _require_ext4_mkfs_bigalloc

On Sun, Sep 07, 2014 at 10:06:08PM -0400, Theodore Ts'o wrote:
> We don't need to make a full-sized file system in order to test
> whether "mkfs.ext4 -O bigalloc" works.

Why wouldn't you just use the "-n" option? If bigalloc is not
supported, that should still fail, right?

Cheers,

Dave.
--
Dave Chinner
[email protected]

2014-09-08 12:02:47

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 2/2] ext4: speed up _require_ext4_bigalloc and _require_ext4_mkfs_bigalloc

On Mon, Sep 08, 2014 at 07:50:00PM +1000, Dave Chinner wrote:
> On Sun, Sep 07, 2014 at 10:06:08PM -0400, Theodore Ts'o wrote:
> > We don't need to make a full-sized file system in order to test
> > whether "mkfs.ext4 -O bigalloc" works.
>
> Why wouldn't you just use the "-n" option? If bigalloc is not
> supported, that should still fail, right?

Good point. We still need to make the file system for
_require_ext4_bigalloc, but there's no need to create one for
_require_ext4_mkfs_bigalloc.

- Ted

2014-09-08 12:15:55

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH -v2] ext4: speed up _require_ext4_bigalloc and _require_ext4_mkfs_bigalloc

We don't need to make a full-sized file system in order to test
whether "mkfs.ext4 -O bigalloc" works.

Signed-off-by: Theodore Ts'o <[email protected]>
---
common/rc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index 285bd7c..0baac86 100644
--- a/common/rc
+++ b/common/rc
@@ -1189,7 +1189,7 @@ _require_xfs_crc()
#
_require_ext4_mkfs_bigalloc()
{
- _scratch_mkfs_ext4 -O bigalloc >/dev/null 2>&1 \
+ $MKFS_EXT4_PROG -F -O bigalloc -n $SCRATCH_DEV 200k >/dev/null 2>&1 \
|| _notrun "mkfs.ext4 doesn't have bigalloc feature"
}

@@ -1197,7 +1197,7 @@ _require_ext4_mkfs_bigalloc()
#
_require_ext4_bigalloc()
{
- _scratch_mkfs_ext4 -O bigalloc >/dev/null 2>&1
+ $MKFS_EXT4_PROG -F -O bigalloc $SCRATCH_DEV 200k >/dev/null 2>&1
_scratch_mount >/dev/null 2>&1 \
|| _notrun "Ext4 kernel doesn't support bigalloc feature"
umount $SCRATCH_MNT
--
2.1.0


2014-09-08 12:35:43

by Dave Chinner

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: define MKFS_EXT4_PROG and use it instead of "mkfs.ext4" / "mkfs -t ext4"

On Sun, Sep 07, 2014 at 10:06:07PM -0400, Theodore Ts'o wrote:
> Signed-off-by: Theodore Ts'o <[email protected]>
> ---
> common/config | 1 +
> common/rc | 7 +++++--
> tests/btrfs/012 | 1 -
> tests/ext4/003 | 2 +-
> tests/ext4/306 | 2 +-
> 5 files changed, 8 insertions(+), 5 deletions(-)

This throws lots of rejects with all the other patches I've already
queued up. Can you rebase it and resend it when I push out my
current tree in the morning, Ted?

Cheers,

Dave.
--
Dave Chinner
[email protected]

2014-09-08 12:36:16

by Dave Chinner

[permalink] [raw]
Subject: Re: [PATCH -v2] ext4: speed up _require_ext4_bigalloc and _require_ext4_mkfs_bigalloc

On Mon, Sep 08, 2014 at 08:15:41AM -0400, Theodore Ts'o wrote:
> We don't need to make a full-sized file system in order to test
> whether "mkfs.ext4 -O bigalloc" works.
>
> Signed-off-by: Theodore Ts'o <[email protected]>

Got it - thanks for the quick turnaround.

Cheers,

Dave.
--
Dave Chinner
[email protected]

2014-09-08 12:39:45

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH -v2] ext4: speed up _require_ext4_bigalloc and _require_ext4_mkfs_bigalloc

On Mon, Sep 08, 2014 at 10:36:02PM +1000, Dave Chinner wrote:
> On Mon, Sep 08, 2014 at 08:15:41AM -0400, Theodore Ts'o wrote:
> > We don't need to make a full-sized file system in order to test
> > whether "mkfs.ext4 -O bigalloc" works.
> >
> > Signed-off-by: Theodore Ts'o <[email protected]>
>
> Got it - thanks for the quick turnaround.

You're not going to apply this one until I send out the rebased
version of the 1/2 version of this patch series, right? Because this
one depends on MKFS_EXT4_PROG...

- Ted

2014-09-08 12:50:57

by Dave Chinner

[permalink] [raw]
Subject: Re: [PATCH -v2] ext4: speed up _require_ext4_bigalloc and _require_ext4_mkfs_bigalloc

On Mon, Sep 08, 2014 at 08:39:41AM -0400, Theodore Ts'o wrote:
> On Mon, Sep 08, 2014 at 10:36:02PM +1000, Dave Chinner wrote:
> > On Mon, Sep 08, 2014 at 08:15:41AM -0400, Theodore Ts'o wrote:
> > > We don't need to make a full-sized file system in order to test
> > > whether "mkfs.ext4 -O bigalloc" works.
> > >
> > > Signed-off-by: Theodore Ts'o <[email protected]>
> >
> > Got it - thanks for the quick turnaround.
>
> You're not going to apply this one until I send out the rebased
> version of the 1/2 version of this patch series, right? Because this
> one depends on MKFS_EXT4_PROG...

No, I haven't applied it, just put it at the end of the patch
queue so I don't forget it...

Cheers,

Dave.
--
Dave Chinner
[email protected]

2014-09-09 11:49:25

by Eryu Guan

[permalink] [raw]
Subject: Re: [PATCH] ext4: use mkfs.ext4 -F instead of piping in yes

On Sun, Sep 07, 2014 at 08:29:02AM -0400, Theodore Ts'o wrote:
> Using "yes | mkfs.ext4 ..." results in the error message results in
> the test failing, at least for some versions of e2fsprogs:
>
> +yes: standard output: Broken pipe
> +yes: write error
>
> It better to use the -F option, which will eliminate the questions.
>
> Signed-off-by: Theodore Ts'o <[email protected]>
> Cc: Xiaoguang Wang <[email protected]>
> Cc: Eryu Guan <[email protected]>

Looks good to me.

Reviewed-by: Eryu Guan <[email protected]>

Thanks,
Eryu
> ---
> tests/ext4/003 | 2 +-
> tests/ext4/306 | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ext4/003 b/tests/ext4/003
> index 572e685..a2e9d75 100755
> --- a/tests/ext4/003
> +++ b/tests/ext4/003
> @@ -43,7 +43,7 @@ _require_ext4_bigalloc
>
> rm -f $seqres.full
>
> -yes | mkfs.ext4 -O bigalloc -C 65536 -g 256 $SCRATCH_DEV 512m \
> +mkfs.ext4 -F -O bigalloc -C 65536 -g 256 $SCRATCH_DEV 512m \
> >> $seqres.full 2>&1
> _scratch_mount || _fail "couldn't mount fs"
>
> diff --git a/tests/ext4/306 b/tests/ext4/306
> index fd50b0e..fd5c3a2 100755
> --- a/tests/ext4/306
> +++ b/tests/ext4/306
> @@ -48,7 +48,7 @@ _require_scratch
> rm -f $seqres.full
>
> # Make a small ext4 fs with extents disabled & mount it
> -yes | mkfs.ext4 -O ^extents,^64bit $SCRATCH_DEV 512m >> $seqres.full 2>&1
> +mkfs.ext4 -F -O ^extents,^64bit $SCRATCH_DEV 512m >> $seqres.full 2>&1
> _scratch_mount || _fail "couldn't mount fs"
>
> # Create a small non-extent-based file
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-09-27 00:11:44

by Dave Chinner

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: define MKFS_EXT4_PROG and use it instead of "mkfs.ext4" / "mkfs -t ext4"

On Mon, Sep 08, 2014 at 10:35:28PM +1000, Dave Chinner wrote:
> On Sun, Sep 07, 2014 at 10:06:07PM -0400, Theodore Ts'o wrote:
> > Signed-off-by: Theodore Ts'o <[email protected]>
> > ---
> > common/config | 1 +
> > common/rc | 7 +++++--
> > tests/btrfs/012 | 1 -
> > tests/ext4/003 | 2 +-
> > tests/ext4/306 | 2 +-
> > 5 files changed, 8 insertions(+), 5 deletions(-)
>
> This throws lots of rejects with all the other patches I've already
> queued up. Can you rebase it and resend it when I push out my
> current tree in the morning, Ted?

Hi Ted,

Can you send an update for this patch if you still need it?

Cheers,

Dave.
--
Dave Chinner
[email protected]

2014-09-27 22:15:04

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: define MKFS_EXT4_PROG and use it instead of "mkfs.ext4" / "mkfs -t ext4"

On Sat, Sep 27, 2014 at 10:11:41AM +1000, Dave Chinner wrote:
>
> Can you send an update for this patch if you still need it?

Thanks for pinging me; I've been so busy these past few weeks I forgot
to check to see if the fstests repo had been updated. I see you did
so quite a while ago, my bad.

- Ted