*** Discovery feature fix in nvme-cli ***
PATCH 1/1 *** Discovery feature fix in nvme-cli ***
Since the only way to do the discovery feature of NVMe-over-Fabrics is through nvme-cli, I thought it will be appropriate to send a patch to this email list (if nothing more than to inform and educate).
I'll push this to Keith Busch (the nvme-cli owner).
Jay Freyensee (1):
nvme-cli: user-defined hostnqn option for discover
fabrics.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
Signature · 1 line
2.4.3
PATCH 1/1 nvme-cli: user-defined hostnqn option for discover
The nvme-cli will always use the default hostnqn
in /dev/nvme-fabrics for the discovery query, even though both the NVMe Target and NVMe Host rdma implementations allow user-defined hostnqn naming.
For example, this is the current, somewhat broken behavior if you used your own hostnqn provision naming on the NVMe kernel target:
nvme discover /dev/nvme-fabrics -t rdma --traddr=192.168.1.3 --trsvcid=4420
in dmesg:
[591910.025779] nvme nvme0: Connect Invalid Data Parameter, hostnqn "nqn.2014-08.org.nvmexpress:NVMf:uuid:a2d7752c-a31b-477a-a003-31a5e1c424a9"
New, fixed behavior introduced by this patch:
[root at fedora23-fabrics-host1 nvme-cli]# nvme discover -t rdma --traddr=192.168.1.3 --trsvcid=4420 --hostnqn=host1-rogue-nqn
Discovery Log Number of Records 1, Generation counter 10 =====Discovery Log Entry 0======
trtype: ipv4 adrfam: rdma nqntype: 2 treq: 0 portid: 1 trsvcid: 4420 subnqn: nullside-nqn traddr: 192.168.1.3 rdma_prtype: 0 rdma_qptype: 0 rdma_cms: 0 rdma_pkey: 0x0000
[root at fedora23-fabrics-host1 nvme-cli]#
--- fabrics.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
fabrics.c +11 -0
@@ -49,6 +49,7 @@ struct config {char *transport;char *traddr;char *trsvcid; +char *hostnqn;char *raw;char *device; } cfg = { 0 };
@@ -395,6 +396,14 @@ static int build_options(char *argstr, int max_len) max_len -= len; } +if (cfg.hostnqn) { + len =snprintf (argstr, max_len,",hostnqn=%s" , cfg.hostnqn); +if (len <0 ) +return -EINVAL ; + argstr += len; + max_len -= len; + } +return 0 ; }
@@ -525,6 +534,8 @@ int discover(const char *desc, int argc, char **argv, bool connect) "transport address" }, {"trsvcid" ,'s' ,"LIST" ,CFG_STRING , &cfg.trsvcid, required_argument, "transport service id (e.g. IP port)" }, + {"hostnqn", 'h', "LIST", CFG_STRING, &cfg.hostnqn, required_argument, + "user-defined hostnqn (if default not used)" }, {"raw", 'r', "LIST", CFG_STRING, &cfg.raw, required_argument, "raw output file" }, {0},
Signature · 1 line
2.4.3
PATCH 1/1 nvme-cli: user-defined hostnqn option for discover
Hi Jay,
Hi Jay,
Actually I planned to send the same patch tomorrow,
I think we should add user-defined hostnqn parameter into 'connect' command.
Quoted 66 lines · Jay Freyensee
The nvme-cli will always use the default hostnqn
in/dev/nvme-fabricsfor the discovery query, even though both the NVMe Target and NVMe Host rdma implementations allow user-defined hostnqn naming.For example, this is the current, somewhat broken behavior if you used your own hostnqn provision naming on the NVMe kernel target:
nvme discover
/dev/nvme-fabrics-t rdma --traddr=192.168.1.3 --trsvcid=4420in dmesg:
[591910.025779] nvme nvme0: Connect Invalid Data Parameter, hostnqn "nqn.2014-08.org.nvmexpress:NVMf:uuid:a2d7752c-a31b-477a-a003-31a5e1c424a9"New, fixed behavior introduced by this patch:
[root at fedora23-fabrics-host1 nvme-cli]# nvme discover -t rdma --traddr=192.168.1.3 --trsvcid=4420 --hostnqn=host1-rogue-nqn
Discovery Log Number of Records 1, Generation counter 10 =====Discovery Log Entry 0======
trtype: ipv4 adrfam: rdma nqntype: 2 treq: 0 portid: 1 trsvcid: 4420 subnqn: nullside-nqn traddr: 192.168.1.3 rdma_prtype: 0 rdma_qptype: 0 rdma_cms: 0 rdma_pkey: 0x0000[root at fedora23-fabrics-host1 nvme-cli]#
Signed-off-by Jay Freyensee <james_p_freyensee at linux.intel.com>--- fabrics.c | 11 +++++++++++ 1 file changed, 11 insertions(+)fabrics.c +9
-0@@ -49,6 +49,7 @@ struct config {char *transport;char *traddr;char *trsvcid; +char *hostnqn;char *raw;char *device; } cfg = { 0 };@@ -395,6 +396,14 @@ static int build_options(char *argstr, int max_len) max_len -= len; } +if (cfg.hostnqn) { + len =snprintf (argstr, max_len,",hostnqn=%s" , cfg.hostnqn); +if (len <0 ) +return -EINVAL ; + argstr += len; + max_len -= len; + } +return 0 ; }
fabrics.c int discover(const char *desc, int argc, char **argv, bool connect) · line 538
Quoted 5 lines
fabrics.c +2
-0@@ -525,6 +534,8 @@ int discover(const char *desc, int argc, char **argv, bool connect) "transport address" }, {"trsvcid" ,'s' ,"LIST" ,CFG_STRING , &cfg.trsvcid, required_argument, "transport service id (e.g. IP port)" }, + {"hostnqn", 'h', "LIST", CFG_STRING, &cfg.hostnqn, required_argument, + "user-defined hostnqn (if default not used)" },
I believe 'h' short option is already dedicated for help parameter. maybe we should change it to different character?
Quoted 3 lines
fabrics.c +0
-0{"raw" ,'r' ,"LIST" ,CFG_STRING , &cfg.raw, required_argument, "raw output file" }, {0},
PATCH 1/1 nvme-cli: user-defined hostnqn option for discover
Quoted 5 lines · Roy Shterman
Hi Jay,
Actually I planned to send the same patch tomorrow,
I think we should add user-defined hostnqn parameter into 'connect' command.
Is it okay to apply this as-is, or should I wait for the patch for connect?
PATCH 1/1 nvme-cli: user-defined hostnqn option for discover
Quoted 10 lines · Keith Busch
Quoted 6 lines · Roy Shterman
Hi Jay,
Actually I planned to send the same patch tomorrow,
I think we should add user-defined hostnqn parameter into 'connect' command.
Is it okay to apply this as-is, or should I wait for the patch for connect?
I think we should break up the patches as one for discover, and one for connect. Each patch can show a usage for each of the commands (one for 'discover', one for 'connect'.
How about apply my discover patch as-is, then Roy re-submit his patch for connect? And in the connect patch, supply an example usage (which I think this will be useful as there is currently no man pages for these new fabrics commands).
Quoted 5 lines
_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH RESEND 1/1 nvme-cli: user-defined hostnqn option for discover
Quoted 5 lines · Roy Shterman
Sounds good to me, but what about the short option character?
I think using 'h' as a short option will not work because it is already dedicated for --help parameter.
That is a good point :-/.
OK, I'll respin my discover patch to use 'q' short option you chose. I'll send the patch out shortly.
Quoted 37 lines
I will send my patch later only for connect after Keith will merge discover patch.
Thanks,
Roy?-29 ????? 2016, ???? 00:22, ??J Freyensee ?<
james_p_freyenseeat linux.intel.com> ???/?:?Quoted 28 lines · Keith Busch
Quoted 9 lines · Roy Shterman
Quoted 5 lines
Hi Jay,
Actually I planned to send the same patch tomorrow, I think we should add user-defined hostnqn parameter into 'connect'
command.Is it okay to apply this as-is, or should I wait for the patch for
connect?I think we should break up the patches as one for discover, and one for
connect. Each patch can show a usage for each of the commands (one for
'discover', one for 'connect'.How about apply my discover patch as-is, then Roy re-submit his patch
for connect? And in the connect patch, supply an example usage (which
I think this will be useful as there is currently no man pages for these new fabrics commands).Quoted 4 lines
_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme