* [PATCH v2 01/10] nvme-cli: ignore arguments that pass in "none"
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
@ 2019-07-19 22:52 ` James Smart
2019-07-19 22:52 ` [PATCH v2 02/10] nvme-cli: support persistent connections to a discovery controller James Smart
` (9 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-19 22:52 UTC (permalink / raw)
As we want to support discovery uevents over different
transports, we want to allow the kernel to provide missing
information in the form of none and have nvme-cli properly
ignore it.
One example is the host_traddr. If it is not set (which means
that the default route determined the host address) we will
want to do the same for newly discovered controllers.
So allow users to pass 'none' arguments as well.
Example:
nvme connect-all ... --hostnqn=none --hostid=none --host_traddr=none
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
Signed-off-by: James Smart <jsmart2021 at gmail.com>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>
---
fabrics.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fabrics.c b/fabrics.c
index 32c3a9c..5757aaf 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -578,7 +578,7 @@ add_argument(char **argstr, int *max_len, char *arg_str, char *arg)
{
int len;
- if (arg) {
+ if (arg && strcmp(arg, "none")) {
len = snprintf(*argstr, *max_len, ",%s=%s", arg_str, arg);
if (len < 0)
return -EINVAL;
@@ -671,14 +671,14 @@ retry:
return -EINVAL;
p += len;
- if (cfg.hostnqn) {
+ if (cfg.hostnqn && strcmp(cfg.hostnqn, "none")) {
len = sprintf(p, ",hostnqn=%s", cfg.hostnqn);
if (len < 0)
return -EINVAL;
p += len;
}
- if (cfg.hostid) {
+ if (cfg.hostid && strcmp(cfg.hostid, "none")) {
len = sprintf(p, ",hostid=%s", cfg.hostid);
if (len < 0)
return -EINVAL;
@@ -713,7 +713,7 @@ retry:
p += len;
}
- if (cfg.host_traddr) {
+ if (cfg.host_traddr && strcmp(cfg.host_traddr, "none")) {
len = sprintf(p, ",host_traddr=%s", cfg.host_traddr);
if (len < 0)
return -EINVAL;
--
2.13.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 02/10] nvme-cli: support persistent connections to a discovery controller
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
2019-07-19 22:52 ` [PATCH v2 01/10] nvme-cli: ignore arguments that pass in "none" James Smart
@ 2019-07-19 22:52 ` James Smart
2019-07-19 22:52 ` [PATCH v2 03/10] nvme-cli: allow discover to address discovery controller by persistent name James Smart
` (8 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-19 22:52 UTC (permalink / raw)
From: Sagi Grimberg <sagi@grimberg.me>
Simply don't destroy the discovery controller after getting the
log pages. Note that persistent connection to a discovery subsystem
require to pass in a non-zero kato value, so if not provided we
simply use a default of 30 seconds kato.
Example:
nvme connect-all ... --persistent
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
Reviewed-by: James Smart <jsmart2021 at gmail.com>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Reviewed-by: Max Gurtovoy <maxg at mellanox.com>
Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>
---
fabrics.c | 15 ++++++++++++---
fabrics.h | 2 ++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/fabrics.c b/fabrics.c
index 5757aaf..75dedf8 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -66,6 +66,7 @@ static struct config {
int disable_sqflow;
int hdr_digest;
int data_digest;
+ bool persistent;
} cfg = { NULL };
#define BUF_SIZE 4096
@@ -872,9 +873,11 @@ static int do_discover(char *argstr, bool connect)
return -errno;
ret = nvmf_get_log_page_discovery(dev_name, &log, &numrec, &status);
free(dev_name);
- err = remove_ctrl(instance);
- if (err)
- return err;
+ if (!cfg.persistent) {
+ err = remove_ctrl(instance);
+ if (err)
+ return err;
+ }
switch (ret) {
case DISC_OK:
@@ -957,6 +960,9 @@ static int discover_from_conf_file(const char *desc, char *argstr,
if (err)
continue;
+ if (cfg.persistent && !cfg.keep_alive_tmo)
+ cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO;
+
err = build_options(argstr, BUF_SIZE);
if (err) {
ret = err;
@@ -999,6 +1005,7 @@ int discover(const char *desc, int argc, char **argv, bool connect)
{"nr-write-queues", 'W', "LIST", CFG_INT, &cfg.nr_write_queues, required_argument, "number of write queues to use (default 0)" },
{"nr-poll-queues", 'P', "LIST", CFG_INT, &cfg.nr_poll_queues, required_argument, "number of poll queues to use (default 0)" },
{"queue-size", 'Q', "LIST", CFG_INT, &cfg.queue_size, required_argument, "number of io queue elements to use (default 128)" },
+ {"persistent", 'p', "LIST", CFG_NONE, &cfg.persistent, no_argument, "persistent discovery connection" },
{NULL},
};
@@ -1013,6 +1020,8 @@ int discover(const char *desc, int argc, char **argv, bool connect)
ret = discover_from_conf_file(desc, argstr,
command_line_options, connect);
} else {
+ if (cfg.persistent && !cfg.keep_alive_tmo)
+ cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO;
ret = build_options(argstr, BUF_SIZE);
if (ret)
goto out;
diff --git a/fabrics.h b/fabrics.h
index 988f3ef..7c1664b 100644
--- a/fabrics.h
+++ b/fabrics.h
@@ -1,6 +1,8 @@
#ifndef _DISCOVER_H
#define _DISCOVER_H
+#define NVMF_DEF_DISC_TMO 30
+
extern int discover(const char *desc, int argc, char **argv, bool connect);
extern int connect(const char *desc, int argc, char **argv);
extern int disconnect(const char *desc, int argc, char **argv);
--
2.13.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 03/10] nvme-cli: allow discover to address discovery controller by persistent name
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
2019-07-19 22:52 ` [PATCH v2 01/10] nvme-cli: ignore arguments that pass in "none" James Smart
2019-07-19 22:52 ` [PATCH v2 02/10] nvme-cli: support persistent connections to a discovery controller James Smart
@ 2019-07-19 22:52 ` James Smart
2019-07-19 22:52 ` [PATCH v2 04/10] nvme-cli: Refactor to create a get_nvme_ctrl_info routine James Smart
` (7 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-19 22:52 UTC (permalink / raw)
To support discovery (connect/connect-all) to operate against a
persistent discovery controller, let the discovery controller to
be specified by its device node name rather than new connection
attributes.
Example:
nvme connect-all ... --device=nvme5
Also centralize extraction of controller instance from the controller
name to a common helper.
Signed-off-by: James Smart <jsmart2021 at gmail.com>
Reviewed-by: Max Gurtovoy <maxg at mellanox.com>
Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
---
fabrics.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/fabrics.c b/fabrics.c
index 75dedf8..d92c2ff 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -190,6 +190,21 @@ static const char *cms_str(__u8 cm)
static int do_discover(char *argstr, bool connect);
+static int ctrl_instance(char *device)
+{
+ char d[64];
+ int ret, instance;
+
+ device = basename(device);
+ ret = sscanf(device, "nvme%d", &instance);
+ if (ret <= 0)
+ return -EINVAL;
+ if (snprintf(d, sizeof(d), "nvme%d", instance) <= 0 ||
+ strcmp(device, d))
+ return -EINVAL;
+ return instance;
+}
+
static int add_ctrl(const char *argstr)
{
substring_t args[MAX_OPT_ARGS];
@@ -865,7 +880,10 @@ static int do_discover(char *argstr, bool connect)
int instance, numrec = 0, ret, err;
int status = 0;
- instance = add_ctrl(argstr);
+ if (!cfg.device)
+ instance = add_ctrl(argstr);
+ else
+ instance = ctrl_instance(cfg.device);
if (instance < 0)
return instance;
@@ -873,7 +891,7 @@ static int do_discover(char *argstr, bool connect)
return -errno;
ret = nvmf_get_log_page_discovery(dev_name, &log, &numrec, &status);
free(dev_name);
- if (!cfg.persistent) {
+ if (!cfg.device && !cfg.persistent) {
err = remove_ctrl(instance);
if (err)
return err;
@@ -996,6 +1014,7 @@ int discover(const char *desc, int argc, char **argv, bool connect)
{"hostnqn", 'q', "LIST", CFG_STRING, &cfg.hostnqn, required_argument, "user-defined hostnqn (if default not used)" },
{"hostid", 'I', "LIST", CFG_STRING, &cfg.hostid, required_argument, "user-defined hostid (if default not used)"},
{"raw", 'r', "LIST", CFG_STRING, &cfg.raw, required_argument, "raw output file" },
+ {"device", 'd', "LIST", CFG_STRING, &cfg.device, required_argument, "use existing discovery controller device" },
{"keep-alive-tmo", 'k', "LIST", CFG_INT, &cfg.keep_alive_tmo, required_argument, "keep alive timeout period in seconds" },
{"reconnect-delay", 'c', "LIST", CFG_INT, &cfg.reconnect_delay, required_argument, "reconnect timeout period in seconds" },
{"ctrl-loss-tmo", 'l', "LIST", CFG_INT, &cfg.ctrl_loss_tmo, required_argument, "controller loss timeout period in seconds" },
@@ -1014,6 +1033,9 @@ int discover(const char *desc, int argc, char **argv, bool connect)
if (ret)
goto out;
+ if (cfg.device && !strcmp(cfg.device, "none"))
+ cfg.device = NULL;
+
cfg.nqn = NVME_DISC_SUBSYS_NAME;
if (!cfg.transport && !cfg.traddr) {
@@ -1157,15 +1179,10 @@ static int disconnect_by_nqn(char *nqn)
static int disconnect_by_device(char *device)
{
int instance;
- int ret;
-
- device = basename(device);
- ret = sscanf(device, "nvme%d", &instance);
- if (ret < 0)
- return ret;
- if (!ret)
- return -1;
+ instance = ctrl_instance(device);
+ if (instance < 0)
+ return instance;
return remove_ctrl(instance);
}
--
2.13.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 04/10] nvme-cli: Refactor to create a get_nvme_ctrl_info routine
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
` (2 preceding siblings ...)
2019-07-19 22:52 ` [PATCH v2 03/10] nvme-cli: allow discover to address discovery controller by persistent name James Smart
@ 2019-07-19 22:52 ` James Smart
2019-07-19 22:53 ` [PATCH v2 05/10] nvme-cli: extend ctrl_list_item for connect attributes James Smart
` (6 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-19 22:52 UTC (permalink / raw)
In preparation for searching controllers to match with connect args:
Refactor the get_nvme_subsystem_info() routine to take the portion
that creates a ctrl_list_item and sets its values and put it in a
separate get_nvme_ctrl_info() routine.
Signed-off-by: James Smart <jsmart2021 at gmail.com>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
---
nvme.c | 77 ++++++++++++++++++++++++++++++++++++++----------------------------
1 file changed, 45 insertions(+), 32 deletions(-)
diff --git a/nvme.c b/nvme.c
index 6fe99eb..dabbbe0 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1544,10 +1544,50 @@ static void free_ctrl_list_item(struct ctrl_list_item *ctrls)
free(ctrls->ana_state);
}
+static int get_nvme_ctrl_info(char *name, char *path,
+ struct ctrl_list_item *ctrl, __u32 nsid)
+{
+ char ctrl_path[512];
+
+ ctrl->name = strdup(name);
+
+ snprintf(ctrl_path, sizeof(ctrl_path), "%s/%s", path, ctrl->name);
+
+ ctrl->address = get_nvme_ctrl_attr(ctrl_path, "address");
+ if (!ctrl->address) {
+ fprintf(stderr, "%s: failed to get controller address.\n",
+ ctrl->name);
+ goto free_ctrl_items;
+ }
+
+ ctrl->transport = get_nvme_ctrl_attr(ctrl_path, "transport");
+ if (!ctrl->transport) {
+ fprintf(stderr, "%s: failed to get controller transport.\n",
+ ctrl->name);
+ goto free_ctrl_items;
+ }
+
+ ctrl->state = get_nvme_ctrl_attr(ctrl_path, "state");
+ if (!ctrl->state) {
+ fprintf(stderr, "%s: failed to get controller state.\n",
+ ctrl->name);
+ goto free_ctrl_items;
+ }
+
+ if (nsid != NVME_NSID_ALL)
+ ctrl->ana_state = get_nvme_ctrl_path_ana_state(ctrl_path, nsid);
+
+ return 0; /* success */
+
+free_ctrl_items:
+ free_ctrl_list_item(ctrl);
+
+ return 1; /* failure */
+}
+
static int get_nvme_subsystem_info(char *name, char *path,
struct subsys_list_item *item, __u32 nsid)
{
- char ctrl_path[512];
struct dirent **ctrls;
int n, i, ret = 1, ccnt = 0;
@@ -1574,38 +1614,11 @@ static int get_nvme_subsystem_info(char *name, char *path,
item->nctrls = n;
for (i = 0; i < n; i++) {
- item->ctrls[ccnt].name = strdup(ctrls[i]->d_name);
-
- snprintf(ctrl_path, sizeof(ctrl_path), "%s/%s", path,
- item->ctrls[ccnt].name);
-
- item->ctrls[ccnt].address =
- get_nvme_ctrl_attr(ctrl_path, "address");
- if (!item->ctrls[ccnt].address) {
- fprintf(stderr, "failed to get controller[%d] address.\n", i);
- free_ctrl_list_item(&item->ctrls[ccnt]);
- continue;
+ if (get_nvme_ctrl_info(ctrls[i]->d_name, path,
+ &item->ctrls[ccnt], nsid)) {
+ fprintf(stderr, "failed to get controller[%d] info.\n",
+ i);
}
-
- item->ctrls[ccnt].transport =
- get_nvme_ctrl_attr(ctrl_path, "transport");
- if (!item->ctrls[ccnt].transport) {
- fprintf(stderr, "failed to get controller[%d] transport.\n", i);
- free_ctrl_list_item(&item->ctrls[ccnt]);
- continue;
- }
-
- item->ctrls[ccnt].state =
- get_nvme_ctrl_attr(ctrl_path, "state");
- if (!item->ctrls[ccnt].state) {
- fprintf(stderr, "failed to get controller[%d] state.\n", i);
- free_ctrl_list_item(&item->ctrls[ccnt]);
- continue;
- }
-
- if (nsid != NVME_NSID_ALL)
- item->ctrls[ccnt].ana_state =
- get_nvme_ctrl_path_ana_state(ctrl_path, nsid);
ccnt++;
}
--
2.13.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 05/10] nvme-cli: extend ctrl_list_item for connect attributes
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
` (3 preceding siblings ...)
2019-07-19 22:52 ` [PATCH v2 04/10] nvme-cli: Refactor to create a get_nvme_ctrl_info routine James Smart
@ 2019-07-19 22:53 ` James Smart
2019-07-19 22:53 ` [PATCH v2 06/10] nvme-cli: Add routine to compare ctrl_list_item to connect args James Smart
` (5 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-19 22:53 UTC (permalink / raw)
In preparation for searching controllers to match with connect args:
Extend the ctrl_list_item with elements that can be compared against
connect arguments. Extend the get_nvme_ctrl_info() routine to set
the fields.
subsysnqn was added as the ctrl_list_item may now be used outside of
a subsystem listing.
traddr, trsvid, and host_traddr are added. Their values come from
the address attribute. A new parsing routine was added that extracts
the values from the address string.
The new parsing routine and supporting field names strings are
declared as a global interface as a subsequent patch will call it
from the fabrics routines.
Signed-off-by: James Smart <jsmart2021 at gmail.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
Reviewed-by: Hannes Reinecke <hare at suse.com>
---
nvme.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
nvme.h | 10 ++++++++++
2 files changed, 82 insertions(+)
diff --git a/nvme.c b/nvme.c
index dabbbe0..66ba2fc 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1542,6 +1542,64 @@ static void free_ctrl_list_item(struct ctrl_list_item *ctrls)
free(ctrls->address);
free(ctrls->state);
free(ctrls->ana_state);
+ free(ctrls->subsysnqn);
+ free(ctrls->traddr);
+ free(ctrls->trsvcid);
+ free(ctrls->host_traddr);
+}
+
+static const char delim_space = ' ';
+const char *conarg_traddr = "traddr";
+const char *conarg_trsvcid = "trsvcid";
+const char *conarg_host_traddr = "host_traddr";
+
+/*
+ * parse strings with connect arguments to find a particular field.
+ * If field found, return string containing field value. If field
+ * not found, return an empty string.
+ */
+char *__parse_connect_arg(char *conargs, const char delim, const char *fieldnm)
+{
+ char *s, *e;
+ size_t cnt;
+
+ /*
+ * There are field name overlaps: traddr and host_traddr.
+ * By chance, both connect arg strings are set up to
+ * have traddr field followed by host_traddr field. Thus field
+ * name matching doesn't overlap in the searches. Technically,
+ * as is, the loop and delimiter checking isn't necessary.
+ * However, better to be prepared.
+ */
+ do {
+ s = strstr(conargs, fieldnm);
+ if (!s)
+ goto empty_field;
+ /* validate prior character is delimiter */
+ if (s == conargs || *(s - 1) == delim) {
+ /* match requires next character to be assignment */
+ s += strlen(fieldnm);
+ if (*s == '=')
+ /* match */
+ break;
+ }
+ /* field overlap: seek to delimiter and keep looking */
+ conargs = strchr(s, delim);
+ if (!conargs)
+ goto empty_field;
+ conargs++; /* skip delimiter */
+ } while (1);
+ s++; /* skip assignment character */
+ e = strchr(s, delim);
+ if (e)
+ cnt = e - s;
+ else
+ cnt = strlen(s);
+
+ return strndup(s, cnt);
+
+empty_field:
+ return strdup("\0");
}
static int get_nvme_ctrl_info(char *name, char *path,
@@ -1577,6 +1635,20 @@ static int get_nvme_ctrl_info(char *name, char *path,
if (nsid != NVME_NSID_ALL)
ctrl->ana_state = get_nvme_ctrl_path_ana_state(ctrl_path, nsid);
+ ctrl->subsysnqn = get_nvme_ctrl_attr(ctrl_path, "subsysnqn");
+ if (!ctrl->subsysnqn) {
+ fprintf(stderr, "%s: failed to get controller subsysnqn.\n",
+ ctrl->name);
+ goto free_ctrl_items;
+ }
+
+ ctrl->traddr = __parse_connect_arg(ctrl->address, delim_space,
+ conarg_traddr);
+ ctrl->trsvcid = __parse_connect_arg(ctrl->address, delim_space,
+ conarg_trsvcid);
+ ctrl->host_traddr = __parse_connect_arg(ctrl->address, delim_space,
+ conarg_host_traddr);
+
return 0; /* success */
free_ctrl_items:
diff --git a/nvme.h b/nvme.h
index 3409515..470f702 100644
--- a/nvme.h
+++ b/nvme.h
@@ -160,6 +160,10 @@ struct ctrl_list_item {
char *transport;
char *state;
char *ana_state;
+ char *subsysnqn;
+ char *traddr;
+ char *trsvcid;
+ char *host_traddr;
};
struct subsys_list_item {
@@ -175,6 +179,12 @@ enum {
BINARY,
};
+char *__parse_connect_arg(char *conargs, const char delim, const char *fieldnm);
+
+extern const char *conarg_traddr;
+extern const char *conarg_trsvcid;
+extern const char *conarg_host_traddr;
+
void register_extension(struct plugin *plugin);
#include "argconfig.h"
--
2.13.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 06/10] nvme-cli: Add routine to compare ctrl_list_item to connect args
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
` (4 preceding siblings ...)
2019-07-19 22:53 ` [PATCH v2 05/10] nvme-cli: extend ctrl_list_item for connect attributes James Smart
@ 2019-07-19 22:53 ` James Smart
2019-07-19 22:53 ` [PATCH v2 07/10] nvme-cli: Add routine to search for controller with specific attributes James Smart
` (4 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-19 22:53 UTC (permalink / raw)
In preparation for searching controllers to match with connect args:
Create a new routine that receives a structure containing connect
arguments and a device name. The routine will build and fill in a
ctrl_list_item for the device and compare its attributes with the
connect arguments. The routine returns true/false on whether they
match.
Routine is defined as a global as a subsequent patch will use it
from the fabrics routines.
Signed-off-by: James Smart <jsmart2021 at gmail.com>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
---
fabrics.c | 1 -
nvme.c | 38 ++++++++++++++++++++++++++++++++++++++
nvme.h | 11 +++++++++++
3 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/fabrics.c b/fabrics.c
index d92c2ff..f3afa0b 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -74,7 +74,6 @@ static struct config {
#define PATH_NVMF_DISC "/etc/nvme/discovery.conf"
#define PATH_NVMF_HOSTNQN "/etc/nvme/hostnqn"
#define PATH_NVMF_HOSTID "/etc/nvme/hostid"
-#define SYS_NVME "/sys/class/nvme"
#define MAX_DISC_ARGS 10
#define MAX_DISC_RETRIES 10
diff --git a/nvme.c b/nvme.c
index 66ba2fc..7f706c8 100644
--- a/nvme.c
+++ b/nvme.c
@@ -2010,6 +2010,44 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
return nvme_status_to_errno(ret, false);
}
+/*
+ * Given a controller name, create a ctrl_list_item with its
+ * attributes and compare the attributes against the connect args
+ * given.
+ * Return true/false based on whether it matches
+ */
+bool ctrl_matches_connectargs(char *name, struct connect_args *args)
+{
+ struct ctrl_list_item *ctrl;
+ bool found = false;
+
+ ctrl = malloc(sizeof(*ctrl));
+ if (!ctrl) {
+ fprintf(stderr, "Failed to allocate controller list element\n");
+ return false;
+ }
+ memset(ctrl, 0, sizeof(*ctrl));
+
+ if (get_nvme_ctrl_info(name, SYS_NVME, ctrl, NVME_NSID_ALL))
+ goto cleanup_exit;
+
+ if (!strcmp(ctrl->subsysnqn, args->subsysnqn) &&
+ !strcmp(ctrl->transport, args->transport) &&
+ (!strcmp(ctrl->traddr, args->traddr) ||
+ !strcmp(args->traddr, "none")) &&
+ (!strcmp(ctrl->trsvcid, args->trsvcid) ||
+ !strcmp(args->trsvcid, "none")) &&
+ (!strcmp(ctrl->host_traddr, args->host_traddr) ||
+ !strcmp(args->host_traddr, "none")))
+ found = true;
+
+cleanup_exit:
+ free_ctrl_list_item(ctrl);
+ free(ctrl);
+
+ return found;
+}
+
int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin, void (*vs)(__u8 *vs, struct json_object *root))
{
const char *desc = "Send an Identify Controller command to "\
diff --git a/nvme.h b/nvme.h
index 470f702..2fecb68 100644
--- a/nvme.h
+++ b/nvme.h
@@ -179,6 +179,17 @@ enum {
BINARY,
};
+struct connect_args {
+ char *subsysnqn;
+ char *transport;
+ char *traddr;
+ char *trsvcid;
+ char *host_traddr;
+};
+
+#define SYS_NVME "/sys/class/nvme"
+
+bool ctrl_matches_connectargs(char *name, struct connect_args *args);
char *__parse_connect_arg(char *conargs, const char delim, const char *fieldnm);
extern const char *conarg_traddr;
--
2.13.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 07/10] nvme-cli: Add routine to search for controller with specific attributes
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
` (5 preceding siblings ...)
2019-07-19 22:53 ` [PATCH v2 06/10] nvme-cli: Add routine to compare ctrl_list_item to connect args James Smart
@ 2019-07-19 22:53 ` James Smart
2019-07-20 8:25 ` Minwoo Im
2019-07-19 22:53 ` [PATCH v2 08/10] nvme-cli: Expand --device argument processing James Smart
` (3 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: James Smart @ 2019-07-19 22:53 UTC (permalink / raw)
In preparation for searching controllers to match with connect args:
Create a new routine find_ctrl_with_connectargs() that will search the
controllers that exist in the system to find one that has attributes
that match the connect arguments specified. If found, the routine
returns the controller name ("nvme?"). If not found, a NULL is returned.
Routine is defined as a global as a subsequent patch will use it
from the fabrics routines.
Signed-off-by: James Smart <jsmart2021 at gmail.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
Reviewed-by: Hannes Reinecke <hare at suse.com>
---
v2:
change "ctlr" to "ctrl" in fprintf
---
nvme.c | 37 +++++++++++++++++++++++++++++++++++++
nvme.h | 1 +
2 files changed, 38 insertions(+)
diff --git a/nvme.c b/nvme.c
index 7f706c8..e4fdb4e 100644
--- a/nvme.c
+++ b/nvme.c
@@ -2048,6 +2048,43 @@ cleanup_exit:
return found;
}
+/*
+ * Look through the system to find an existing controller whose
+ * attributes match the connect arguments specified
+ * If found, a string containing the controller name (ex: "nvme?")
+ * is returned.
+ * If not found, a NULL is returned.
+ */
+char *find_ctrl_with_connectargs(struct connect_args *args)
+{
+ struct dirent **devices;
+ char *devname = NULL;
+ int i, n;
+
+ n = scandir(SYS_NVME, &devices, scan_ctrls_filter, alphasort);
+ if (n < 0) {
+ fprintf(stderr, "no NVMe controller(s) detected.\n");
+ return NULL;
+ }
+
+ for (i = 0; i < n; i++) {
+ if (ctrl_matches_connectargs(devices[i]->d_name, args)) {
+ devname = strdup(devices[i]->d_name);
+ if (devname == NULL)
+ fprintf(stderr, "no memory for ctrl name %s\n",
+ devices[i]->d_name);
+ goto cleanup_devices;
+ }
+ }
+
+cleanup_devices:
+ for (i = 0; i < n; i++)
+ free(devices[i]);
+ free(devices);
+
+ return devname;
+}
+
int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin, void (*vs)(__u8 *vs, struct json_object *root))
{
const char *desc = "Send an Identify Controller command to "\
diff --git a/nvme.h b/nvme.h
index 2fecb68..b91a22c 100644
--- a/nvme.h
+++ b/nvme.h
@@ -190,6 +190,7 @@ struct connect_args {
#define SYS_NVME "/sys/class/nvme"
bool ctrl_matches_connectargs(char *name, struct connect_args *args);
+char *find_ctrl_with_connectargs(struct connect_args *args);
char *__parse_connect_arg(char *conargs, const char delim, const char *fieldnm);
extern const char *conarg_traddr;
--
2.13.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 07/10] nvme-cli: Add routine to search for controller with specific attributes
2019-07-19 22:53 ` [PATCH v2 07/10] nvme-cli: Add routine to search for controller with specific attributes James Smart
@ 2019-07-20 8:25 ` Minwoo Im
2019-07-20 18:33 ` James Smart
2019-07-22 22:41 ` Sagi Grimberg
0 siblings, 2 replies; 24+ messages in thread
From: Minwoo Im @ 2019-07-20 8:25 UTC (permalink / raw)
On 19-07-19 15:53:02, James Smart wrote:
> In preparation for searching controllers to match with connect args:
>
> Create a new routine find_ctrl_with_connectargs() that will search the
> controllers that exist in the system to find one that has attributes
> that match the connect arguments specified. If found, the routine
> returns the controller name ("nvme?"). If not found, a NULL is returned.
>
> Routine is defined as a global as a subsequent patch will use it
> from the fabrics routines.
>
> Signed-off-by: James Smart <jsmart2021 at gmail.com>
> Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
> Reviewed-by: Hannes Reinecke <hare at suse.com>
>
> ---
> v2:
> change "ctlr" to "ctrl" in fprintf
This looks good to me.
Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>
> ---
> nvme.c | 37 +++++++++++++++++++++++++++++++++++++
> nvme.h | 1 +
> 2 files changed, 38 insertions(+)
>
> diff --git a/nvme.c b/nvme.c
> index 7f706c8..e4fdb4e 100644
> --- a/nvme.c
> +++ b/nvme.c
> @@ -2048,6 +2048,43 @@ cleanup_exit:
> return found;
> }
>
> +/*
> + * Look through the system to find an existing controller whose
> + * attributes match the connect arguments specified
> + * If found, a string containing the controller name (ex: "nvme?")
> + * is returned.
> + * If not found, a NULL is returned.
> + */
> +char *find_ctrl_with_connectargs(struct connect_args *args)
> +{
> + struct dirent **devices;
> + char *devname = NULL;
> + int i, n;
> +
> + n = scandir(SYS_NVME, &devices, scan_ctrls_filter, alphasort);
> + if (n < 0) {
> + fprintf(stderr, "no NVMe controller(s) detected.\n");
> + return NULL;
> + }
> +
> + for (i = 0; i < n; i++) {
> + if (ctrl_matches_connectargs(devices[i]->d_name, args)) {
James,
It might be too late to discuss about this argument, but did you decided
to have (struct connect_args) as a parameter for this function?
Sorry for not giving any thoughts on the following mail[1], but I would
prefer not introducing a new data structure for this because it's just a
bypass argument from a perspective of find_ctrl_with_connetargs(). But
If you think it's okay to go with, then I'm fine with that too.
[1] http://lists.infradead.org/pipermail/linux-nvme/2019-July/025646.html
Thanks!
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v2 07/10] nvme-cli: Add routine to search for controller with specific attributes
2019-07-20 8:25 ` Minwoo Im
@ 2019-07-20 18:33 ` James Smart
2019-07-22 22:41 ` Sagi Grimberg
1 sibling, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-20 18:33 UTC (permalink / raw)
On 7/20/2019 1:25 AM, Minwoo Im wrote:
> James,
> It might be too late to discuss about this argument, but did you decided
> to have (struct connect_args) as a parameter for this function?
>
> Sorry for not giving any thoughts on the following mail[1], but I would
> prefer not introducing a new data structure for this because it's just a
> bypass argument from a perspective of find_ctrl_with_connetargs(). But
> If you think it's okay to go with, then I'm fine with that too.
>
> [1] http://lists.infradead.org/pipermail/linux-nvme/2019-July/025646.html
>
> Thanks!
It's never too late.? I talked briefly with Sagi before reposting. He
had a request for a future item that looked like it may be able to use
the structure.? So I left it as is.
-- james
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 07/10] nvme-cli: Add routine to search for controller with specific attributes
2019-07-20 8:25 ` Minwoo Im
2019-07-20 18:33 ` James Smart
@ 2019-07-22 22:41 ` Sagi Grimberg
2019-07-23 13:36 ` Minwoo Im
1 sibling, 1 reply; 24+ messages in thread
From: Sagi Grimberg @ 2019-07-22 22:41 UTC (permalink / raw)
> James,
>
> It might be too late to discuss about this argument, but did you decided
> to have (struct connect_args) as a parameter for this function?
>
> Sorry for not giving any thoughts on the following mail[1], but I would
> prefer not introducing a new data structure for this because it's just a
> bypass argument from a perspective of find_ctrl_with_connetargs(). But
> If you think it's okay to go with, then I'm fine with that too.
I think its just fine with the connect_args. This can also extend to
other search/match functionality we may need in the future (that is
insensitive to short/long args).
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 07/10] nvme-cli: Add routine to search for controller with specific attributes
2019-07-22 22:41 ` Sagi Grimberg
@ 2019-07-23 13:36 ` Minwoo Im
0 siblings, 0 replies; 24+ messages in thread
From: Minwoo Im @ 2019-07-23 13:36 UTC (permalink / raw)
On 19-07-22 15:41:08, Sagi Grimberg wrote:
>
> > James,
> >
> > It might be too late to discuss about this argument, but did you decided
> > to have (struct connect_args) as a parameter for this function?
> >
> > Sorry for not giving any thoughts on the following mail[1], but I would
> > prefer not introducing a new data structure for this because it's just a
> > bypass argument from a perspective of find_ctrl_with_connetargs(). But
> > If you think it's okay to go with, then I'm fine with that too.
>
> I think its just fine with the connect_args. This can also extend to
> other search/match functionality we may need in the future (that is
> insensitive to short/long args).
Agreed. I think it's good to go with what James just got sent.
Thanks for the clarification, Sagi!
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 08/10] nvme-cli: Expand --device argument processing
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
` (6 preceding siblings ...)
2019-07-19 22:53 ` [PATCH v2 07/10] nvme-cli: Add routine to search for controller with specific attributes James Smart
@ 2019-07-19 22:53 ` James Smart
2019-07-19 22:53 ` [PATCH v2 09/10] nvme-cli: add --quiet option James Smart
` (2 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-19 22:53 UTC (permalink / raw)
The connect-all --device argument was used to specify a specific device
to be used as the discovery controller. The device is typically a
long-lived discovery controller that posted a discovery event.
No attempt was made to ensure the device is who it is supposed to be
before using it.
Revised the code to use the other arguments in the connect-all request
to validate that the device is the entity that was expected. If the
device doesn't match, the cli will look for an existing matching device
in the system (should be a discovery controller due to nqn) with the
same connect parameters and use it. If one is not found on the system,
a new discovery controller will be created for the connect-all request.
The revision uses new routines to parse the connect arguments given
in the argstr parameter. As a couple of new fieldnames are needed, the
parse routine constants were expanded for them.
The revision uses the new routines to match the specified device vs
it's attributes as well as the search routine that looks for a device
with the connect arguments.
Signed-off-by: James Smart <jsmart2021 at gmail.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
Reviewed-by: Hannes Reinecke <hare at suse.com>
---
fabrics.c | 38 ++++++++++++++++++++++++++++++++++++++
nvme.c | 2 ++
nvme.h | 2 ++
3 files changed, 42 insertions(+)
diff --git a/fabrics.c b/fabrics.c
index f3afa0b..9a7f832 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -872,6 +872,8 @@ static int connect_ctrls(struct nvmf_disc_rsp_page_hdr *log, int numrec)
return ret;
}
+static const char delim_comma = ',';
+
static int do_discover(char *argstr, bool connect)
{
struct nvmf_disc_rsp_page_hdr *log = NULL;
@@ -879,6 +881,42 @@ static int do_discover(char *argstr, bool connect)
int instance, numrec = 0, ret, err;
int status = 0;
+ if (cfg.device) {
+ struct connect_args cargs;
+
+ memset(&cargs, 0, sizeof(cargs));
+ cargs.subsysnqn = __parse_connect_arg(argstr, delim_comma,
+ conarg_nqn);
+ cargs.transport = __parse_connect_arg(argstr, delim_comma,
+ conarg_transport);
+ cargs.traddr = __parse_connect_arg(argstr, delim_comma,
+ conarg_traddr);
+ cargs.trsvcid = __parse_connect_arg(argstr, delim_comma,
+ conarg_trsvcid);
+ cargs.host_traddr = __parse_connect_arg(argstr, delim_comma,
+ conarg_host_traddr);
+
+ /*
+ * if the cfg.device passed in matches the connect args
+ * cfg.device is left as-is
+ * else if there exists a controller that matches the
+ * connect args
+ * cfg.device is the matching ctrl name
+ * else if no ctrl matches the connect args
+ * cfg.device is set to null. This will attempt to
+ * create a new ctrl.
+ * endif
+ */
+ if (!ctrl_matches_connectargs(cfg.device, &cargs))
+ cfg.device = find_ctrl_with_connectargs(&cargs);
+
+ free(cargs.subsysnqn);
+ free(cargs.transport);
+ free(cargs.traddr);
+ free(cargs.trsvcid);
+ free(cargs.host_traddr);
+ }
+
if (!cfg.device)
instance = add_ctrl(argstr);
else
diff --git a/nvme.c b/nvme.c
index e4fdb4e..1a1191a 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1549,6 +1549,8 @@ static void free_ctrl_list_item(struct ctrl_list_item *ctrls)
}
static const char delim_space = ' ';
+const char *conarg_nqn = "nqn";
+const char *conarg_transport = "transport";
const char *conarg_traddr = "traddr";
const char *conarg_trsvcid = "trsvcid";
const char *conarg_host_traddr = "host_traddr";
diff --git a/nvme.h b/nvme.h
index b91a22c..e630b10 100644
--- a/nvme.h
+++ b/nvme.h
@@ -193,6 +193,8 @@ bool ctrl_matches_connectargs(char *name, struct connect_args *args);
char *find_ctrl_with_connectargs(struct connect_args *args);
char *__parse_connect_arg(char *conargs, const char delim, const char *fieldnm);
+extern const char *conarg_nqn;
+extern const char *conarg_transport;
extern const char *conarg_traddr;
extern const char *conarg_trsvcid;
extern const char *conarg_host_traddr;
--
2.13.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 09/10] nvme-cli: add --quiet option
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
` (7 preceding siblings ...)
2019-07-19 22:53 ` [PATCH v2 08/10] nvme-cli: Expand --device argument processing James Smart
@ 2019-07-19 22:53 ` James Smart
2019-07-19 22:53 ` [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts James Smart
2019-07-22 22:43 ` [PATCH v2 00/10] " Sagi Grimberg
10 siblings, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-19 22:53 UTC (permalink / raw)
From: Sagi Grimberg <sagi@grimberg.me>
Now we are going to have discovery log change events, so
instead of trying to figure out which NVM subsystem ports
are already connected, we let the driver do that and
only suppress the failure messages.
Example:
nvme connect-all ... --quiet
This option will be used by the discovery log change corresponding
udev rule.
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
Reviewed-by: James Smart <jsmart2021 at gmail.com>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>
---
fabrics.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/fabrics.c b/fabrics.c
index 9a7f832..fd5a742 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -67,6 +67,7 @@ static struct config {
int hdr_digest;
int data_digest;
bool persistent;
+ bool quiet;
} cfg = { NULL };
#define BUF_SIZE 4096
@@ -218,9 +219,11 @@ static int add_ctrl(const char *argstr)
goto out;
}
- if (write(fd, argstr, len) != len) {
- fprintf(stderr, "Failed to write to %s: %s\n",
- PATH_NVME_FABRICS, strerror(errno));
+ ret = write(fd, argstr, len);
+ if (ret != len) {
+ if (errno != EALREADY || !cfg.quiet)
+ fprintf(stderr, "Failed to write to %s: %s\n",
+ PATH_NVME_FABRICS, strerror(errno));
ret = -errno;
goto out_close;
}
@@ -854,10 +857,13 @@ static int connect_ctrls(struct nvmf_disc_rsp_page_hdr *log, int numrec)
/* already connected print message */
if (instance == -EALREADY) {
const char *traddr = log->entries[i].traddr;
- fprintf(stderr,
- "traddr=%.*s is already connected\n",
- space_strip_len(NVMF_TRADDR_SIZE, traddr),
- traddr);
+
+ if (!cfg.quiet)
+ fprintf(stderr,
+ "traddr=%.*s is already connected\n",
+ space_strip_len(NVMF_TRADDR_SIZE,
+ traddr),
+ traddr);
continue;
}
@@ -1062,6 +1068,7 @@ int discover(const char *desc, int argc, char **argv, bool connect)
{"nr-poll-queues", 'P', "LIST", CFG_INT, &cfg.nr_poll_queues, required_argument, "number of poll queues to use (default 0)" },
{"queue-size", 'Q', "LIST", CFG_INT, &cfg.queue_size, required_argument, "number of io queue elements to use (default 128)" },
{"persistent", 'p', "LIST", CFG_NONE, &cfg.persistent, no_argument, "persistent discovery connection" },
+ {"quiet", 'Q', "LIST", CFG_NONE, &cfg.quiet, no_argument, "suppress already connected errors" },
{NULL},
};
--
2.13.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
` (8 preceding siblings ...)
2019-07-19 22:53 ` [PATCH v2 09/10] nvme-cli: add --quiet option James Smart
@ 2019-07-19 22:53 ` James Smart
2019-07-23 2:52 ` Sagi Grimberg
2019-07-23 3:57 ` Sagi Grimberg
2019-07-22 22:43 ` [PATCH v2 00/10] " Sagi Grimberg
10 siblings, 2 replies; 24+ messages in thread
From: James Smart @ 2019-07-19 22:53 UTC (permalink / raw)
This set of scripts is a combination of those sent by Hannes, Sagi,
and I in earlier patches and RFC's.
Auto-connect operates by the nvme core layer or nvme-fc transport
generating a udev event with directory-controller addressing
information. The nvme core layer generates an event when a
persistent discovery controller generates a Discovery Log Change
Notification AEN. The nvme-fc transport generates an event when
an FC rport that has a NVME Discovery controller is detected or
when a FC state change event occurs for for an FC rport that has
a NVME Discovery controller
The udev event is handled by a script that extracts the Discovery
controller addressing information and initiates a systemd service
to perform a "nvme connect-all" to the Discovery controller.
The "nvme connect-all" request is not called directly from the udev
handler itself as the request may take some time or stall altogether,
which would block other udev event handling. By transitioning to
a sytemd service, the call can take as much time as needed to
complete.
The scripts consist of:
- A udev script that handles nvme core and nvme-fc udev events.
The udev handler starts a nvmf-connect systemd service.
- A nvmf-connect systemd service. The service, in its instance
name, is passed the connect arguments for the discovery
controller. The service performs a "nvme connect-all" to the
discovery controller.
- A nvmefc-boot-connections systemd service. This is a run-once
service run after udev is enabled, which will replay events
generated by NVME-FC devices detected during boot while udev
is not yet running.
- To stop autoconnect an additional nvmefc-connect.target has
been added, which will instruct systemd to cancel all
outstanding autoconnect services.
Note: Although the nvme-fc subsystem is converting to use the
same nvme core layer event mechanism, the nvme-fc-specific
udev event that has been in existence for a while is contained
in in the script so that the utilities may run against older
kernels.
Signed-off-by: James Smart <jsmart2021 at gmail.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
Reviewed-by: Hannes Reinecke <hare at suse.com>
CC: Simon Schricker <sschricker at suse.com>
---
v2:
change nvme.spec.in to load systemd mods before loading udev mods
---
Makefile | 22 +++++++++++++++++++---
nvme.spec.in | 9 +++++++++
nvmf-autoconnect/70-nvmf-autoconnect.conf | 1 +
nvmf-autoconnect/70-nvmf-autoconnect.rules | 18 ++++++++++++++++++
nvmf-autoconnect/nvmefc-boot-connections.service | 9 +++++++++
nvmf-autoconnect/nvmf-connect.target | 2 ++
nvmf-autoconnect/nvmf-connect at .service | 14 ++++++++++++++
7 files changed, 72 insertions(+), 3 deletions(-)
create mode 100644 nvmf-autoconnect/70-nvmf-autoconnect.conf
create mode 100644 nvmf-autoconnect/70-nvmf-autoconnect.rules
create mode 100644 nvmf-autoconnect/nvmefc-boot-connections.service
create mode 100644 nvmf-autoconnect/nvmf-connect.target
create mode 100644 nvmf-autoconnect/nvmf-connect at .service
diff --git a/Makefile b/Makefile
index db594a3..2ce255b 100644
--- a/Makefile
+++ b/Makefile
@@ -5,9 +5,13 @@ LIBUUID = $(shell $(LD) -o /dev/null -luuid >/dev/null 2>&1; echo $$?)
NVME = nvme
INSTALL ?= install
DESTDIR =
-PREFIX ?= /usr/local
+PREFIX ?= /usr
SYSCONFDIR = /etc
SBINDIR = $(PREFIX)/sbin
+LIBDIR ?= $(PREFIX)/lib
+SYSTEMDDIR ?= $(LIBDIR)/systemd
+UDEVDIR ?= $(LIBDIR)/udev
+DRACUTDIR ?= $(LIBDIR)/dracut
LIB_DEPENDS =
ifeq ($(LIBUUID),0)
@@ -89,6 +93,18 @@ install-bash-completion:
$(INSTALL) -d $(DESTDIR)$(PREFIX)/share/bash-completion/completions
$(INSTALL) -m 644 -T ./completions/bash-nvme-completion.sh $(DESTDIR)$(PREFIX)/share/bash-completion/completions/nvme
+install-systemd:
+ $(INSTALL) -d $(DESTDIR)$(SYSTEMDDIR)/system
+ $(INSTALL) -m 644 ./nvmf-autoconnect/*.service ./nvmf-autoconnect/*.target $(DESTDIR)$(SYSTEMDDIR)/system
+
+install-udev:
+ $(INSTALL) -d $(DESTDIR)$(UDEVDIR)/rules.d
+ $(INSTALL) -m 644 ./nvmf-autoconnect/*.rules $(DESTDIR)$(UDEVDIR)/rules.d
+
+install-dracut:
+ $(INSTALL) -d $(DESTDIR)$(DRACUTDIR)/dracut.conf.d
+ $(INSTALL) -m 644 ./nvmf-autoconnect/*.conf $(DESTDIR)$(DRACUTDIR)/dracut.conf.d
+
install-zsh-completion:
$(INSTALL) -d $(DESTDIR)$(PREFIX)/share/zsh/site-functions
$(INSTALL) -m 644 -T ./completions/_nvme $(DESTDIR)$(PREFIX)/share/zsh/site-functions/_nvme
@@ -109,7 +125,7 @@ install-etc:
$(INSTALL) -m 644 -T ./etc/discovery.conf.in $(DESTDIR)$(SYSCONFDIR)/nvme/discovery.conf; \
fi
-install-spec: install-bin install-man install-bash-completion install-zsh-completion install-etc
+install-spec: install-bin install-man install-bash-completion install-zsh-completion install-etc install-systemd install-udev install-dracut
install: install-spec install-hostparams
nvme.spec: nvme.spec.in NVME-VERSION-FILE
@@ -177,7 +193,7 @@ deb-light: $(NVME) pkg nvme.control.in
dpkg-deb --build nvme-$(NVME_VERSION)
rpm: dist
- $(RPMBUILD) -ta nvme-$(NVME_VERSION).tar.gz
+ $(RPMBUILD) --define '_libdir ${LIBDIR}' -ta nvme-$(NVME_VERSION).tar.gz
.PHONY: default doc all clean clobber install-man install-bin install
.PHONY: dist pkg dist-orig deb deb-light rpm FORCE test
diff --git a/nvme.spec.in b/nvme.spec.in
index 6934f8f..b8e3377 100644
--- a/nvme.spec.in
+++ b/nvme.spec.in
@@ -35,6 +35,11 @@ make install-spec DESTDIR=%{buildroot} PREFIX=/usr
%{_sysconfdir}/nvme/hostnqn
%{_sysconfdir}/nvme/hostid
%{_sysconfdir}/nvme/discovery.conf
+%{_libdir}/udev/rules.d/70-nvmf-autoconnect.rules
+%{_libdir}/dracut/dracut.conf.d/70-nvmf-autoconnect.conf
+%{_libdir}/systemd/system/nvmf-connect at .service
+%{_libdir}/systemd/system/nvmefc-boot-connections.service
+%{_libdir}/systemd/system/nvmf-connect.target
%clean
rm -rf $RPM_BUILD_ROOT
@@ -47,6 +52,10 @@ if [ $1 -eq 1 ]; then # 1 : This package is being installed for the first time
if [ ! -s %{_sysconfdir}/nvme/hostid ]; then
uuidgen > %{_sysconfdir}/nvme/hostid
fi
+
+ # apply udev and systemd changes that we did
+ systemctl daemon-reload
+ udevadm control --reload-rules && udevadm trigger
fi
%changelog
diff --git a/nvmf-autoconnect/70-nvmf-autoconnect.conf b/nvmf-autoconnect/70-nvmf-autoconnect.conf
new file mode 100644
index 0000000..844f3d9
--- /dev/null
+++ b/nvmf-autoconnect/70-nvmf-autoconnect.conf
@@ -0,0 +1 @@
+install_items+="/usr/lib/udev/rules.d/70-nvmf-autoconnect.rules"
diff --git a/nvmf-autoconnect/70-nvmf-autoconnect.rules b/nvmf-autoconnect/70-nvmf-autoconnect.rules
new file mode 100644
index 0000000..f901bd1
--- /dev/null
+++ b/nvmf-autoconnect/70-nvmf-autoconnect.rules
@@ -0,0 +1,18 @@
+#
+# nvmf-autoconnect.rules:
+# Handles udev events which invoke automatically scan via discovery
+# controller and connect to elements in the discovery log.
+#
+#
+
+# Events from persistent discovery controllers or nvme-fc transport events
+ACTION=="change", SUBSYSTEM=="nvme", ENV{NVME_EVENT}=="discovery",\
+ ENV{NVME_CTRL_NAME}=="*", ENV{NVME_TRTYPE}=="*", ENV{NVME_TRADDR}=="*", \
+ ENV{NVME_TRSVCID}=="*", ENV{NVME_HOST_TRADDR}=="*", \
+ RUN+="/usr/bin/systemctl --no-block start nvmf-connect at --device=$env{NVME_CTRL_NAME}\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}.service"
+
+# nvme-fc transport generated events (old-style for compatibility)
+ACTION=="change", SUBSYSTEM=="fc", ENV{FC_EVENT}=="nvmediscovery", \
+ ENV{NVMEFC_HOST_TRADDR}=="*", ENV{NVMEFC_TRADDR}=="*", \
+ RUN+="/usr/bin/systemctl --no-block start nvmf-connect at --device=none\t--transport=fc\t--traddr=$env{NVMEFC_TRADDR}\t--trsvcid=none\t--host-traddr=$env{NVMEFC_HOST_TRADDR}.service"
+
diff --git a/nvmf-autoconnect/nvmefc-boot-connections.service b/nvmf-autoconnect/nvmefc-boot-connections.service
new file mode 100644
index 0000000..84f6486
--- /dev/null
+++ b/nvmf-autoconnect/nvmefc-boot-connections.service
@@ -0,0 +1,9 @@
+[Unit]
+Description=Auto-connect to subsystems on FC-NVME devices found during boot
+
+[Service]
+Type=oneshot
+ExecStart=/bin/sh -c "echo add > /sys/class/fc/fc_udev_device/nvme_discovery"
+
+[Install]
+WantedBy=default.target
diff --git a/nvmf-autoconnect/nvmf-connect.target b/nvmf-autoconnect/nvmf-connect.target
new file mode 100644
index 0000000..f64a37c
--- /dev/null
+++ b/nvmf-autoconnect/nvmf-connect.target
@@ -0,0 +1,2 @@
+[Unit]
+Description=All instances of nvmf-autoconnect daemon
diff --git a/nvmf-autoconnect/nvmf-connect at .service b/nvmf-autoconnect/nvmf-connect at .service
new file mode 100644
index 0000000..25dca0e
--- /dev/null
+++ b/nvmf-autoconnect/nvmf-connect at .service
@@ -0,0 +1,14 @@
+#
+# Unit file used by 70-nvmf-autoconnect.rules.
+#
+
+[Unit]
+Description=NVMf auto-connect scan upon nvme discovery controller Events
+After=syslog.target
+PartOf=nvmf-connect.target
+Requires=nvmf-connect.target
+
+[Service]
+Type=simple
+Environment="CONNECT_ARGS=%i"
+ExecStart=/bin/sh -c "nvme connect-all --quiet `echo -e $CONNECT_ARGS`"
--
2.13.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts
2019-07-19 22:53 ` [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts James Smart
@ 2019-07-23 2:52 ` Sagi Grimberg
2019-07-23 16:01 ` James Smart
2019-07-23 3:57 ` Sagi Grimberg
1 sibling, 1 reply; 24+ messages in thread
From: Sagi Grimberg @ 2019-07-23 2:52 UTC (permalink / raw)
> Makefile | 22 +++++++++++++++++++---
> nvme.spec.in | 9 +++++++++
> nvmf-autoconnect/70-nvmf-autoconnect.conf | 1 +
> nvmf-autoconnect/70-nvmf-autoconnect.rules | 18 ++++++++++++++++++
> nvmf-autoconnect/nvmefc-boot-connections.service | 9 +++++++++
> nvmf-autoconnect/nvmf-connect.target | 2 ++
> nvmf-autoconnect/nvmf-connect at .service | 14 ++++++++++++++
> 7 files changed, 72 insertions(+), 3 deletions(-)
> create mode 100644 nvmf-autoconnect/70-nvmf-autoconnect.conf
> create mode 100644 nvmf-autoconnect/70-nvmf-autoconnect.rules
> create mode 100644 nvmf-autoconnect/nvmefc-boot-connections.service
> create mode 100644 nvmf-autoconnect/nvmf-connect.target
> create mode 100644 nvmf-autoconnect/nvmf-connect at .service
James, didn't we agree we are going to split these into local
directories: systemd/ rules/ dracut/ ?
I asked this for us to be able to add more scripts easily that
would not necessarily be related to autoconnect...
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts
2019-07-23 2:52 ` Sagi Grimberg
@ 2019-07-23 16:01 ` James Smart
0 siblings, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-23 16:01 UTC (permalink / raw)
On 7/22/2019 7:52 PM, Sagi Grimberg wrote:
>
>> Makefile???????????????????????????????????????? | 22
>> +++++++++++++++++++---
>> ? nvme.spec.in???????????????????????????????????? |? 9 +++++++++
>> ? nvmf-autoconnect/70-nvmf-autoconnect.conf??????? |? 1 +
>> ? nvmf-autoconnect/70-nvmf-autoconnect.rules?????? | 18
>> ++++++++++++++++++
>> ? nvmf-autoconnect/nvmefc-boot-connections.service |? 9 +++++++++
>> ? nvmf-autoconnect/nvmf-connect.target???????????? |? 2 ++
>> ? nvmf-autoconnect/nvmf-connect at .service?????????? | 14 ++++++++++++++
>> ? 7 files changed, 72 insertions(+), 3 deletions(-)
>> ? create mode 100644 nvmf-autoconnect/70-nvmf-autoconnect.conf
>> ? create mode 100644 nvmf-autoconnect/70-nvmf-autoconnect.rules
>> ? create mode 100644 nvmf-autoconnect/nvmefc-boot-connections.service
>> ? create mode 100644 nvmf-autoconnect/nvmf-connect.target
>> ? create mode 100644 nvmf-autoconnect/nvmf-connect at .service
>
> James, didn't we agree we are going to split these into local
> directories: systemd/ rules/ dracut/ ?
>
> I asked this for us to be able to add more scripts easily that
> would not necessarily be related to autoconnect...
I guess I didn't understand you.? I created separate Make install
targets for each of those.
-- james
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts
2019-07-19 22:53 ` [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts James Smart
2019-07-23 2:52 ` Sagi Grimberg
@ 2019-07-23 3:57 ` Sagi Grimberg
2019-07-23 16:04 ` James Smart
1 sibling, 1 reply; 24+ messages in thread
From: Sagi Grimberg @ 2019-07-23 3:57 UTC (permalink / raw)
> +# Events from persistent discovery controllers or nvme-fc transport events
> +ACTION=="change", SUBSYSTEM=="nvme", ENV{NVME_EVENT}=="discovery",\
> + ENV{NVME_CTRL_NAME}=="*", ENV{NVME_TRTYPE}=="*", ENV{NVME_TRADDR}=="*", \
> + ENV{NVME_TRSVCID}=="*", ENV{NVME_HOST_TRADDR}=="*", \
> + RUN+="/usr/bin/systemctl --no-block start nvmf-connect at --device=$env{NVME_CTRL_NAME}\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}.service"
James, shouldn't this be /bin/systemctl?
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts
2019-07-23 3:57 ` Sagi Grimberg
@ 2019-07-23 16:04 ` James Smart
2019-07-23 17:32 ` Sagi Grimberg
0 siblings, 1 reply; 24+ messages in thread
From: James Smart @ 2019-07-23 16:04 UTC (permalink / raw)
On 7/22/2019 8:57 PM, Sagi Grimberg wrote:
>
>> +# Events from persistent discovery controllers or nvme-fc transport
>> events
>> +ACTION=="change", SUBSYSTEM=="nvme", ENV{NVME_EVENT}=="discovery",\
>> +? ENV{NVME_CTRL_NAME}=="*", ENV{NVME_TRTYPE}=="*",
>> ENV{NVME_TRADDR}=="*", \
>> +? ENV{NVME_TRSVCID}=="*", ENV{NVME_HOST_TRADDR}=="*", \
>> +? RUN+="/usr/bin/systemctl --no-block start
>> nvmf-connect at --device=$env{NVME_CTRL_NAME}\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}.service"
>
> James, shouldn't this be /bin/systemctl?
I don't believe so.?? On the distros I checked, /bin/systemctl is a soft
or hard link to /usr/bin/systemctl
-- james
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts
2019-07-23 16:04 ` James Smart
@ 2019-07-23 17:32 ` Sagi Grimberg
2019-07-24 16:38 ` James Smart
0 siblings, 1 reply; 24+ messages in thread
From: Sagi Grimberg @ 2019-07-23 17:32 UTC (permalink / raw)
>>> +# Events from persistent discovery controllers or nvme-fc transport
>>> events
>>> +ACTION=="change", SUBSYSTEM=="nvme", ENV{NVME_EVENT}=="discovery",\
>>> +? ENV{NVME_CTRL_NAME}=="*", ENV{NVME_TRTYPE}=="*",
>>> ENV{NVME_TRADDR}=="*", \
>>> +? ENV{NVME_TRSVCID}=="*", ENV{NVME_HOST_TRADDR}=="*", \
>>> +? RUN+="/usr/bin/systemctl --no-block start
>>> nvmf-connect at --device=$env{NVME_CTRL_NAME}\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}.service"
>>>
>>
>> James, shouldn't this be /bin/systemctl?
>
> I don't believe so.?? On the distros I checked, /bin/systemctl is a soft
> or hard link to /usr/bin/systemctl
Not always the case. Lets change that to /bin/systemctl.
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts
2019-07-23 17:32 ` Sagi Grimberg
@ 2019-07-24 16:38 ` James Smart
2019-07-24 18:15 ` Sagi Grimberg
0 siblings, 1 reply; 24+ messages in thread
From: James Smart @ 2019-07-24 16:38 UTC (permalink / raw)
On 7/23/2019 10:32 AM, Sagi Grimberg wrote:
>
>>>> +# Events from persistent discovery controllers or nvme-fc
>>>> transport events
>>>> +ACTION=="change", SUBSYSTEM=="nvme", ENV{NVME_EVENT}=="discovery",\
>>>> +? ENV{NVME_CTRL_NAME}=="*", ENV{NVME_TRTYPE}=="*",
>>>> ENV{NVME_TRADDR}=="*", \
>>>> +? ENV{NVME_TRSVCID}=="*", ENV{NVME_HOST_TRADDR}=="*", \
>>>> +? RUN+="/usr/bin/systemctl --no-block start
>>>> nvmf-connect at --device=$env{NVME_CTRL_NAME}\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}.service"
>>>>
>>>
>>> James, shouldn't this be /bin/systemctl?
>>
>> I don't believe so.?? On the distros I checked, /bin/systemctl is a
>> soft or hard link to /usr/bin/systemctl
>
> Not always the case. Lets change that to /bin/systemctl.
I have no problem changing it, but am wondering what resource is giving
you the reference locations. I can't find anything that actually
specifies the location with some having /bin and others /usr/bin. The
other files seem to lean toward a preference of /usr/bin.
-- james
references that didn't help:
http://www.tldp.org/LDP/Linux-Filesystem-Hierarchy/html
https://www.freedesktop.org/wiki/Software/systemd/
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts
2019-07-24 16:38 ` James Smart
@ 2019-07-24 18:15 ` Sagi Grimberg
2019-07-24 23:45 ` James Smart
0 siblings, 1 reply; 24+ messages in thread
From: Sagi Grimberg @ 2019-07-24 18:15 UTC (permalink / raw)
>>>>> +# Events from persistent discovery controllers or nvme-fc
>>>>> transport events
>>>>> +ACTION=="change", SUBSYSTEM=="nvme", ENV{NVME_EVENT}=="discovery",\
>>>>> +? ENV{NVME_CTRL_NAME}=="*", ENV{NVME_TRTYPE}=="*",
>>>>> ENV{NVME_TRADDR}=="*", \
>>>>> +? ENV{NVME_TRSVCID}=="*", ENV{NVME_HOST_TRADDR}=="*", \
>>>>> +? RUN+="/usr/bin/systemctl --no-block start
>>>>> nvmf-connect at --device=$env{NVME_CTRL_NAME}\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}.service"
>>>>>
>>>>
>>>> James, shouldn't this be /bin/systemctl?
>>>
>>> I don't believe so.?? On the distros I checked, /bin/systemctl is a
>>> soft or hard link to /usr/bin/systemctl
>>
>> Not always the case. Lets change that to /bin/systemctl.
>
> I have no problem changing it, but am wondering what resource is giving
> you the reference locations. I can't find anything that actually
> specifies the location with some having /bin and others /usr/bin. The
> other files seem to lean toward a preference of /usr/bin.
I'm running ubuntu and systemctl is under /bin, not a softlink.
I think we should be safe on relying that systemctl is under /bin..
btw, remind me, must we call with absolute path in the rules?
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts
2019-07-24 18:15 ` Sagi Grimberg
@ 2019-07-24 23:45 ` James Smart
0 siblings, 0 replies; 24+ messages in thread
From: James Smart @ 2019-07-24 23:45 UTC (permalink / raw)
On 7/24/2019 11:15 AM, Sagi Grimberg wrote:
>
>>>>>> +# Events from persistent discovery controllers or nvme-fc
>>>>>> transport events
>>>>>> +ACTION=="change", SUBSYSTEM=="nvme", ENV{NVME_EVENT}=="discovery",\
>>>>>> +? ENV{NVME_CTRL_NAME}=="*", ENV{NVME_TRTYPE}=="*",
>>>>>> ENV{NVME_TRADDR}=="*", \
>>>>>> +? ENV{NVME_TRSVCID}=="*", ENV{NVME_HOST_TRADDR}=="*", \
>>>>>> +? RUN+="/usr/bin/systemctl --no-block start
>>>>>> nvmf-connect at --device=$env{NVME_CTRL_NAME}\t--transport=$env{NVME_TRTYPE}\t--traddr=$env{NVME_TRADDR}\t--trsvcid=$env{NVME_TRSVCID}\t--host-traddr=$env{NVME_HOST_TRADDR}.service"
>>>>>>
>>>>>
>>>>> James, shouldn't this be /bin/systemctl?
>>>>
>>>> I don't believe so.?? On the distros I checked, /bin/systemctl is a
>>>> soft or hard link to /usr/bin/systemctl
>>>
>>> Not always the case. Lets change that to /bin/systemctl.
>>
>> I have no problem changing it, but am wondering what resource is
>> giving you the reference locations. I can't find anything that
>> actually specifies the location with some having /bin and others
>> /usr/bin. The other files seem to lean toward a preference of /usr/bin.
>
> I'm running ubuntu and systemctl is under /bin, not a softlink.
>
> I think we should be safe on relying that systemctl is under /bin..
>
> btw, remind me, must we call with absolute path in the rules?
ok.
I really don't know. I assume, from a sysadm perspective, full name is
better than environment-based. But, I paired most of the other scripts
back to no path so we can probably do so here.
I'll do a repost. let me know if there's anything else besides this item
that you want to change.
-- james
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts
2019-07-19 22:52 [PATCH v2 00/10] nvme-cli: nvmf auto-connect scripts James Smart
` (9 preceding siblings ...)
2019-07-19 22:53 ` [PATCH v2 10/10] nvme-cli: nvmf auto-connect scripts James Smart
@ 2019-07-22 22:43 ` Sagi Grimberg
10 siblings, 0 replies; 24+ messages in thread
From: Sagi Grimberg @ 2019-07-22 22:43 UTC (permalink / raw)
> This is a formal posting of the patches, not an RFC.
I think we are good to go here.
Keith,
We will probably pick up the kernel component as soon as
we create nvme-5.4 tree. I think its safe to get these in
as is to nvme-cli. It's transparent to the existing usage
and will not be triggered without the kernel firing uevents.
^ permalink raw reply [flat|nested] 24+ messages in thread