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

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

On Jul 01, 2022 / 09:43, Chaitanya Kulkarni 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.

-- 
Shin'ichiro Kawasaki