On 02/20/2019 10:11 AM, Omar Sandoval wrote:
> On Tue, Feb 19, 2019 at 03:28:18PM -0800, Chaitanya Kulkarni wrote:
>> ---
>> tests/nvme/021 | 15 ++++++---------
>> tests/nvme/021.out | 1 -
>> 2 files changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/tests/nvme/021 b/tests/nvme/021
>> index 8e9512a..f204a2f 100755
>> --- a/tests/nvme/021
>> +++ b/tests/nvme/021
>> @@ -38,11 +38,13 @@ test() {
>> cat "/sys/block/${nvmedev}n1/uuid"
>> cat "/sys/block/${nvmedev}n1/wwid"
>>
>> - nvme list > /dev/null 2>&1
>> -
>> - rc=$?
>> + if ! nvme list | grep -q "${nvmedev}n1" > /dev/null 2>&1; then
>> + echo "Test Failed"
>> + else
>> + echo "Test complete"
>> + fi
>>
>> - nvme disconnect -n "${subsys_name}"
>> + nvme disconnect -n "${subsys_name}" > /dev/null 2>&1
>>
>> _remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
>> _remove_nvmet_subsystem "${subsys_name}"
>> @@ -53,9 +55,4 @@ test() {
>> modprobe -r nvme-loop
>> modprobe -r nvmet
>>
>> - if [ $rc -ne 0 ]; then
>> - echo "Test Failed"
>> - else
>> - echo "Test complete"
>> - fi
>> }
>> diff --git a/tests/nvme/021.out b/tests/nvme/021.out
>> index 5fb0601..b6b1a7c 100644
>> --- a/tests/nvme/021.out
>> +++ b/tests/nvme/021.out
>> @@ -1,5 +1,4 @@
>> Running nvme/021
>> 91fdba0d-f87b-4c25-b80f-db7be1418b9e
>> uuid.91fdba0d-f87b-4c25-b80f-db7be1418b9e
>> -NQN:blktests-subsystem-1 disconnected 1 controller(s)
>> Test complete
>> --
>> 1.8.3.1
>>
>
> For all of these, I think it'd be useful to have a more descriptive
> error, like so:
>
> diff --git a/tests/nvme/021 b/tests/nvme/021
> index 8e9512a..0b215c4 100755
> --- a/tests/nvme/021
> +++ b/tests/nvme/021
> @@ -38,9 +38,9 @@ test() {
> cat "/sys/block/${nvmedev}n1/uuid"
> cat "/sys/block/${nvmedev}n1/wwid"
>
> - nvme list > /dev/null 2>&1
> -
> - rc=$?
> + if ! nvme list | grep -q "${nvmedev}n1"; then
> + echo "NVMeOF device not listed"
> + fi
>
> nvme disconnect -n "${subsys_name}"
>
> @@ -53,9 +53,5 @@ test() {
> modprobe -r nvme-loop
> modprobe -r nvmet
>
> - if [ $rc -ne 0 ]; then
> - echo "Test Failed"
> - else
> - echo "Test complete"
> - fi
> + echo "Test complete"
> }
>
Okay, will send out a new version along with if any more
comments that you have.