* [PATCH v2] *** Discovery feature fix in nvme-cli *** @ 2016-07-29 20:38 Jay Freyensee 2016-07-29 20:38 ` [PATCH v2] nvme-cli: user-defined hostnqn option for discover Jay Freyensee 0 siblings, 1 reply; 6+ messages in thread From: Jay Freyensee @ 2016-07-29 20:38 UTC (permalink / raw) 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). Changes since v1: patch formatting lint, no changed functionality. Changes since v0: changed short 'h' option to 'q'. Jay Freyensee (1): nvme-cli: user-defined hostnqn option for discover fabrics.c | 11 +++++++++++ 1 file changed, 11 insertions(+) -- 2.4.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] nvme-cli: user-defined hostnqn option for discover 2016-07-29 20:38 [PATCH v2] *** Discovery feature fix in nvme-cli *** Jay Freyensee @ 2016-07-29 20:38 ` Jay Freyensee 2016-08-01 11:18 ` Christoph Hellwig 0 siblings, 1 reply; 6+ messages in thread From: Jay Freyensee @ 2016-07-29 20:38 UTC (permalink / raw) 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]# Signed-off-by: Jay Freyensee <james_p_freyensee at linux.intel.com> Signed-off-by: Roy Shterman <roysh at mellanox.com> Reviewed-by: Sagi Grimberg <sagi at grimberg.me> --- changes since v1: tags, patch formatting, no functionality change changes since v0: changed short 'h' option to 'q' fabrics.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fabrics.c b/fabrics.c index 3666a01..41a80df 100644 --- a/fabrics.c +++ b/fabrics.c @@ -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", 'q', "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}, -- 2.4.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] nvme-cli: user-defined hostnqn option for discover 2016-07-29 20:38 ` [PATCH v2] nvme-cli: user-defined hostnqn option for discover Jay Freyensee @ 2016-08-01 11:18 ` Christoph Hellwig 2016-08-01 21:32 ` J Freyensee 0 siblings, 1 reply; 6+ messages in thread From: Christoph Hellwig @ 2016-08-01 11:18 UTC (permalink / raw) On Fri, Jul 29, 2016@01:38:39PM -0700, Jay Freyensee wrote: > 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: Maybe -n or -N would be a good choice for the short option flag? Otherwise this looks fine to me: Reviewed-by: Christoph Hellwig <hch at lst.de> Any chance you could also send a patch that reads /etc/nvme/hostnqn for a default hostnqn so that we can have a persistent one? ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] nvme-cli: user-defined hostnqn option for discover 2016-08-01 11:18 ` Christoph Hellwig @ 2016-08-01 21:32 ` J Freyensee 2016-08-02 6:48 ` Sagi Grimberg 0 siblings, 1 reply; 6+ messages in thread From: J Freyensee @ 2016-08-01 21:32 UTC (permalink / raw) On Mon, 2016-08-01@04:18 -0700, Christoph Hellwig wrote: > On Fri, Jul 29, 2016@01:38:39PM -0700, Jay Freyensee wrote: > > 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: > > Maybe -n or -N would be a good choice for the short option flag? I thought Roy's suggestion for -q was fine. > > Otherwise this looks fine to me: > > Reviewed-by: Christoph Hellwig <hch at lst.de> Cool, thanks. > > Any chance you could also send a patch that reads /etc/nvme/hostnqn > for a default hostnqn so that we can have a persistent one? I'm a tad unclear what is the request? The default hostnqn is buried inside /dev/nvme-fabrics. You want nvme-cli to read out the default hostnqn from /dev/nvme-fabrics and place it in /etc/nvme/hostnqn? > > _______________________________________________ > Linux-nvme mailing list > Linux-nvme at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] nvme-cli: user-defined hostnqn option for discover 2016-08-01 21:32 ` J Freyensee @ 2016-08-02 6:48 ` Sagi Grimberg 2016-08-02 23:31 ` J Freyensee 0 siblings, 1 reply; 6+ messages in thread From: Sagi Grimberg @ 2016-08-02 6:48 UTC (permalink / raw) >> Any chance you could also send a patch that reads /etc/nvme/hostnqn >> for a default hostnqn so that we can have a persistent one? > > I'm a tad unclear what is the request? The default hostnqn is buried > inside /dev/nvme-fabrics. You want nvme-cli to read out the default > hostnqn from /dev/nvme-fabrics and place it in /etc/nvme/hostnqn? I think Christoph is requesting that when the user doesn't pass a hostnqn, nvme-cli will try to read it from a file located in /etc/nvme/hostnqn (and if this doesn't exist then go ahead without it). This way, the host can have a persistent hostnqn across reboots (the driver generates one on module init time). ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] nvme-cli: user-defined hostnqn option for discover 2016-08-02 6:48 ` Sagi Grimberg @ 2016-08-02 23:31 ` J Freyensee 0 siblings, 0 replies; 6+ messages in thread From: J Freyensee @ 2016-08-02 23:31 UTC (permalink / raw) On Tue, 2016-08-02@09:48 +0300, Sagi Grimberg wrote: > > > Any chance you could also send a patch that reads > > > /etc/nvme/hostnqn > > > for a default hostnqn so that we can have a persistent one? > > > > I'm a tad unclear what is the request? The default hostnqn is > > buried > > inside /dev/nvme-fabrics. You want nvme-cli to read out the > > default > > hostnqn from /dev/nvme-fabrics and place it in /etc/nvme/hostnqn? > > I think Christoph is requesting that when the user doesn't pass a > hostnqn, nvme-cli will try to read it from a file located in > /etc/nvme/hostnqn (and if this doesn't exist then go ahead without > it). This way, the host can have a persistent hostnqn across reboots > (the driver generates one on module init time). hmmm...the scenario I set up that found this issue, the host generates a default hostnqn in the nvme-fabrics driver at init(). And if there is no --hostnqn flag passed, the /dev/nvme-fabrics driver will use the default generated by the nvme-fabrics driver at init(). And no /etc/nvme/hostnqn file gets created or exists currently. So is the request to have nvme-cli use whatever is in /etc/nvme/hostnqn (if it exists) over the default in the nvme-fabrics driver? So preference of hostnqn input would be: 1. --hostnqn flag 2. /etc/nvme/hostnqn 3. default hostnqn generated by nvme-fabrics driver (ex: nqn.2014 -08.org.nvmexpress:NVMf:uuid:a2d7752c-a31b-477a-a003-31a5e1c424a9) (note 2. may contain a hostnqn different from the default) ?? ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-02 23:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-07-29 20:38 [PATCH v2] *** Discovery feature fix in nvme-cli *** Jay Freyensee 2016-07-29 20:38 ` [PATCH v2] nvme-cli: user-defined hostnqn option for discover Jay Freyensee 2016-08-01 11:18 ` Christoph Hellwig 2016-08-01 21:32 ` J Freyensee 2016-08-02 6:48 ` Sagi Grimberg 2016-08-02 23:31 ` J Freyensee
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).