Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 28 Jul 2023 10:55:27 +0200
From:   Daniel Wagner <dwagner@suse.de>
To:     Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Cc:     "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        Chaitanya Kulkarni <kch@nvidia.com>,
        Max Gurtovoy <mgurtovoy@nvidia.com>,
        Hannes Reinecke <hare@suse.de>,
        Sagi Grimberg <sagi@grimberg.me>,
        James Smart <jsmart2021@gmail.com>
Subject: Re: [PATCH blktests v1 11/11] nvme: Add explicitly host to
 allow_host list
Message-ID: <tgchxlramrmairlqojfw5tim2hqgvwu3iug532lyu2lzrjk4pw@geegddpo3zqw>
References: <20230726124644.12619-1-dwagner@suse.de>
 <20230726124644.12619-12-dwagner@suse.de>
 <4eztk7hhup2le6nqd4u4udvdrek3sngrljfr22b7rnhaqcr4of@aoe2j52jfwzb>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4eztk7hhup2le6nqd4u4udvdrek3sngrljfr22b7rnhaqcr4of@aoe2j52jfwzb>
Precedence: bulk

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.