On 4/18/22 08:57, Luis Chamberlain wrote:
On Fri, Apr 15, 2022 at 09:01:03PM -0700, Bart Van Assche wrote:
On 4/15/22 18:49, Luis Chamberlain wrote:
- modprobe -r nvme-"${nvme_trtype}" 2>/dev/null
- if [[ "${nvme_trtype}" != "loop" ]]; then
- modprobe -r nvmet-"${nvme_trtype}" 2>/dev/null
- fi
- modprobe -r nvmet 2>/dev/null
+ if [[ "${nvme_trtype}" == "loop" ]]; then
+ _patient_rmmod nvme_"${nvme_trtype}"
+ else
+ _patient_rmmod nvme-"${nvme_trtype}"
+ _patient_rmmod nvmet-"${nvme_trtype}"
+ fi
+ _patient_rmmod nvmet 2>/dev/null
The statement _patient_rmmod nvme-"${nvme_trtype}" occurs twice in the above
code. How about preserving the structure of the existing code such that that
statement only occurs once?
There is one call for nvme-"${nvme_trtype}", the other is for the
underscore version, so there are no two calls.
Did I miss something?
It's only now that I see the underscore/hyphen difference in the if and else
branches. It is not clear to me why this behavior change has been introduced?
The following command produces no output on my test setup:
find /lib/modules -name 'nvme_*'
# Unload the SRP initiator driver.
stop_srp_ini() {
- local i
-
log_out
- for ((i=40;i>=0;i--)); do
- remove_mpath_devs || return $?
- unload_module ib_srp >/dev/null 2>&1 && break
- sleep 1
- done
- if [ -e /sys/module/ib_srp ]; then
- echo "Error: unloading kernel module ib_srp failed"
- return 1
- fi
- unload_module scsi_transport_srp || return $?
+ remove_mpath_devs || return $?
+ _patient_rmmod ib_srp || return 1
+ _patient_rmmod scsi_transport_srp || return $?
}
Removing the loop from around remove_mpath_devs is wrong. It is important
that that loop is preserved.
Why ? Can you test and verify?
If I remember correctly I put remove_mpath_devs call inside the loop because
multipathd keeps running while the loop is ongoing and hence can modify paths
while the loop is running.
Thanks,
Bart.
