On Aug 22, 2024 / 16:45, Nilay Shroff wrote:
>
>
> On 8/22/24 14:03, Shinichiro Kawasaki wrote:
> > On Aug 21, 2024 / 14:58, Nilay Shroff wrote:
[...]
> >> Another point: I think, we may always suppress error from _find_nvme_ns() irrespective
> >> of it's being called while "creating" or "removing" the namespace assuming we always
> >> check the return status of nvme_wait_for_ns() in the main loop. So ideally we shall
> >> invoke _find_nvme_ns() from nvme_wait_for_ns() as below:
> >>
> >> ns=$(_find_nvme_ns "${uuid}" 2>/dev/null)
> >
> > It doesn't sound correct to suppress the _find_nvme_ns errors always. We found
> > the issue since _find_nvme_ns reported the error at after _create_nvmet_ns()
> > call. If we suppress it, I can not be confident that this fix avoids the error.
> >
> Agreed, however my thought was that if we always(while creating as well as deleting namespace)
> validate the return value from nvme_wait_for_ns() in the main loop then we should be OK and
> we may not need to worry about any error generated from the _find_nvme_ns(). The return
> status from nvme_wait_for_ns() is good enough for main loop to proceed further or bail out.
> Having said that, it's debatable as what you pointed out is also valid. The only thing which
> looks odd to me is that we have to suppress the error from _find_nvme_ns() for one case but
> not for the other.
I can agree with this point: it is odd to suppress errors only for the namespace
removal case. I did so to catch other potential errors that _find_nvme_ns() may
return in the future for the namespace creation case. But still this way misses
other potential errors for the namespace removal case. Maybe I was overthinking.
Let's simplify the test with just doing
ns=$(_find_nvme_ns "${uuid}" 2>/dev/null)
as you suggest. Still the test case can detect the kernel regression, and I
think it's good enough. Will reflect this to v2.
