[PATCH 1/2 blktests] nvme: add udevadm settle before disconnecting

[PATCH 1/2 blktests] nvme: add udevadm settle before disconnecting

[Sagi Grimberg]

otherwise we can see some I/O failures as udev didn't get enough time
to do its thing.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 tests/nvme/008 | 2 ++
 tests/nvme/009 | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/tests/nvme/008 b/tests/nvme/008
index d73ec7d6d97a..04ff0bda42e3 100755
--- a/tests/nvme/008
+++ b/tests/nvme/008
@@ -41,6 +41,8 @@ test() {
 	cat "/sys/block/${nvmedev}n1/uuid"
 	cat "/sys/block/${nvmedev}n1/wwid"
 
+	udevadm settle
+
 	nvme disconnect -n "${subsys_name}"
 
 	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
diff --git a/tests/nvme/009 b/tests/nvme/009
index a46168123c56..81e61f99aba5 100755
--- a/tests/nvme/009
+++ b/tests/nvme/009
@@ -37,6 +37,8 @@ test() {
 	cat "/sys/block/${nvmedev}n1/uuid"
 	cat "/sys/block/${nvmedev}n1/wwid"
 
+	udevadm settle
+
 	nvme disconnect -n "${subsys_name}"
 
 	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
-- 
2.17.1

[PATCH 2/2 blktests] nvme: test out-of-range I/O access

[Sagi Grimberg]

Test that we correctly fail an out-of-range access
with a proper NVMe status code.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 tests/nvme/018     | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/018.out |  6 +++++
 2 files changed, 64 insertions(+)
 create mode 100755 tests/nvme/018
 create mode 100755 tests/nvme/018.out

diff --git a/tests/nvme/018 b/tests/nvme/018
new file mode 100755
index 000000000000..939573263c87
--- /dev/null
+++ b/tests/nvme/018
@@ -0,0 +1,58 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2018 Sagi Grimberg
+#
+# Test NVMe out of range access.
+
+. tests/nvme/rc
+
+DESCRIPTION="unit test NVMe-oF out of range access"
+QUICK=1
+
+requires() {
+	_have_program nvme && _have_modules nvme-loop && _have_modules loop && \
+		_have_configfs
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	modprobe nvmet
+	modprobe nvme-loop
+
+	local port
+	local nvmedev
+	local file_path="$TMPDIR/img"
+	local subsys_name="blktests-subsystem-1"
+
+	truncate -s 1G "${file_path}"
+
+	_create_nvmet_subsystem "${subsys_name}" "${file_path}" \
+		 "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
+	port="$(_create_nvmet_port "loop")"
+	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
+
+	nvme connect -t loop -n "${subsys_name}"
+
+	nvmedev="$(_find_nvme_loop_dev)"
+	cat "/sys/block/${nvmedev}n1/uuid"
+	cat "/sys/block/${nvmedev}n1/wwid"
+
+	sectors=$(blockdev --getsz /dev/${nvmedev}n1)
+	bs=$(blockdev --getbsz /dev/${nvmedev}n1)
+
+	nvme read "/dev/${nvmedev}n1" -s $sectors -c 0 -z $bs
+	if [[ ! $? ]]; then
+		echo "Sucessfully read out of device lba range"
+	fi
+
+	nvme disconnect -n "${subsys_name}"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
+	_remove_nvmet_subsystem "${subsys_name}"
+	_remove_nvmet_port "${port}"
+
+	rm "${file_path}"
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/018.out b/tests/nvme/018.out
new file mode 100755
index 000000000000..0ab3b2a892d3
--- /dev/null
+++ b/tests/nvme/018.out
@@ -0,0 +1,6 @@
+Running nvme/018
+91fdba0d-f87b-4c25-b80f-db7be1418b9e
+uuid.91fdba0d-f87b-4c25-b80f-db7be1418b9e
+read:CAP_EXCEEDED: The execution of the command has caused the capacity of the namespace to be exceeded(6081)
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.17.1

[PATCH 1/2 blktests] nvme: add udevadm settle before disconnecting

[Chaitanya Kulkarni]

On 3/6/19 1:47 AM, Sagi Grimberg wrote:
> otherwise we can see some I/O failures as udev didn't get enough time
> to do its thing.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   tests/nvme/008 | 2 ++
>   tests/nvme/009 | 2 ++
>   2 files changed, 4 insertions(+)
> 
> diff --git a/tests/nvme/008 b/tests/nvme/008
> index d73ec7d6d97a..04ff0bda42e3 100755
> --- a/tests/nvme/008
> +++ b/tests/nvme/008
> @@ -41,6 +41,8 @@ test() {
>   	cat "/sys/block/${nvmedev}n1/uuid"
>   	cat "/sys/block/${nvmedev}n1/wwid"
>   
> +	udevadm settle
> +
>   	nvme disconnect -n "${subsys_name}"
>   
>   	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
> diff --git a/tests/nvme/009 b/tests/nvme/009
> index a46168123c56..81e61f99aba5 100755
> --- a/tests/nvme/009
> +++ b/tests/nvme/009
> @@ -37,6 +37,8 @@ test() {
>   	cat "/sys/block/${nvmedev}n1/uuid"
>   	cat "/sys/block/${nvmedev}n1/wwid"
>   
> +	udevadm settle
> +
>   	nvme disconnect -n "${subsys_name}"
>   
>   	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
> 

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

[PATCH 2/2 blktests] nvme: test out-of-range I/O access

[Chaitanya Kulkarni]

On 3/6/19 1:47 AM, Sagi Grimberg wrote:
> Test that we correctly fail an out-of-range access
> with a proper NVMe status code.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   tests/nvme/018     | 58 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/nvme/018.out |  6 +++++
>   2 files changed, 64 insertions(+)
>   create mode 100755 tests/nvme/018
>   create mode 100755 tests/nvme/018.out
> 
> diff --git a/tests/nvme/018 b/tests/nvme/018
> new file mode 100755
> index 000000000000..939573263c87
> --- /dev/null
> +++ b/tests/nvme/018
> @@ -0,0 +1,58 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2018 Sagi Grimberg
> +#
> +# Test NVMe out of range access.
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="unit test NVMe-oF out of range access"
it will be great if you can specify above back-end type (bdev/file)
> +QUICK=1
> +
> +requires() {
> +	_have_program nvme && _have_modules nvme-loop && _have_modules loop && \
> +		_have_configfs

need to check for the blockdev.
> +}
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	modprobe nvmet
> +	modprobe nvme-loop
> +
> +	local port
> +	local nvmedev
> +	local file_path="$TMPDIR/img"
> +	local subsys_name="blktests-subsystem-1"
> +
> +	truncate -s 1G "${file_path}"
> +
> +	_create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> +		 "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> +	port="$(_create_nvmet_port "loop")"
> +	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
> +
> +	nvme connect -t loop -n "${subsys_name}"
> +
> +	nvmedev="$(_find_nvme_loop_dev)"
> +	cat "/sys/block/${nvmedev}n1/uuid"
> +	cat "/sys/block/${nvmedev}n1/wwid"
> +
> +	sectors=$(blockdev --getsz /dev/${nvmedev}n1)
> +	bs=$(blockdev --getbsz /dev/${nvmedev}n1)
> +
> +	nvme read "/dev/${nvmedev}n1" -s $sectors -c 0 -z $bs
> +	if [[ ! $? ]]; then
> +		echo "Sucessfully read out of device lba range"
Can we prefix this message with error as we should get an error for
out of range ?

Also s/Sucessfully/Successfully/.

> +	fi
> +
> +	nvme disconnect -n "${subsys_name}"
> +
> +	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
> +	_remove_nvmet_subsystem "${subsys_name}"
> +	_remove_nvmet_port "${port}"
> +
> +	rm "${file_path}"
> +
> +	echo "Test complete"
> +}
> diff --git a/tests/nvme/018.out b/tests/nvme/018.out
> new file mode 100755
> index 000000000000..0ab3b2a892d3
> --- /dev/null
> +++ b/tests/nvme/018.out
> @@ -0,0 +1,6 @@
> +Running nvme/018
> +91fdba0d-f87b-4c25-b80f-db7be1418b9e
> +uuid.91fdba0d-f87b-4c25-b80f-db7be1418b9e
> +read:CAP_EXCEEDED: The execution of the command has caused the capacity of the namespace to be exceeded(6081)
> +NQN:blktests-subsystem-1 disconnected 1 controller(s)
> +Test complete
> 

[PATCH 2/2 blktests] nvme: test out-of-range I/O access

[Sagi Grimberg]

>> +DESCRIPTION="unit test NVMe-oF out of range access"
> it will be great if you can specify above back-end type (bdev/file)

OK

>> +QUICK=1
>> +
>> +requires() {
>> +	_have_program nvme && _have_modules nvme-loop && _have_modules loop && \
>> +		_have_configfs
> 
> need to check for the blockdev.

Other tests use blockdev and don't check for it..

>> +}
>> +
>> +test() {
>> +	echo "Running ${TEST_NAME}"
>> +
>> +	modprobe nvmet
>> +	modprobe nvme-loop
>> +
>> +	local port
>> +	local nvmedev
>> +	local file_path="$TMPDIR/img"
>> +	local subsys_name="blktests-subsystem-1"
>> +
>> +	truncate -s 1G "${file_path}"
>> +
>> +	_create_nvmet_subsystem "${subsys_name}" "${file_path}" \
>> +		 "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
>> +	port="$(_create_nvmet_port "loop")"
>> +	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
>> +
>> +	nvme connect -t loop -n "${subsys_name}"
>> +
>> +	nvmedev="$(_find_nvme_loop_dev)"
>> +	cat "/sys/block/${nvmedev}n1/uuid"
>> +	cat "/sys/block/${nvmedev}n1/wwid"
>> +
>> +	sectors=$(blockdev --getsz /dev/${nvmedev}n1)
>> +	bs=$(blockdev --getbsz /dev/${nvmedev}n1)
>> +
>> +	nvme read "/dev/${nvmedev}n1" -s $sectors -c 0 -z $bs
>> +	if [[ ! $? ]]; then
>> +		echo "Sucessfully read out of device lba range"
> Can we prefix this message with error as we should get an error for
> out of range ?
> 
> Also s/Sucessfully/Successfully/.

Will do.

[PATCH 2/2 blktests] nvme: test out-of-range I/O access

[Chaitanya Kulkarni]

On 3/6/19 12:28 PM, Sagi Grimberg wrote:
> 
>>> +DESCRIPTION="unit test NVMe-oF out of range access"
>> it will be great if you can specify above back-end type (bdev/file)
> 
> OK
> 
>>> +QUICK=1
>>> +
>>> +requires() {
>>> +	_have_program nvme && _have_modules nvme-loop && _have_modules loop && \
>>> +		_have_configfs
>>
>> need to check for the blockdev.
> 
> Other tests use blockdev and don't check for it..

Yes, while looking at the code, I've also observed we don't check for 
the truncate as well. At least for the NVMe I'll send a patch-series to 
fix that for all the other tests.
> 
>>> +}
>>> +
>>> +test() {
>>> +	echo "Running ${TEST_NAME}"
>>> +
>>> +	modprobe nvmet
>>> +	modprobe nvme-loop
>>> +
>>> +	local port
>>> +	local nvmedev
>>> +	local file_path="$TMPDIR/img"
>>> +	local subsys_name="blktests-subsystem-1"
>>> +
>>> +	truncate -s 1G "${file_path}"
>>> +
>>> +	_create_nvmet_subsystem "${subsys_name}" "${file_path}" \
>>> +		 "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
>>> +	port="$(_create_nvmet_port "loop")"
>>> +	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
>>> +
>>> +	nvme connect -t loop -n "${subsys_name}"
>>> +
>>> +	nvmedev="$(_find_nvme_loop_dev)"
>>> +	cat "/sys/block/${nvmedev}n1/uuid"
>>> +	cat "/sys/block/${nvmedev}n1/wwid"
>>> +
>>> +	sectors=$(blockdev --getsz /dev/${nvmedev}n1)
>>> +	bs=$(blockdev --getbsz /dev/${nvmedev}n1)
>>> +
>>> +	nvme read "/dev/${nvmedev}n1" -s $sectors -c 0 -z $bs
>>> +	if [[ ! $? ]]; then
>>> +		echo "Sucessfully read out of device lba range"
>> Can we prefix this message with error as we should get an error for
>> out of range ?
>>
>> Also s/Sucessfully/Successfully/.
> 
> Will do.
> 

[PATCH 2/2 blktests] nvme: test out-of-range I/O access

[Johannes Thumshirn]

On 06/03/2019 21:31, Chaitanya Kulkarni wrote:
> On 3/6/19 12:28 PM, Sagi Grimberg wrote:
>>
>>>> +DESCRIPTION="unit test NVMe-oF out of range access"
>>> it will be great if you can specify above back-end type (bdev/file)
>>
>> OK
>>
>>>> +QUICK=1
>>>> +
>>>> +requires() {
>>>> +	_have_program nvme && _have_modules nvme-loop && _have_modules loop && \
>>>> +		_have_configfs
>>>
>>> need to check for the blockdev.
>>
>> Other tests use blockdev and don't check for it..
> 
> Yes, while looking at the code, I've also observed we don't check for 
> the truncate as well. At least for the NVMe I'll send a patch-series to 
> fix that for all the other tests.


Truncate is in coreutils and IIRC util-linux and coreutils are the olny
dependency blktests has, so this should be ok.


-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 2/2 blktests] nvme: test out-of-range I/O access

[Omar Sandoval]

On Wed, Mar 06, 2019@01:46:43AM -0800, Sagi Grimberg wrote:
> Test that we correctly fail an out-of-range access
> with a proper NVMe status code.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>  tests/nvme/018     | 58 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/nvme/018.out |  6 +++++
>  2 files changed, 64 insertions(+)
>  create mode 100755 tests/nvme/018
>  create mode 100755 tests/nvme/018.out
> 
> diff --git a/tests/nvme/018 b/tests/nvme/018
> new file mode 100755
> index 000000000000..939573263c87
> --- /dev/null
> +++ b/tests/nvme/018
> @@ -0,0 +1,58 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2018 Sagi Grimberg
> +#
> +# Test NVMe out of range access.

Is this a test for a specific kernel commit? It fails on 5.0.

[PATCH 1/2 blktests] nvme: add udevadm settle before disconnecting

[Omar Sandoval]

On Wed, Mar 06, 2019@01:46:42AM -0800, Sagi Grimberg wrote:
> otherwise we can see some I/O failures as udev didn't get enough time
> to do its thing.

Thanks, applied this one. I'll wait for your response on patch 2.

> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>  tests/nvme/008 | 2 ++
>  tests/nvme/009 | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/tests/nvme/008 b/tests/nvme/008
> index d73ec7d6d97a..04ff0bda42e3 100755
> --- a/tests/nvme/008
> +++ b/tests/nvme/008
> @@ -41,6 +41,8 @@ test() {
>  	cat "/sys/block/${nvmedev}n1/uuid"
>  	cat "/sys/block/${nvmedev}n1/wwid"
>  
> +	udevadm settle
> +
>  	nvme disconnect -n "${subsys_name}"
>  
>  	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
> diff --git a/tests/nvme/009 b/tests/nvme/009
> index a46168123c56..81e61f99aba5 100755
> --- a/tests/nvme/009
> +++ b/tests/nvme/009
> @@ -37,6 +37,8 @@ test() {
>  	cat "/sys/block/${nvmedev}n1/uuid"
>  	cat "/sys/block/${nvmedev}n1/wwid"
>  
> +	udevadm settle
> +
>  	nvme disconnect -n "${subsys_name}"
>  
>  	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
> -- 
> 2.17.1
> 

[PATCH 2/2 blktests] nvme: test out-of-range I/O access

[Sagi Grimberg]

>> Test that we correctly fail an out-of-range access
>> with a proper NVMe status code.
>>
>> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
>> ---
>>   tests/nvme/018     | 58 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/nvme/018.out |  6 +++++
>>   2 files changed, 64 insertions(+)
>>   create mode 100755 tests/nvme/018
>>   create mode 100755 tests/nvme/018.out
>>
>> diff --git a/tests/nvme/018 b/tests/nvme/018
>> new file mode 100755
>> index 000000000000..939573263c87
>> --- /dev/null
>> +++ b/tests/nvme/018
>> @@ -0,0 +1,58 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2018 Sagi Grimberg
>> +#
>> +# Test NVMe out of range access.
> 
> Is this a test for a specific kernel commit? It fails on 5.0.

It failed when nvmet file backend was introduced but fixed in one
of the -rc cycles. Thought it'd be worth keeping the test though...

I'll send v2

[PATCH 2/2 blktests] nvme: test out-of-range I/O access

[Omar Sandoval]

On Mon, Mar 11, 2019@03:14:27PM -0700, Sagi Grimberg wrote:
> 
> > > Test that we correctly fail an out-of-range access
> > > with a proper NVMe status code.
> > > 
> > > Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> > > ---
> > >   tests/nvme/018     | 58 ++++++++++++++++++++++++++++++++++++++++++++++
> > >   tests/nvme/018.out |  6 +++++
> > >   2 files changed, 64 insertions(+)
> > >   create mode 100755 tests/nvme/018
> > >   create mode 100755 tests/nvme/018.out
> > > 
> > > diff --git a/tests/nvme/018 b/tests/nvme/018
> > > new file mode 100755
> > > index 000000000000..939573263c87
> > > --- /dev/null
> > > +++ b/tests/nvme/018
> > > @@ -0,0 +1,58 @@
> > > +#!/bin/bash
> > > +# SPDX-License-Identifier: GPL-3.0+
> > > +# Copyright (C) 2018 Sagi Grimberg
> > > +#
> > > +# Test NVMe out of range access.
> > 
> > Is this a test for a specific kernel commit? It fails on 5.0.
> 
> It failed when nvmet file backend was introduced but fixed in one
> of the -rc cycles. Thought it'd be worth keeping the test though...

Do you know which patch fixed it, so I can add a reference to it in the
test?

[PATCH 2/2 blktests] nvme: test out-of-range I/O access

[Sagi Grimberg]

> Do you know which patch fixed it, so I can add a reference to it in the
> test?

commit 9c891c139894ce2ec0ca585a00e0bec5e6b4ccab
Author: Sagi Grimberg <sagi at grimberg.me>
Date:   Wed Jul 11 16:13:04 2018 +0300

     nvmet: check fileio lba range access boundaries

     Fail out-of-bounds with a proper status code.

     Fixes: d5eff33ee6f8 ("nvmet: add simple file backed ns support")
     Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
     Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
     Signed-off-by: Christoph Hellwig <hch at lst.de>