Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
To:     Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        Omar Sandoval <osandov@osandov.com>
Cc:     Sagi Grimberg <sagi@grimberg.me>,
        Stephen Bates <sbates@raithlin.com>
References: <20200930185422.11494-1-logang@deltatee.com>
 <20200930185422.11494-6-logang@deltatee.com>
 <BYAPR04MB49654DCBAD729B05759EB6FF860A0@BYAPR04MB4965.namprd04.prod.outlook.com>
From:   Logan Gunthorpe <logang@deltatee.com>
Message-ID: <e7bdf462-c86d-f248-58f4-d9d41b2bd343@deltatee.com>
Date:   Tue, 6 Oct 2020 18:13:33 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.12.0
MIME-Version: 1.0
In-Reply-To: <BYAPR04MB49654DCBAD729B05759EB6FF860A0@BYAPR04MB4965.namprd04.prod.outlook.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-CA
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH blktests v2 05/11] nvme: Add common helpers for passthru
 tests
Precedence: bulk


On 2020-10-06 6:02 p.m., Chaitanya Kulkarni wrote:
> On 9/30/20 12:01, Logan Gunthorpe wrote:
>> Add some simple helpers to setup a passthru target that passes through
>> to a nvme test device.
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> ---
>>  tests/nvme/rc | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 76 insertions(+)
>>
>> diff --git a/tests/nvme/rc b/tests/nvme/rc
>> index dfa57a299625..1ea23308a3f7 100644
>> --- a/tests/nvme/rc
>> +++ b/tests/nvme/rc
>> @@ -73,6 +73,17 @@ _require_nvme_trtype_is_fabrics() {
>>  	return 0
>>  }
>>  
>> +_test_dev_nvme_ctrl() {
>> +	local dev
>> +
>> +	dev=$(cat "${TEST_DEV_SYSFS}/device/dev")
>  can you initialize dev this at the time of declaration ?

Yup, will fix.

>> +	echo "/dev/char/${dev}"
>> +}
>> +
>> +_test_dev_nvme_nsid() {
>> +	cat "${TEST_DEV_SYSFS}/nsid"
>> +}
>> +
>>  _cleanup_nvmet() {
>>  	local dev
>>  	local port
>> @@ -257,6 +268,27 @@ _remove_nvmet_subsystem() {
>>  	rmdir "${subsys_path}"
>>  }
>>  
>> +_create_nvmet_passthru() {
>> +	local nvmet_subsystem="$1"
>> +	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
>> +	local passthru_path="${subsys_path}/passthru"
>> +
>> +	mkdir -p "${subsys_path}"
>> +	printf 1 > "${subsys_path}/attr_allow_any_host"
>> +
>> +	printf "%s" "$(_test_dev_nvme_ctrl)" > "${passthru_path}/device_path"
>> +	printf 1 > "${passthru_path}/enable"
> 
> can you please echo in general and printf only when it is needed ?>
> I know existing code is a bit inconsistent I'll send a clenup to make it
> uniform.

Yes, I agree. I will it fix in v3.
Thanks,

Logan