On May 01, 2023 / 17:10, Sagi Grimberg wrote:
> On 4/30/23 13:34, Shinichiro Kawasaki wrote:
[...]
> > Sagi, this comment above was not clear for me. Is Daniel's patch ok for you?
> >
> > IMO, it is reasonable to "do clean-up in reverse order as setup" as a general
> > guide. It will reduce the chance to see module related failures when the test
> > cases do not expect such failures. Instead, we can have dedicated test cases for
> > the module load/unload order related failures. start_soft_rdma and
> > stop_soft_rdma do module load and unload. So I think the guide is good for those
> > helper functions also.
>
> As I mentioned here, this change exercises a code path in the driver
> that is a surprise unplug of the rdma device. It is equivalent to
> triggering a surprise removal of the pci device normally during
> nvme-pci test teardown. While this is worth testing, I'm not sure we
> want the default behavior to do that, but rather add dedicated tests for
> it.
>
> Hence, my suggestion was to leave nvme-rdma as is.
Thanks for the clarification. I assume that stop_soft_rdma is the "surprise
unplug of the rdma device". If I understand it correctly, the change for nvme-fc
will be like this:
diff --git a/tests/nvme/rc b/tests/nvme/rc
index ec0cc2d..24803af 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -260,6 +260,11 @@ _cleanup_nvmet() {
shopt -u nullglob
trap SIGINT
+ if [[ "${nvme_trtype}" == "fc" ]]; then
+ _cleanup_fcloop "${def_local_wwnn}" "${def_local_wwpn}" \
+ "${def_remote_wwnn}" "${def_remote_wwpn}"
+ modprobe -rq nvme-fcloop
+ fi
modprobe -rq nvme-"${nvme_trtype}" 2>/dev/null
if [[ "${nvme_trtype}" != "loop" ]]; then
modprobe -rq nvmet-"${nvme_trtype}" 2>/dev/null
@@ -268,10 +273,6 @@ _cleanup_nvmet() {
if [[ "${nvme_trtype}" == "rdma" ]]; then
stop_soft_rdma
fi
- if [[ "${nvme_trtype}" == "fc" ]]; then
- _cleanup_fcloop "${def_local_wwnn}" "${def_local_wwpn}" \
- "${def_remote_wwnn}" "${def_remote_wwpn}"
- fi
}
_setup_nvmet() {
