diff mbox series

[1/3] xfs/530: Do not pass block size argument to _scratch_mkfs

Message ID 20210726064313.19153-1-chandanrlinux@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/3] xfs/530: Do not pass block size argument to _scratch_mkfs | expand

Commit Message

Chandan Babu R July 26, 2021, 6:43 a.m. UTC
_scratch_do_mkfs constructs a mkfs command line by concatenating the values of
1. $mkfs_cmd
2. $MKFS_OPTIONS
3. $extra_mkfs_options

The block size argument passed by xfs/530 to _scratch_mkfs() will cause
mkfs.xfs to fail if $MKFS_OPTIONS also has a block size specified. In such a
case, _scratch_do_mkfs() will construct and invoke an mkfs command line
without including the value of $MKFS_OPTIONS.

To prevent such silent failures, this commit removes the block size option
that was being explicitly passed to _scratch_mkfs().

Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/530 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Darrick J. Wong July 26, 2021, 5:08 p.m. UTC | #1
On Mon, Jul 26, 2021 at 12:13:11PM +0530, Chandan Babu R wrote:
> _scratch_do_mkfs constructs a mkfs command line by concatenating the values of
> 1. $mkfs_cmd
> 2. $MKFS_OPTIONS
> 3. $extra_mkfs_options
> 
> The block size argument passed by xfs/530 to _scratch_mkfs() will cause
> mkfs.xfs to fail if $MKFS_OPTIONS also has a block size specified. In such a
> case, _scratch_do_mkfs() will construct and invoke an mkfs command line
> without including the value of $MKFS_OPTIONS.
> 
> To prevent such silent failures, this commit removes the block size option
> that was being explicitly passed to _scratch_mkfs().

Yes, that makes sense.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> 
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> ---
>  tests/xfs/530 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/xfs/530 b/tests/xfs/530
> index 4d168ac5..16dc426c 100755
> --- a/tests/xfs/530
> +++ b/tests/xfs/530
> @@ -60,7 +60,7 @@ echo "Format and mount rt volume"
>  
>  export USE_EXTERNAL=yes
>  export SCRATCH_RTDEV=$rtdev
> -_scratch_mkfs -d size=$((1024 * 1024 * 1024)) -b size=${dbsize} \
> +_scratch_mkfs -d size=$((1024 * 1024 * 1024)) \
>  	      -r size=${rtextsz},extsize=${rtextsz} >> $seqres.full
>  _try_scratch_mount || _notrun "Couldn't mount fs with synthetic rt volume"
>  
> -- 
> 2.30.2
>
Eryu Guan Aug. 1, 2021, 11:18 a.m. UTC | #2
On Mon, Jul 26, 2021 at 10:08:27AM -0700, Darrick J. Wong wrote:
> On Mon, Jul 26, 2021 at 12:13:11PM +0530, Chandan Babu R wrote:
> > _scratch_do_mkfs constructs a mkfs command line by concatenating the values of
> > 1. $mkfs_cmd
> > 2. $MKFS_OPTIONS
> > 3. $extra_mkfs_options
> > 
> > The block size argument passed by xfs/530 to _scratch_mkfs() will cause
> > mkfs.xfs to fail if $MKFS_OPTIONS also has a block size specified. In such a
> > case, _scratch_do_mkfs() will construct and invoke an mkfs command line
> > without including the value of $MKFS_OPTIONS.
> > 
> > To prevent such silent failures, this commit removes the block size option
> > that was being explicitly passed to _scratch_mkfs().

Patch looks fine to me, and queued for update.

> 
> Yes, that makes sense.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>

I'm wondering if mkfs.xfs could restore the behavior that allows
re-specified options, and the last specified wins, e.g.

mkfs -t xfs -b 1k -b 2k -f /dev/sda2

and mkfs.xfs didn't report failure but created xfs with 2k blocksize.
This may cause less problems like this patch resolved.

Thanks,
Eryu

> 
> --D
> 
> > 
> > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> > ---
> >  tests/xfs/530 | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/xfs/530 b/tests/xfs/530
> > index 4d168ac5..16dc426c 100755
> > --- a/tests/xfs/530
> > +++ b/tests/xfs/530
> > @@ -60,7 +60,7 @@ echo "Format and mount rt volume"
> >  
> >  export USE_EXTERNAL=yes
> >  export SCRATCH_RTDEV=$rtdev
> > -_scratch_mkfs -d size=$((1024 * 1024 * 1024)) -b size=${dbsize} \
> > +_scratch_mkfs -d size=$((1024 * 1024 * 1024)) \
> >  	      -r size=${rtextsz},extsize=${rtextsz} >> $seqres.full
> >  _try_scratch_mount || _notrun "Couldn't mount fs with synthetic rt volume"
> >  
> > -- 
> > 2.30.2
> >
diff mbox series

Patch

diff --git a/tests/xfs/530 b/tests/xfs/530
index 4d168ac5..16dc426c 100755
--- a/tests/xfs/530
+++ b/tests/xfs/530
@@ -60,7 +60,7 @@  echo "Format and mount rt volume"
 
 export USE_EXTERNAL=yes
 export SCRATCH_RTDEV=$rtdev
-_scratch_mkfs -d size=$((1024 * 1024 * 1024)) -b size=${dbsize} \
+_scratch_mkfs -d size=$((1024 * 1024 * 1024)) \
 	      -r size=${rtextsz},extsize=${rtextsz} >> $seqres.full
 _try_scratch_mount || _notrun "Couldn't mount fs with synthetic rt volume"