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.