On Wed, Feb 20, 2019 at 06:19:58PM +0000, Chaitanya Kulkarni wrote:
> 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.
The same comment applies for the rest, but I don't have any other
concerns.
