On 2/8/22 13:21, Luis Chamberlain wrote:
On Mon, Feb 07, 2022 at 05:10:24PM -0800, Bart Van Assche wrote:
On 11/16/21 09:29, Luis Chamberlain wrote:
+ if [[ -f /sys/module/$module/refcnt ]]; then
Has this patch been analyzed with 'make check'? I expect checkpatch to
complain about missing double quotes for the above statement.
No, I was not aware this was a thing. I fixed all the stupid complaints
however I should note at least tests/srp/rc has existing complaints
before my changes. I won't address those.
Please consider opening a pull request for the
https://github.com/osandov/blktests repository. That will cause the
blktests presubmit tests to be run. See also .github/workflows/ci.yml in
the blktests source code.
I can't say I agree with the changes to remove checks for $?, but
whatever. Consinstancy is better than not having one. Perhaps we should
embrace this on fstests as well.
+1
No, I'll clarify to this:
# Tries to wait patiently to remove a module by ensuring first
# the refcnt is 0 and then trying to remove the module over and over
# again within the time allowed
Looks better to me :-)
for m in ib_srpt iscsi_target_mod target_core_pscsi target_core_iblock \
target_core_file target_core_stgt target_core_user \
target_core_mod
- do
- unload_module $m 10 || return $?
- done
+ _patient_rmmod $m || return $1
Please proofread your changes and test your changes before posting these. I
think that both "do" and "done" should have been preserved above.
And the return value shoudl be $? not $1. I couldn't test srp so wanted
someone who would to help review, but sure, I should have caught this.
Hmm ... the SRP tests can be run inside a virtual machine so it is not
clear to me what prevents to run these tests?
Thanks,
Bart.
