Re: [PATCH v3] fstests: teach _scratch_mkfs to handle mkfs option conflicts

[Eryu Guan <eguan@redhat.com>]

On Fri, Dec 16, 2016 at 09:22:09AM -0500, Brian Foster wrote:
> On Mon, Dec 05, 2016 at 04:42:56PM +0800, Eryu Guan wrote:
> > Currently in _scratch_mkfs only xfs and ext4 could handle the mkfs
> > failure caused by conflicts between $MKFS_OPTIONS and mkfs options
> > specified by tests, because of _scratch_mkfs_xfs and
> > _scratch_mkfs_ext4. This is a very useful functionality that allows
> > tests to specify mkfs options safely and to test specific fs
> > configurations, without worrying about mkfs failures caused by these
> > options.
> > 
> > Now teach _scratch_mkfs to handle such mkfs option conflicts for
> > other filesystems too, i.e. mkfs again only with mkfs options
> > specified by tests. Also add the ability to filter unnecessary
> > messages from mkfs stderr.
> > 
> > Also update some btrfs tests to throw away _scratch_mkfs stdout,
> > because previously _scratch_mkfs did this for btrfs.
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > 
> > v3:
> > - rebase against master HEAD, some changes go to common/xfs
> > 
> > v2:
> > - return in each case if fstyp is special-handled in _scratch_mkfs
> > - introduce _scratch_do_mkfs helper and convert _scratch_mkfs_xfs and
> >   _scratch_mkfs_ext4 to use it too. I'm not good at naming functions,
> >   please suggest if it's badly named..
> > - update some btrfs tests to avoid failures caused by mkfs stdout output
> > 
> >  common/rc       | 180 +++++++++++++++++++++++++++++++++-----------------------
> >  common/xfs      |  47 ++++-----------
> >  tests/btrfs/028 |   2 +-
> >  tests/btrfs/121 |   2 +-
> >  tests/btrfs/122 |   2 +-
> >  tests/btrfs/123 |   2 +-
> >  tests/btrfs/126 |   2 +-
> >  7 files changed, 126 insertions(+), 111 deletions(-)
> > 
> > diff --git a/common/rc b/common/rc
> > index 2719b23..7e45c14 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -410,6 +410,52 @@ _scratch_mkfs_options()
> >      echo $SCRATCH_OPTIONS $MKFS_OPTIONS $* $SCRATCH_DEV
> >  }
> >  
> > +# Do the actual mkfs work on SCRATCH_DEV. Firstly mkfs with both MKFS_OPTIONS
> > +# and user specified mkfs options, if that fails (due to conflicts between mkfs
> > +# options), do a second mkfs with only user provided mkfs options.
> > +#
> > +# First param is the mkfs command without any mkfs optoins and device.
> 
> Typo here:					      options

Will fix at commit time.

> 
> Otherwise looks Ok to me:
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>

Thanks for the review!

Eryu