| Message ID | 1467697153-23189-1-git-send-email-eguan@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, Jul 05, 2016 at 01:39:13PM +0800, Eryu Guan wrote: > In recent mkfs.xfs updates in xfsprogs, commit 9090e187bc3e ("mkfs: > add respecification detection to generic parsing") added > re-specification detection to "-m" option, it causes several tests > _notrun if MKFS_OPTIONS has the same options as those being tested > in _scratch_mkfs_xfs_supported(), because they're specified multiple > times. > > MKFS_OPTIONS="-m crc=0" ./check xfs/001 > xfs/001 3s ... [not run] mkfs.xfs doesn't have crc feature > > Fix it by creating XFS again without MKFS_OPTIONS in > _scratch_mkfs_xfs_supported(), in case there's conflict between > MKFS_OPTIONS and mkfs_opts, like what we do in _scratch_mkfs_xfs(). > > Signed-off-by: Eryu Guan <eguan@redhat.com> Ping on this. My first attempt was removing $MKFS_OPTIONS from $MKFS_XFS_PROG command line, but that broke xfs/186, this is the second attempt to fix this issue. Thanks, Eryu > --- > common/rc | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/common/rc b/common/rc > index 883bd7b..ad81461 100644 > --- a/common/rc > +++ b/common/rc > @@ -473,11 +473,20 @@ _scratch_mkfs_xfs_opts() > > _scratch_mkfs_xfs_supported() > { > - mkfs_opts=$* > + local mkfs_opts=$* > > _scratch_options mkfs > > $MKFS_XFS_PROG -N $MKFS_OPTIONS $SCRATCH_OPTIONS $mkfs_opts $SCRATCH_DEV > + local mkfs_status=$? > + > + # if $mkfs_opts conflits with $MKFS_OPTIONS, > + # try again without $MKFS_OPTIONS > + if [ $mkfs_status -ne 0 -a -n "$MKFS_OPTIONS" ]; then > + $MKFS_XFS_PROG -N $SCRATCH_OPTIONS $mkfs_opts $SCRATCH_DEV > + mkfs_status=$? > + fi > + return $mkfs_status > } > > _scratch_mkfs_xfs() > -- > 2.7.4 >
On Tue, Jul 05, 2016 at 01:39:13PM +0800, Eryu Guan wrote: > In recent mkfs.xfs updates in xfsprogs, commit 9090e187bc3e ("mkfs: > add respecification detection to generic parsing") added > re-specification detection to "-m" option, it causes several tests > _notrun if MKFS_OPTIONS has the same options as those being tested > in _scratch_mkfs_xfs_supported(), because they're specified multiple > times. > > MKFS_OPTIONS="-m crc=0" ./check xfs/001 > xfs/001 3s ... [not run] mkfs.xfs doesn't have crc feature > > Fix it by creating XFS again without MKFS_OPTIONS in > _scratch_mkfs_xfs_supported(), in case there's conflict between > MKFS_OPTIONS and mkfs_opts, like what we do in _scratch_mkfs_xfs(). > > Signed-off-by: Eryu Guan <eguan@redhat.com> > --- > common/rc | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/common/rc b/common/rc > index 883bd7b..ad81461 100644 > --- a/common/rc > +++ b/common/rc > @@ -473,11 +473,20 @@ _scratch_mkfs_xfs_opts() > > _scratch_mkfs_xfs_supported() > { > - mkfs_opts=$* > + local mkfs_opts=$* > > _scratch_options mkfs > > $MKFS_XFS_PROG -N $MKFS_OPTIONS $SCRATCH_OPTIONS $mkfs_opts $SCRATCH_DEV > + local mkfs_status=$? > + > + # if $mkfs_opts conflits with $MKFS_OPTIONS, > + # try again without $MKFS_OPTIONS > + if [ $mkfs_status -ne 0 -a -n "$MKFS_OPTIONS" ]; then > + $MKFS_XFS_PROG -N $SCRATCH_OPTIONS $mkfs_opts $SCRATCH_DEV > + mkfs_status=$? > + fi > + return $mkfs_status > } This should match what _scratch_mkfs_xfs does. i.e: # a mkfs failure may be caused by conflicts between # $MKFS_OPTIONS and $extra_mkfs_options if [ $mkfs_status -ne 0 -a ! -z "$extra_mkfs_options" ]; then ( echo -n "** mkfs failed with extra mkfs options " echo "added to \"$MKFS_OPTIONS\" by test $seq **" echo -n "** attempting to mkfs using only test $seq " echo "options: $extra_mkfs_options **" ) >> $seqres.full # running mkfs again. overwrite previous mkfs output files _scratch_mkfs_xfs_opts $extra_mkfs_options \ 2>$tmp_dir.mkfserr 1>$tmp_dir.mkfsstd local mkfs_status=$? fi i.e. it checks if $extra_mkfs_options is not empty, rather than $MKFS_OPTIONS Cheers, Dave.
On Fri, Jul 15, 2016 at 10:01:29AM +1000, Dave Chinner wrote: > On Tue, Jul 05, 2016 at 01:39:13PM +0800, Eryu Guan wrote: > > In recent mkfs.xfs updates in xfsprogs, commit 9090e187bc3e ("mkfs: > > add respecification detection to generic parsing") added > > re-specification detection to "-m" option, it causes several tests > > _notrun if MKFS_OPTIONS has the same options as those being tested > > in _scratch_mkfs_xfs_supported(), because they're specified multiple > > times. > > > > MKFS_OPTIONS="-m crc=0" ./check xfs/001 > > xfs/001 3s ... [not run] mkfs.xfs doesn't have crc feature > > > > Fix it by creating XFS again without MKFS_OPTIONS in > > _scratch_mkfs_xfs_supported(), in case there's conflict between > > MKFS_OPTIONS and mkfs_opts, like what we do in _scratch_mkfs_xfs(). > > > > Signed-off-by: Eryu Guan <eguan@redhat.com> > > --- > > common/rc | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/common/rc b/common/rc > > index 883bd7b..ad81461 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -473,11 +473,20 @@ _scratch_mkfs_xfs_opts() > > > > _scratch_mkfs_xfs_supported() > > { > > - mkfs_opts=$* > > + local mkfs_opts=$* > > > > _scratch_options mkfs > > > > $MKFS_XFS_PROG -N $MKFS_OPTIONS $SCRATCH_OPTIONS $mkfs_opts $SCRATCH_DEV > > + local mkfs_status=$? > > + > > + # if $mkfs_opts conflits with $MKFS_OPTIONS, > > + # try again without $MKFS_OPTIONS > > + if [ $mkfs_status -ne 0 -a -n "$MKFS_OPTIONS" ]; then > > + $MKFS_XFS_PROG -N $SCRATCH_OPTIONS $mkfs_opts $SCRATCH_DEV > > + mkfs_status=$? > > + fi > > + return $mkfs_status > > } > > > This should match what _scratch_mkfs_xfs does. i.e: > > # a mkfs failure may be caused by conflicts between > # $MKFS_OPTIONS and $extra_mkfs_options > if [ $mkfs_status -ne 0 -a ! -z "$extra_mkfs_options" ]; then > ( > echo -n "** mkfs failed with extra mkfs options " > echo "added to \"$MKFS_OPTIONS\" by test $seq **" > echo -n "** attempting to mkfs using only test $seq " > echo "options: $extra_mkfs_options **" > ) >> $seqres.full > > # running mkfs again. overwrite previous mkfs output files > _scratch_mkfs_xfs_opts $extra_mkfs_options \ > 2>$tmp_dir.mkfserr 1>$tmp_dir.mkfsstd > local mkfs_status=$? > fi > > i.e. it checks if $extra_mkfs_options is not empty, rather than > $MKFS_OPTIONS Thanks for the review! I thought about it too, but I realized that _scratch_mkfs_supported() is supposed to have "$extra_mkfs_options" (always non-empty), because what it tests is to check whether mkfs.xfs works well with these "$extra_mkfs_options". But making sure $mkfs_opts is not empty before using it makes more sense, and it matches _scratch_mkfs_xfs(). I'll send v2 patch. Thanks, Eryu
diff --git a/common/rc b/common/rc index 883bd7b..ad81461 100644 --- a/common/rc +++ b/common/rc @@ -473,11 +473,20 @@ _scratch_mkfs_xfs_opts() _scratch_mkfs_xfs_supported() { - mkfs_opts=$* + local mkfs_opts=$* _scratch_options mkfs $MKFS_XFS_PROG -N $MKFS_OPTIONS $SCRATCH_OPTIONS $mkfs_opts $SCRATCH_DEV + local mkfs_status=$? + + # if $mkfs_opts conflits with $MKFS_OPTIONS, + # try again without $MKFS_OPTIONS + if [ $mkfs_status -ne 0 -a -n "$MKFS_OPTIONS" ]; then + $MKFS_XFS_PROG -N $SCRATCH_OPTIONS $mkfs_opts $SCRATCH_DEV + mkfs_status=$? + fi + return $mkfs_status } _scratch_mkfs_xfs()
In recent mkfs.xfs updates in xfsprogs, commit 9090e187bc3e ("mkfs: add respecification detection to generic parsing") added re-specification detection to "-m" option, it causes several tests _notrun if MKFS_OPTIONS has the same options as those being tested in _scratch_mkfs_xfs_supported(), because they're specified multiple times. MKFS_OPTIONS="-m crc=0" ./check xfs/001 xfs/001 3s ... [not run] mkfs.xfs doesn't have crc feature Fix it by creating XFS again without MKFS_OPTIONS in _scratch_mkfs_xfs_supported(), in case there's conflict between MKFS_OPTIONS and mkfs_opts, like what we do in _scratch_mkfs_xfs(). Signed-off-by: Eryu Guan <eguan@redhat.com> --- common/rc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)