On May 30, 2022 / 15:08, Christoph Hellwig wrote:
> Use _have_driver instead of _have_modules in _have_null_blk for the basic
> null_blk check, and instead only require an actual module in
> _init_null_blk when specific module parameters are passed.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> common/null_blk | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/common/null_blk b/common/null_blk
> index 6611db0..ccf3750 100644
> --- a/common/null_blk
> +++ b/common/null_blk
> @@ -5,7 +5,7 @@
> # null_blk helper functions.
>
> _have_null_blk() {
> - _have_modules null_blk
> + _have_driver null_blk
> }
>
> _remove_null_blk_devices() {
> @@ -16,15 +16,19 @@ _remove_null_blk_devices() {
> }
>
> _init_null_blk() {
> - _remove_null_blk_devices
> + local modparams="$@"
Shellcheck complains on this line:
common/null_blk:19:18: warning: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate. [SC2124]
I think array would be the better.
>
> - local zoned=""
> - if (( RUN_FOR_ZONED )); then zoned="zoned=1"; fi
> + if (( RUN_FOR_ZONED )); then
> + modparams="${modparams} zoned=1"
> + fi
>
> - if ! modprobe -r null_blk || ! modprobe null_blk "$@" "${zoned}" ; then
> + if [ -n "${modparams}" ] && ! _have_modules null_blk; then
I tried this part on my test machine and it looks that '_have_modules null_blk'
returns 0=success even when null_blk module is built-in. This may not be what
you intended here. I think _have_modules needs modification so that it returns
1=failure when one of the modules is built-in. (At least until all null_blk
parameters get supported by configfs.)
> return 1
> fi
>
> + _remove_null_blk_devices
> + modprobe -qr null_blk && modprobe -q null_blk ${modparams}
Double quotes for ${modparams} is required for shellcheck.
--
Shin'ichiro Kawasaki
