On Aug 22, 2024 / 21:13, Nilay Shroff wrote:
>
>
> On 8/22/24 20:19, Daniel Wagner wrote:
> > On Thu, Aug 22, 2024 at 11:59:35AM GMT, Shinichiro Kawasaki wrote:
> >> 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)
While I was preparing for v3, I noticed that it would be the better to redirect
the stderr to $FULL than /dev/null, since it will leave some clue on failure.
Will reflect this idea to v3.
> >>
> >> as you suggest. Still the test case can detect the kernel regression, and I
> >> think it's good enough. Will reflect this to v2.
> >
> > Not sure if this is relevant, but I'd like to see that we return error
> > codes so that the caller can actually decide to ignore the failure or
> > not.
> The _find_nvme_ns(), when fails to find the relevant namespace, it returns
> "empty string" and if it could find namespace then it returns the namespace
> value to the caller. And then caller (in this case nvmf_wait_for_ns()) would
> take action depending on the return value from _find_nvme_ns().
Even if _find_nvme_ns() returns a different error code for the "cat:
/sys/block/nvmeXnY/uuid: No such file or directory" error, it does not decide
the condition to ignore the error or not. So I think the "empty string" check
for the _find_nvme_ns() is enough.
Anyway, will send out v3 soon.
