| Message ID | 20220530130811.3006554-6-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [blktests,1/9] nvme: use _have_loop instead of _have_modules loop | expand |
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@lst.de> > --- > 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.
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="$@" - 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 return 1 fi + _remove_null_blk_devices + modprobe -qr null_blk && modprobe -q null_blk ${modparams} + udevadm settle return 0 } @@ -46,5 +50,5 @@ _configure_null_blk() { _exit_null_blk() { _remove_null_blk_devices udevadm settle - modprobe -r null_blk + modprobe -qr null_blk }
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@lst.de> --- common/null_blk | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)