On 9/27/24 07:41, Shinichiro Kawasaki wrote:
> On Sep 25, 2024 / 13:01, Nilay Shroff wrote:
>> Avoid waiting indefinitely for nvme passthru namespace block device
>> to appear. Wait for up to 5 seconds and during this time if namespace
>> device doesn't appear then bail out and FAIL the test.
>>
>> Signed-off-by: Nilay Shroff <nilay@xxxxxxxxxxxxx>
>> ---
>> Changes from v1:
>> - Add a meaningful error message if test fails (Shnichiro
>> Kawasaki)
>> - Use sleep "1" instead of ".1" while waiting for nsdev to be
>> created as we don't see much gain in test runtime with short
>> duration of sleep. This would also help further optimize
>> the sleep logic (Shnichiro Kawasaki)
>> - Few other trivial cleanups (Shnichiro Kawasaki)
>
> Thanks for this v2 patch.
>
> [...]
>
>> diff --git a/tests/nvme/036 b/tests/nvme/036
>> index 442ffe7..a114a7c 100755
>> --- a/tests/nvme/036
>> +++ b/tests/nvme/036
>> @@ -27,6 +27,7 @@ test_device() {
>> _setup_nvmet
>>
>> local ctrldev
>> + local nsdev
>>
>> _nvmet_passthru_target_setup
>> nsdev=$(_nvmet_passthru_target_connect)
>
> I commented for v1 that "unnecsseary change" was made for nmve/036. With that
> comment, I meant that one empty line removal was unnecessary. However, you
> removed the nsdev check in nvme/036 for v2. I guess you might have misunderstood
> my comment then removed the check. I suggest to put back the nsdev check in
> nvme/036. If you agree, could you send out v3? Or I can do it when I apply this
> patch.
>
Ah, yes I think that was misunderstanding. When I reviewed your last comment about
nvme/036, I thought you wanted to remove nsdev check just because the intention of
the nvme/036 test was to test "reset controller" command. So in that sense, the nsdev
check is not important. Having said that, yes keeping nsdev check may also not
harm. So I would add that nsdev check back in nvme/036 and send out patch v3.
> Other than that, this patch looks good to me. Let's wait and see if anyone has
> other comments.
Thanks,
--Nilay
