diff mbox series

[v3,2/2] common/rc: improve block_size support for bcachefs

Message ID 20240129140101.4259-4-l@damenly.org (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Su Yue Jan. 29, 2024, 2:01 p.m. UTC 
From: Su Yue <glass.su@suse.com>

mkfs.bcachefs now supports option '--block_size' to allow
custom block_size.

Add the pattern to set def_blksz if MKFS_OPTIONS contains the
option in _scratch_mkfs_sized.
Also let mkfs.bcachefs decide blocksize if no option is given in
local.config or _scratch_mkfs_sized parameter.

Signed-off-by: Su Yue <glass.su@suse.com>
---
changelog:
v3:
    Add logic to Let mkfs.bcachefs decide blocksize if no option is given in
    local.config or _scratch_mkfs_sized parameter.
v2:
    Born.
---
 common/rc | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Brian Foster Jan. 29, 2024, 3:44 p.m. UTC | #1 
On Mon, Jan 29, 2024 at 10:01:01PM +0800, Su Yue wrote:
> From: Su Yue <glass.su@suse.com>
> 
> mkfs.bcachefs now supports option '--block_size' to allow
> custom block_size.
> 
> Add the pattern to set def_blksz if MKFS_OPTIONS contains the
> option in _scratch_mkfs_sized.
> Also let mkfs.bcachefs decide blocksize if no option is given in
> local.config or _scratch_mkfs_sized parameter.
> 
> Signed-off-by: Su Yue <glass.su@suse.com>
> ---
> changelog:
> v3:
>     Add logic to Let mkfs.bcachefs decide blocksize if no option is given in
>     local.config or _scratch_mkfs_sized parameter.
> v2:
>     Born.
> ---

Looks like the series duplicates the patches for some reason..

>  common/rc | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index 31c21d2a8360..315a2413f963 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -930,6 +930,7 @@ _scratch_mkfs_sized()
>  	local fssize=$1
>  	local blocksize=$2
>  	local def_blksz
> +	local blocksize_opt
>  
>  	case $FSTYP in
>  	xfs)
> @@ -950,6 +951,13 @@ _scratch_mkfs_sized()
>  	jfs)
>  		def_blksz=4096
>  		;;
> +	bcachefs)
> +		def_blksz=`echo $MKFS_OPTIONS | sed -rn 's/.*(--block_size)[ =]?+([0-9]+).*/\2/p'`
> +		[ -n "$def_blksize" ] && blocksize_opt="--block_size=$def_blksize"
> +		[ -n "$blocksize" ] && blocksize_opt="--block_size=$blocksize"

This seems reasonable to me, but if I follow this function correctly the
behavior when both the param ($blocksize) and MKFS_OPTIONS specify a
block size is that MKFS_OPTIONS overrides the former. For bcachefs it
looks like the above does the opposite. Should we switch around the
above two statements?

Brian

> +		# If no block size is given by local.confg or parameter, blocksize_opt is empty.
> +		# Let MKFS_BCACHEFS_PROG decide block size on its own.
> +		;;
>  	esac
>  
>  	[ -n "$def_blksz" ] && blocksize=$def_blksz
> @@ -1051,7 +1059,7 @@ _scratch_mkfs_sized()
>  		export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS"
>  		;;
>  	bcachefs)
> -		$MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize --block_size=$blocksize $SCRATCH_DEV
> +		$MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt $SCRATCH_DEV
>  		;;
>  	*)
>  		_notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized"
> -- 
> 2.43.0
> 
>
Su Yue Jan. 29, 2024, 11:44 p.m. UTC | #2 
On Mon 29 Jan 2024 at 10:44, Brian Foster <bfoster@redhat.com> 
wrote:

> On Mon, Jan 29, 2024 at 10:01:01PM +0800, Su Yue wrote:
>> From: Su Yue <glass.su@suse.com>
>>
>> mkfs.bcachefs now supports option '--block_size' to allow
>> custom block_size.
>>
>> Add the pattern to set def_blksz if MKFS_OPTIONS contains the
>> option in _scratch_mkfs_sized.
>> Also let mkfs.bcachefs decide blocksize if no option is given 
>> in
>> local.config or _scratch_mkfs_sized parameter.
>>
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> changelog:
>> v3:
>>     Add logic to Let mkfs.bcachefs decide blocksize if no 
>>     option is given in
>>     local.config or _scratch_mkfs_sized parameter.
>> v2:
>>     Born.
>> ---
>
> Looks like the series duplicates the patches for some reason..
>
Yeah..I sent two patches but 4 patches in
https://lore.kernel.org/fstests/20240129140101.4259-3-l@damenly.org/T/#t

>
>>  common/rc | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/rc b/common/rc
>> index 31c21d2a8360..315a2413f963 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -930,6 +930,7 @@ _scratch_mkfs_sized()
>>  	local fssize=$1
>>  	local blocksize=$2
>>  	local def_blksz
>> +	local blocksize_opt
>>
>>  	case $FSTYP in
>>  	xfs)
>> @@ -950,6 +951,13 @@ _scratch_mkfs_sized()
>>  	jfs)
>>  		def_blksz=4096
>>  		;;
>> +	bcachefs)
>> +		def_blksz=`echo $MKFS_OPTIONS | sed -rn 
>> 's/.*(--block_size)[ =]?+([0-9]+).*/\2/p'`
>> +		[ -n "$def_blksize" ] && 
>> blocksize_opt="--block_size=$def_blksize"
>> +		[ -n "$blocksize" ] && 
>> blocksize_opt="--block_size=$blocksize"
>
> This seems reasonable to me, but if I follow this function 
> correctly the
> behavior when both the param ($blocksize) and MKFS_OPTIONS 
> specify a
> block size is that MKFS_OPTIONS overrides the former. For 
> bcachefs it
> looks like the above does the opposite. Should we switch around 
> the
> above two statements?
>
Thanks for the catch. Nights turned my brain to mush.

V4 was sent.
--
Su
> Brian
>
>> +		# If no block size is given by local.confg or 
>> parameter, blocksize_opt is empty.
>> +		# Let MKFS_BCACHEFS_PROG decide block size on its own.
>> +		;;
>>  	esac
>>
>>  	[ -n "$def_blksz" ] && blocksize=$def_blksz
>> @@ -1051,7 +1059,7 @@ _scratch_mkfs_sized()
>>  		export MOUNT_OPTIONS="-o size=$fssize 
>>  $TMPFS_MOUNT_OPTIONS"
>>  		;;
>>  	bcachefs)
>> -		$MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize 
>> --block_size=$blocksize $SCRATCH_DEV
>> +		$MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize 
>> $blocksize_opt $SCRATCH_DEV
>>  		;;
>>  	*)
>>  		_notrun "Filesystem $FSTYP not supported in 
>>  _scratch_mkfs_sized"
>> --
>> 2.43.0
>>
>>
diff mbox series

Patch

diff --git a/common/rc b/common/rc
index 31c21d2a8360..315a2413f963 100644
--- a/common/rc
+++ b/common/rc
@@ -930,6 +930,7 @@  _scratch_mkfs_sized()
 	local fssize=$1
 	local blocksize=$2
 	local def_blksz
+	local blocksize_opt
 
 	case $FSTYP in
 	xfs)
@@ -950,6 +951,13 @@  _scratch_mkfs_sized()
 	jfs)
 		def_blksz=4096
 		;;
+	bcachefs)
+		def_blksz=`echo $MKFS_OPTIONS | sed -rn 's/.*(--block_size)[ =]?+([0-9]+).*/\2/p'`
+		[ -n "$def_blksize" ] && blocksize_opt="--block_size=$def_blksize"
+		[ -n "$blocksize" ] && blocksize_opt="--block_size=$blocksize"
+		# If no block size is given by local.confg or parameter, blocksize_opt is empty.
+		# Let MKFS_BCACHEFS_PROG decide block size on its own.
+		;;
 	esac
 
 	[ -n "$def_blksz" ] && blocksize=$def_blksz
@@ -1051,7 +1059,7 @@  _scratch_mkfs_sized()
 		export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS"
 		;;
 	bcachefs)
-		$MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize --block_size=$blocksize $SCRATCH_DEV
+		$MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt $SCRATCH_DEV
 		;;
 	*)
 		_notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized"