[PATCH V3 00/11] nvmet: fixes and some cleanups

Chaitanya Kulkarni
2021-02-10T05:48:27Z

Hi,

This patch-series fixes commit bffcd507780e
("nvmet: set right status on error in id-ns handler") and does cleanup
for nvmet_find_namespace() to get rid of the duplicate code and remove
the inconsistent error behavior of the host, Also this adds an helper
for unhandled commands to have uniform error message reporting for the
bdev, file and passthru backend. Patch 9/10 fix compilation warnings.
Last patch removes an unnecessary else at the tail of the
nvmet_parse_io_cmd().

This is based on nvme-5.12. All the blktests are seem to pass on this
series.

Below is the testlog for latest change.

-ck

Changes from V2:-

1. Set the status to 0 when nvmet_req_find_ns() returns an error
for nvmet_execute_identify_ns().

Changes from V1:-

1, In nvmet_execute_identify_ns() zeroout buffer for unallocated nsid.
2. Adjust the patch #2 to reflect above behavior.
3. Merge patch #3 and #4 from V1 to change the nvmet_find_namespace()
prototype to only accept request.
4. Rename nvmet_find_namespace()->nvmet_req_find_namespace().
5. Remove the backened string print from nvmet_report_invalid_opcode() &
replace pr_err() with pr_debug().
6. Add a new patch that creates a helper nvmet_req_subsys() to replace
a chain of the structures all over code to get the subsys from
nvmet_req.
7. Add a patch to remove the unnecessay else at the tail of the
nvmet_parse_io_cmd().

Chaitanya Kulkarni (11):
nvmet: set status to 0 in case for invalid nsid
nvmet: return uniform error for invalid ns
nvmet: make nvmet_find_namespace() req based
nvmet: remove extra variable in id-ns handler
nvmet: add helper to report invalid opcode
nvmet: use invalid cmd opcode helper
nvmet: use invalid cmd opcode helper
nvmet: use min of device_path and disk len
nvme-loop: rename variable to get rid of the warn
nvmet: add nvmet_req_subsys() helper
nvmet: remove else at the end of the function

drivers/nvme/target/admin-cmd.c | 59 +++++++++++++------------------
drivers/nvme/target/core.c | 28 ++++++++-------
drivers/nvme/target/io-cmd-bdev.c | 5 +--
drivers/nvme/target/io-cmd-file.c | 5 +--
drivers/nvme/target/loop.c | 2 +-
drivers/nvme/target/nvmet.h | 18 ++++++++--
drivers/nvme/target/passthru.c | 6 ++--
drivers/nvme/target/trace.h | 4 ++-
8 files changed, 64 insertions(+), 63 deletions(-)

# gitlog -11
2bd1fdd47f27 (HEAD -> nvme-5.12) nvmet: remove else at the end of the function
f3ba4d069b9a nvmet: add nvmet_req_subsys() helper
f72b99a93c10 nvme-loop: rename variable to get rid of the warn
b8caa57f886b nvmet: use min of device_path and disk len
736b56163c13 nvmet: use invalid cmd opcode helper
7ddd60b3d6cf nvmet: use invalid cmd opcode helper
1f83a1024596 nvmet: add helper to report invalid opcode
a3904c9ca583 nvmet: remove extra variable in id-ns handler
c6d99eca6ec9 nvmet: make nvmet_find_namespace() req based
cfe473bf95a2 nvmet: return uniform error for invalid ns
87c302ac8edc nvmet: set status to 0 in case for invalid nsid

# Create 1 subsys with 1 ns
++ FILE=/dev/nvme0n1
++ NN=1
++ NQN=testnqn
++ let NR_DEVICES=NN+1
++ modprobe -r null_blk
++ modprobe null_blk nr_devices=0
++ modprobe nvme
++ modprobe nvme-fabrics
++ modprobe nvmet
++ modprobe nvme-loop
++ dmesg -c
++ sleep 2
++ tree /sys/kernel/config
/sys/kernel/config
├── nullb
│   └── features
└── nvmet
├── hosts
├── ports
└── subsystems

5 directories, 1 file
++ mkdir /sys/kernel/config/nvmet/subsystems/testnqn
++ mkdir /sys/kernel/config/nvmet/ports/1/
++ echo -n loop
++ echo -n 1
++ ln -s /sys/kernel/config/nvmet/subsystems/testnqn /sys/kernel/config/nvmet/ports/1/subsystems/
++ sleep 1
++ echo transport=loop,nqn=testnqn
+++ shuf -i 1-1 -n 1
++ for i in '`shuf -i 1-$NN -n $NN`'
++ mkdir config/nullb/nullb1
++ echo 4096
++ echo 20971520
++ echo 1
+++ cat config/nullb/nullb1/index
++ IDX=0
++ mkdir /sys/kernel/config/nvmet/subsystems/testnqn/namespaces/1
++ echo ' ####### /dev/nullb0'
####### /dev/nullb0
++ echo -n /dev/nullb0
++ cat /sys/kernel/config/nvmet/subsystems/testnqn/namespaces/1/device_path
/dev/nullb0
++ echo 1
++ dmesg -c
[24762.913991] nvmet: creating controller 1 for subsystem testnqn for NQN nqn.2014-08.org.nvmexpress:uuid:6a4a7451-6274-4d7f-93d3-49724cc1147e.
[24762.914802] nvme nvme1: Please enable CONFIG_NVME_MULTIPATH for full support of multi-port devices.
[24762.915034] nvme nvme1: creating 64 I/O queues.
[24762.932598] nvme nvme1: new ctrl: "testnqn"
[24762.966156] nvmet: adding nsid 1 to subsystem testnqn
[24762.972384] nvme nvme1: rescanning namespaces.
++ sleep 1
++ mount
++ column -t
++ grep nvme
++ '[' 1 ']'
+++ wc -l
+++ ls -l /dev/nvme1 /dev/nvme1n1
++ cnt=2
++ echo 2
2
++ '[' 2 -gt 1 ']'
++ break
++ dmesg -c
#
# nvme list
Node SN Model Namespace Usage Format FW Rev
---------------- -------------------- ---------------------------------------- --------- -------------------------- ---------------- --------
/dev/nvme0n1 foo QEMU NVMe Ctrl 1 1.07 GB / 1.07 GB 512 B + 0 B 1.0
/dev/nvme1n1 fc13d75abd55eafa Linux 1 21.99 TB / 21.99 TB 4 KiB + 0 B 5.11.0-r
# nvme id-ns /dev/nvme1 -n 1
NVME Identify Namespace 1:
nsze : 0x140000000
ncap : 0x140000000
nuse : 0x140000000
nsfeat : 0x12
nlbaf : 0
flbas : 0
mc : 0
dpc : 0
dps : 0
nmic : 0x1
rescap : 0
fpi : 0
dlfeat : 0
nawun : 0
nawupf : 0
nacwu : 0
nabsn : 0
nabo : 0
nabspf : 0
noiob : 0
nvmcap : 0
npwg : 0
npwa : 0
npdg : 0
npda : 0
nows : 0
mssrl : 0
mcl : 0
msrc : 0
nsattr : 0
nvmsetid: 0
anagrpid: 1
endgid : 0
nguid : 00000000000000000000000000000000
eui64 : 0000000000000000
lbaf 0 : ms:0 lbads:12 rp:0 (in use)
# Make sure we get the status 0 for identify namesapce command
# for unallocated nsid
# nvme id-ns /dev/nvme1 -n 2
NVME Identify Namespace 2:
nsze : 0
ncap : 0
nuse : 0
nsfeat : 0
nlbaf : 0
flbas : 0
mc : 0
dpc : 0
dps : 0
nmic : 0
rescap : 0
fpi : 0
dlfeat : 0
nawun : 0
nawupf : 0
nacwu : 0
nabsn : 0
nabo : 0
nabspf : 0
noiob : 0
nvmcap : 0
mssrl : 0
mcl : 0
msrc : 0
nsattr : 0
nvmsetid: 0
anagrpid: 0
endgid : 0
nguid : 00000000000000000000000000000000
eui64 : 0000000000000000
lbaf 0 : ms:0 lbads:0 rp:0 (in use)