Re: [PATCH blktests v1 11/11] nvme: Add explicitly host to allow_host list

[Daniel Wagner]

On Fri, Jul 28, 2023 at 08:15:45AM +0000, Shinichiro Kawasaki wrote:
> On Jul 26, 2023 / 14:46, Daniel Wagner wrote:
> > Only allow to connect to our setup target with the correct hostnqn.
> > 
> > Thus we have to explicitly add the test hostnqn to the test subsysnqn
> > allow_host list.
> 
> [...]
> 
> > --- a/tests/nvme/030
> > +++ b/tests/nvme/030
> > @@ -28,6 +28,7 @@ test() {
> >  
> >  	_create_nvmet_subsystem "${subsys}1" "$(losetup -f)"
> >  	_add_nvmet_subsys_to_port "${port}" "${subsys}1"
> > +	_create_nvmet_host "${subsys}1" "${def_hostnqn}"
> >  
> >  	genctr=$(_discovery_genctr)
> >  
> > @@ -36,13 +37,13 @@ test() {
> >  
> >  	genctr=$(_check_genctr "${genctr}" "adding a subsystem to a port")
> >  
> > -	echo 0 > "${NVMET_CFS}/subsystems/${subsys}2/attr_allow_any_host"
> > +	_add_nvmet_allow_hosts "${subsys}2" "${def_hostnqn}"
> >  
> > -	genctr=$(_check_genctr "${genctr}" "clearing attr_allow_any_host")
> > +	genctr=$(_check_genctr "${genctr}" "adding host to allow_hosts")
> >  
> > -	echo 1 > "${NVMET_CFS}/subsystems/${subsys}2/attr_allow_any_host"
> > +	_remove_nvmet_allow_hosts "${subsys}2" "${def_hostnqn}"
> >  
> > -	genctr=$(_check_genctr "${genctr}" "setting attr_allow_any_host")
> > +	genctr=$(_check_genctr "${genctr}" "removing host from allow_hosts")
> >  
> >  	_remove_nvmet_subsystem_from_port "${port}" "${subsys}2"
> >  	_remove_nvmet_subsystem "${subsys}2"
> 
> The hunk above looks different from other changes. Is it changing test
> content slightly to meet request by Max? If so, it would be good to note
> in the commit message.

I should have explained this change in the commit message. When we use
allowed_hosts, the attr_allow_any_host feature is disabled and thus has no side
effects like updating the genctr. This makes the test fail. I opted to use
_[add|remove]_nvmet_allow_hosts to trigger the same side effect. While at it, I
also updated the logging info.