Re: [PATCH blktests v2] nvme: add nvmf reset/disconnect during traffic test

[Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

On Jul 11, 2022 / 11:22, Sagi Grimberg wrote:
> 
> > > O
> > > > +
> > > > +	# do reset/remove operation
> > > > +	echo "resetting controller"
> > > > +	_nvme_reset_ctrl ${nvmedev}
> > > > +	sleep 1
> > > > +	echo "deleting controller"
> > > > +	_nvme_delete_ctrl ${nvmedev}
> > > > +
> > > > +	echo "stopping background fio"
> > > > +	{ kill $!; wait; } &> /dev/null
> > > 
> > > do we really need to kill the fio process explicitly ?
> > > I think graceful termination of the traffic application should be
> > > a part of this test without explicitly killing the process.
> > > 
> > > I ran the test with commenting above line it runs fine :-
> > > 
> > > blktests (master) # git diff
> > > diff --git a/tests/nvme/040 b/tests/nvme/040
> > > index d259784..0dce128 100755
> > > --- a/tests/nvme/040
> > > +++ b/tests/nvme/040
> > > @@ -49,7 +49,7 @@ test() {
> > >           _nvme_delete_ctrl ${nvmedev}
> > > 
> > >           echo "stopping background fio"
> > > -       { kill $!; wait; } &> /dev/null
> > > +#{ kill $!; wait; } &> /dev/null
> > > 
> > >           _remove_nvmet_subsystem_from_port "${port}" "${subsys}"
> > >           _remove_nvmet_subsystem "${subsys}"
> > > blktests (master) # ./check nvme/040
> > > nvme/040 (test nvme fabrics controller reset/disconnect operation during
> > > I/O) [passed]
> > >       runtime  8.119s  ...  8.084s
> > > blktests (master) #
> > > 
> > > -ck
> > 
> > Sagi, what do you think on this comment?
> > 
> > To keep this test case consistent with nvme/032, it would be the better to keep
> > the kill and wait. But we may not need to stick to it if it does better testing
> > without the kill and wait.
> 
> I don't particularly care. Can you remove it when applying?

Sure, I've applied with the edit.

-- 
Shin'ichiro Kawasaki