[PATCH nvme-cli 0/4] Some useful fabrics patches

[PATCH nvme-cli 0/4] Some useful fabrics patches

[Sagi Grimberg]

patch #1 is cleanup
patch #2 is a bug fix
patch #3,#4 are are adding a useful fabrics command to disconnect
all the existing controllers.

Sagi Grimberg (4):
  fabrics: make some arguments integers
  fabrics: don't fail empty discovery log page
  nvme: commonize subsystems info in a helper
  fabrics: add disconnect-all command

 Documentation/nvme-disconnect-all.txt | 34 ++++++++++
 fabrics.c                             | 92 ++++++++++++++++++++-------
 fabrics.h                             |  1 +
 nvme-builtin.h                        |  1 +
 nvme.c                                | 83 ++++++++++++++----------
 nvme.h                                |  2 +
 6 files changed, 155 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/nvme-disconnect-all.txt

-- 
2.17.1

[PATCH nvme-cli 1/4] fabrics: make some arguments integers

[Sagi Grimberg]

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 fabrics.c | 60 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/fabrics.c b/fabrics.c
index af4d7463d891..28a3e3d9ac82 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -54,11 +54,11 @@ static struct config {
 	char *host_traddr;
 	char *hostnqn;
 	char *hostid;
-	char *nr_io_queues;
-	char *queue_size;
-	char *keep_alive_tmo;
-	char *reconnect_delay;
-	char *ctrl_loss_tmo;
+	int  nr_io_queues;
+	int  queue_size;
+	int  keep_alive_tmo;
+	int  reconnect_delay;
+	int  ctrl_loss_tmo;
 	char *raw;
 	char *device;
 	int  duplicate_connect;
@@ -505,6 +505,22 @@ add_bool_argument(char **argstr, int *max_len, char *arg_str, bool arg)
 	return 0;
 }
 
+static int
+add_int_argument(char **argstr, int *max_len, char *arg_str, int arg)
+{
+	int len;
+
+	if (arg) {
+		len = snprintf(*argstr, *max_len, ",%s=%d", arg_str, arg);
+		if (len < 0)
+			return -EINVAL;
+		*argstr += len;
+		*max_len -= len;
+	}
+
+	return 0;
+}
+
 static int
 add_argument(char **argstr, int *max_len, char *arg_str, char *arg)
 {
@@ -552,14 +568,14 @@ static int build_options(char *argstr, int max_len)
 		    add_argument(&argstr, &max_len, "hostnqn", cfg.hostnqn)) ||
 	    ((cfg.hostid || nvmf_hostid_file()) &&
 		    add_argument(&argstr, &max_len, "hostid", cfg.hostid)) ||
-	    add_argument(&argstr, &max_len, "nr_io_queues",
+	    add_int_argument(&argstr, &max_len, "nr_io_queues",
 				cfg.nr_io_queues) ||
-	    add_argument(&argstr, &max_len, "queue_size", cfg.queue_size) ||
-	    add_argument(&argstr, &max_len, "keep_alive_tmo",
+	    add_int_argument(&argstr, &max_len, "queue_size", cfg.queue_size) ||
+	    add_int_argument(&argstr, &max_len, "keep_alive_tmo",
 				cfg.keep_alive_tmo) ||
-	    add_argument(&argstr, &max_len, "reconnect_delay",
+	    add_int_argument(&argstr, &max_len, "reconnect_delay",
 				cfg.reconnect_delay) ||
-	    add_argument(&argstr, &max_len, "ctrl_loss_tmo",
+	    add_int_argument(&argstr, &max_len, "ctrl_loss_tmo",
 				cfg.ctrl_loss_tmo) ||
 	    add_bool_argument(&argstr, &max_len, "duplicate_connect",
 				cfg.duplicate_connect))
@@ -605,14 +621,14 @@ static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e)
 	}
 
 	if (cfg.queue_size) {
-		len = sprintf(p, ",queue_size=%s", cfg.queue_size);
+		len = sprintf(p, ",queue_size=%d", cfg.queue_size);
 		if (len < 0)
 			return -EINVAL;
 		p += len;
 	}
 
 	if (cfg.nr_io_queues) {
-		len = sprintf(p, ",nr_io_queues=%s", cfg.nr_io_queues);
+		len = sprintf(p, ",nr_io_queues=%d", cfg.nr_io_queues);
 		if (len < 0)
 			return -EINVAL;
 		p += len;
@@ -626,14 +642,14 @@ static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e)
 	}
 
 	if (cfg.ctrl_loss_tmo) {
-		len = sprintf(p, ",ctrl_loss_tmo=%s", cfg.ctrl_loss_tmo);
+		len = sprintf(p, ",ctrl_loss_tmo=%d", cfg.ctrl_loss_tmo);
 		if (len < 0)
 			return -EINVAL;
 		p += len;
 	}
 
 	if (cfg.keep_alive_tmo && !discover) {
-		len = sprintf(p, ",keep_alive_tmo=%s", cfg.keep_alive_tmo);
+		len = sprintf(p, ",keep_alive_tmo=%d", cfg.keep_alive_tmo);
 		if (len < 0)
 			return -EINVAL;
 		p += len;
@@ -835,9 +851,9 @@ 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" },
-		{"keep-alive-tmo",  'k', "LIST", CFG_STRING, &cfg.keep_alive_tmo,  required_argument, "keep alive timeout period in seconds" },
-		{"reconnect-delay", 'c', "LIST", CFG_STRING, &cfg.reconnect_delay, required_argument, "reconnect timeout period in seconds" },
-		{"ctrl-loss-tmo",   'l', "LIST", CFG_STRING, &cfg.ctrl_loss_tmo,   required_argument, "controller loss timeout period in seconds" },
+		{"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" },
 		{NULL},
 	};
 
@@ -872,11 +888,11 @@ int connect(const char *desc, int argc, char **argv)
 		{"host-traddr",     'w', "LIST", CFG_STRING, &cfg.host_traddr,     required_argument, "host traddr (e.g. FC WWN's)" },
 		{"hostnqn",         'q', "LIST", CFG_STRING, &cfg.hostnqn,         required_argument, "user-defined hostnqn" },
 		{"hostid",          'I', "LIST", CFG_STRING, &cfg.hostid,      required_argument, "user-defined hostid (if default not used)"},
-		{"nr-io-queues",    'i', "LIST", CFG_STRING, &cfg.nr_io_queues,    required_argument, "number of io queues to use (default is core count)" },
-		{"queue-size",      'Q', "LIST", CFG_STRING, &cfg.queue_size,      required_argument, "number of io queue elements to use (default 128)" },
-		{"keep-alive-tmo",  'k', "LIST", CFG_STRING, &cfg.keep_alive_tmo,  required_argument, "keep alive timeout period in seconds" },
-		{"reconnect-delay", 'c', "LIST", CFG_STRING, &cfg.reconnect_delay, required_argument, "reconnect timeout period in seconds" },
-		{"ctrl-loss-tmo",   'l', "LIST", CFG_STRING, &cfg.ctrl_loss_tmo,   required_argument, "controller loss timeout period in seconds" },
+		{"nr-io-queues",    'i', "LIST", CFG_INT, &cfg.nr_io_queues,    required_argument, "number of io queues to use (default is core count)" },
+		{"queue-size",      'Q', "LIST", CFG_INT, &cfg.queue_size,      required_argument, "number of io queue elements to use (default 128)" },
+		{"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" },
 		{"duplicate_connect", 'D', "", CFG_NONE, &cfg.duplicate_connect, no_argument, "allow duplicate connections between same transport host and subsystem port" },
 		{NULL},
 	};
-- 
2.17.1

[PATCH nvme-cli 2/4] fabrics: don't fail empty discovery log page

[Sagi Grimberg]

It can be possible that discovery subsystem will not return
any valid discovery records.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 fabrics.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fabrics.c b/fabrics.c
index 28a3e3d9ac82..09cb97c5b880 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -762,7 +762,8 @@ static int do_discover(char *argstr, bool connect)
 		fprintf(stderr, "Get discovery log entries failed.\n");
 		break;
 	case DISC_NO_LOG:
-		fprintf(stderr, "No discovery log entries to fetch.\n");
+		fprintf(stdout, "No discovery log entries to fetch.\n");
+		ret = DISC_OK;
 		break;
 	case DISC_NOT_EQUAL:
 		fprintf(stderr,
-- 
2.17.1

[PATCH nvme-cli 3/4] nvme: commonize subsystems info in a helper

[Sagi Grimberg]

We will want that to reuse for other ops that
will require to scan subsystems or controllers

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 nvme.c | 77 ++++++++++++++++++++++++++++++++--------------------------
 nvme.h |  2 ++
 2 files changed, 44 insertions(+), 35 deletions(-)

diff --git a/nvme.c b/nvme.c
index d7be2e04c9f4..80422516e6ad 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1498,7 +1498,7 @@ static void free_subsys_list_item(struct subsys_list_item *item)
 	free(item->name);
 }
 
-static void free_subsys_list(struct subsys_list_item *slist, int n)
+void free_subsys_list(struct subsys_list_item *slist, int n)
 {
 	int i;
 
@@ -1508,13 +1508,50 @@ static void free_subsys_list(struct subsys_list_item *slist, int n)
 	free(slist);
 }
 
-static int list_subsys(int argc, char **argv, struct command *cmd,
-		       struct plugin *plugin)
+struct subsys_list_item *get_subsys_list(int *subcnt)
 {
 	char path[310];
 	struct dirent **subsys;
 	struct subsys_list_item *slist;
-	int fmt, n, i, ret = 0, subcnt = 0;
+	int n, i, ret = 0;
+
+	n = scandir(subsys_dir, &subsys, scan_subsys_filter, alphasort);
+	if (n < 0) {
+		fprintf(stderr, "no NVMe subsystem(s) detected.\n");
+		return NULL;
+	}
+
+	slist = calloc(n, sizeof(struct subsys_list_item));
+	if (!slist)
+		goto free_subsys;
+
+	for (i = 0; i < n; i++) {
+		snprintf(path, sizeof(path), "%s%s", subsys_dir,
+			subsys[i]->d_name);
+		ret = get_nvme_subsystem_info(subsys[i]->d_name, path,
+				&slist[*subcnt]);
+		if (ret) {
+			fprintf(stderr,
+				"%s: failed to get subsystem info: %s\n",
+				path, strerror(errno));
+			free_subsys_list_item(&slist[*subcnt]);
+		} else
+			(*subcnt)++;
+	}
+
+free_subsys:
+	for (i = 0; i < n; i++)
+		free(subsys[i]);
+	free(subsys);
+
+	return slist;
+}
+
+static int list_subsys(int argc, char **argv, struct command *cmd,
+		struct plugin *plugin)
+{
+	struct subsys_list_item *slist;
+	int fmt, ret, subcnt = 0;
 	const char *desc = "Retrieve information for subsystems";
 	struct config {
 		char *output_format;
@@ -1535,47 +1572,17 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 		return ret;
 
 	fmt = validate_output_format(cfg.output_format);
-
 	if (fmt != JSON && fmt != NORMAL)
 		return -EINVAL;
-	n = scandir(subsys_dir, &subsys, scan_subsys_filter, alphasort);
-	if (n < 0) {
-		fprintf(stderr, "no NVMe subsystem(s) detected.\n");
-		return n;
-	}
 
-	slist = calloc(n, sizeof(struct subsys_list_item));
-	if (!slist) {
-		ret = ENOMEM;
-		goto free_subsys;
-	}
-
-	for (i = 0; i < n; i++) {
-		snprintf(path, sizeof(path), "%s%s", subsys_dir,
-			subsys[i]->d_name);
-		ret = get_nvme_subsystem_info(subsys[i]->d_name, path,
-				&slist[subcnt]);
-		if (ret) {
-			fprintf(stderr,
-				"%s: failed to get subsystem info: %s\n",
-				path, strerror(errno));
-			free_subsys_list_item(&slist[subcnt]);
-		} else
-			subcnt++;
-	}
+	slist = get_subsys_list(&subcnt);
 
 	if (fmt == JSON)
 		json_print_nvme_subsystem_list(slist, subcnt);
 	else
 		show_nvme_subsystem_list(slist, subcnt);
 
-free_subsys:
 	free_subsys_list(slist, subcnt);
-
-	for (i = 0; i < n; i++)
-		free(subsys[i]);
-	free(subsys);
-
 	return ret;
 }
 
diff --git a/nvme.h b/nvme.h
index 5098b0ef9c7c..e509e9c64f05 100644
--- a/nvme.h
+++ b/nvme.h
@@ -155,4 +155,6 @@ extern const char *devicename;
 int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin, void (*vs)(__u8 *vs, struct json_object *root));
 int	validate_output_format(char *format);
 
+struct subsys_list_item *get_subsys_list(int *subcnt);
+void free_subsys_list(struct subsys_list_item *slist, int n);
 #endif /* _NVME_H */
-- 
2.17.1

[PATCH nvme-cli 4/4] fabrics: add disconnect-all command

[Sagi Grimberg]

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 Documentation/nvme-disconnect-all.txt | 34 +++++++++++++++++++++++++++
 fabrics.c                             | 29 +++++++++++++++++++++++
 fabrics.h                             |  1 +
 nvme-builtin.h                        |  1 +
 nvme.c                                |  6 +++++
 5 files changed, 71 insertions(+)
 create mode 100644 Documentation/nvme-disconnect-all.txt

diff --git a/Documentation/nvme-disconnect-all.txt b/Documentation/nvme-disconnect-all.txt
new file mode 100644
index 000000000000..6be7e414dea7
--- /dev/null
+++ b/Documentation/nvme-disconnect-all.txt
@@ -0,0 +1,34 @@
+nvme-disconnect-all(1)
+======================
+
+NAME
+----
+nvme-disconnect-all - Disconnect from all connected Fabrics controllers.
+
+SYNOPSIS
+--------
+[verse]
+'nvme disconnect-all'
+
+DESCRIPTION
+-----------
+Disconnects and removes all existing NVMe over Fabrics controllers.
+
+See the documentation for the nvme-disconnect(1) command for further
+background.
+
+EXAMPLES
+--------
+* Disconnect all existing nvme controllers:
++
+------------
+# nvme disconnect-all
+------------
+
+SEE ALSO
+--------
+nvme-disconnect(1)
+
+NVME
+----
+Part of the nvme-user suite
diff --git a/fabrics.c b/fabrics.c
index 09cb97c5b880..029105b227ff 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -1048,3 +1048,32 @@ int disconnect(const char *desc, int argc, char **argv)
 
 	return ret;
 }
+
+int disconnect_all(const char *desc, int argc, char **argv)
+{
+	struct subsys_list_item *slist;
+	int i, j, ret = 0, subcnt = 0;
+	const struct argconfig_commandline_options command_line_options[] = {
+		{NULL},
+	};
+
+	ret = argconfig_parse(argc, argv, desc, command_line_options, &cfg,
+			sizeof(cfg));
+	if (ret)
+		return ret;
+
+	slist = get_subsys_list(&subcnt);
+	for (i = 0; i < subcnt; i++) {
+		struct subsys_list_item *subsys = &slist[i];
+
+		for (j = 0; j < subsys->nctrls; j++) {
+			struct ctrl_list_item *ctrl = &subsys->ctrls[j];
+
+			ret = disconnect_by_device(ctrl->name);
+			if (ret)
+				goto out;
+		}
+	}
+out:
+	return ret;
+}
diff --git a/fabrics.h b/fabrics.h
index 91aec90f0ab0..988f3ef2fbc4 100644
--- a/fabrics.h
+++ b/fabrics.h
@@ -4,5 +4,6 @@
 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);
+extern int disconnect_all(const char *desc, int argc, char **argv);
 
 #endif
diff --git a/nvme-builtin.h b/nvme-builtin.h
index f9e47b33803c..afd12b055910 100644
--- a/nvme-builtin.h
+++ b/nvme-builtin.h
@@ -63,6 +63,7 @@ COMMAND_LIST(
 	ENTRY("connect-all", "Discover and Connect to NVMeoF subsystems", connect_all_cmd)
 	ENTRY("connect", "Connect to NVMeoF subsystem", connect_cmd)
 	ENTRY("disconnect", "Disconnect from NVMeoF subsystem", disconnect_cmd)
+	ENTRY("disconnect-all", "Disconnect from all connected NVMeoF subsystems", disconnect_all_cmd)
 	ENTRY("gen-hostnqn", "Generate NVMeoF host NQN", gen_hostnqn_cmd)
 	ENTRY("dir-receive", "Submit a Directive Receive command, return results", dir_receive)
 	ENTRY("dir-send", "Submit a Directive Send command, return results", dir_send)
diff --git a/nvme.c b/nvme.c
index 80422516e6ad..a17a4f9393dc 100644
--- a/nvme.c
+++ b/nvme.c
@@ -4624,6 +4624,12 @@ static int disconnect_cmd(int argc, char **argv, struct command *command, struct
 	return disconnect(desc, argc, argv);
 }
 
+static int disconnect_all_cmd(int argc, char **argv, struct command *command, struct plugin *plugin)
+{
+	const char *desc = "Disconnect from all connected NVMeoF subsystems";
+	return disconnect_all(desc, argc, argv);
+}
+
 void register_extension(struct plugin *plugin)
 {
 	plugin->parent = &nvme;
-- 
2.17.1

[PATCH nvme-cli 1/4] fabrics: make some arguments integers

[Chaitanya Kulkarni]

Looks good to me, Reviewed-by : Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>.

?On 8/31/18, 6:36 PM, "Linux-nvme on behalf of Sagi Grimberg" <linux-nvme-bounces@lists.infradead.org on behalf of sagi@grimberg.me> wrote:

    Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
    ---
     fabrics.c | 60 +++++++++++++++++++++++++++++++++++--------------------
     1 file changed, 38 insertions(+), 22 deletions(-)
    
    diff --git a/fabrics.c b/fabrics.c
    index af4d7463d891..28a3e3d9ac82 100644
    --- a/fabrics.c
    +++ b/fabrics.c
    @@ -54,11 +54,11 @@ static struct config {
     	char *host_traddr;
     	char *hostnqn;
     	char *hostid;
    -	char *nr_io_queues;
    -	char *queue_size;
    -	char *keep_alive_tmo;
    -	char *reconnect_delay;
    -	char *ctrl_loss_tmo;
    +	int  nr_io_queues;
    +	int  queue_size;
    +	int  keep_alive_tmo;
    +	int  reconnect_delay;
    +	int  ctrl_loss_tmo;
     	char *raw;
     	char *device;
     	int  duplicate_connect;
    @@ -505,6 +505,22 @@ add_bool_argument(char **argstr, int *max_len, char *arg_str, bool arg)
     	return 0;
     }
     
    +static int
    +add_int_argument(char **argstr, int *max_len, char *arg_str, int arg)
    +{
    +	int len;
    +
    +	if (arg) {
    +		len = snprintf(*argstr, *max_len, ",%s=%d", arg_str, arg);
    +		if (len < 0)
    +			return -EINVAL;
    +		*argstr += len;
    +		*max_len -= len;
    +	}
    +
    +	return 0;
    +}
    +
     static int
     add_argument(char **argstr, int *max_len, char *arg_str, char *arg)
     {
    @@ -552,14 +568,14 @@ static int build_options(char *argstr, int max_len)
     		    add_argument(&argstr, &max_len, "hostnqn", cfg.hostnqn)) ||
     	    ((cfg.hostid || nvmf_hostid_file()) &&
     		    add_argument(&argstr, &max_len, "hostid", cfg.hostid)) ||
    -	    add_argument(&argstr, &max_len, "nr_io_queues",
    +	    add_int_argument(&argstr, &max_len, "nr_io_queues",
     				cfg.nr_io_queues) ||
    -	    add_argument(&argstr, &max_len, "queue_size", cfg.queue_size) ||
    -	    add_argument(&argstr, &max_len, "keep_alive_tmo",
    +	    add_int_argument(&argstr, &max_len, "queue_size", cfg.queue_size) ||
    +	    add_int_argument(&argstr, &max_len, "keep_alive_tmo",
     				cfg.keep_alive_tmo) ||
    -	    add_argument(&argstr, &max_len, "reconnect_delay",
    +	    add_int_argument(&argstr, &max_len, "reconnect_delay",
     				cfg.reconnect_delay) ||
    -	    add_argument(&argstr, &max_len, "ctrl_loss_tmo",
    +	    add_int_argument(&argstr, &max_len, "ctrl_loss_tmo",
     				cfg.ctrl_loss_tmo) ||
     	    add_bool_argument(&argstr, &max_len, "duplicate_connect",
     				cfg.duplicate_connect))
    @@ -605,14 +621,14 @@ static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e)
     	}
     
     	if (cfg.queue_size) {
    -		len = sprintf(p, ",queue_size=%s", cfg.queue_size);
    +		len = sprintf(p, ",queue_size=%d", cfg.queue_size);
     		if (len < 0)
     			return -EINVAL;
     		p += len;
     	}
     
     	if (cfg.nr_io_queues) {
    -		len = sprintf(p, ",nr_io_queues=%s", cfg.nr_io_queues);
    +		len = sprintf(p, ",nr_io_queues=%d", cfg.nr_io_queues);
     		if (len < 0)
     			return -EINVAL;
     		p += len;
    @@ -626,14 +642,14 @@ static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e)
     	}
     
     	if (cfg.ctrl_loss_tmo) {
    -		len = sprintf(p, ",ctrl_loss_tmo=%s", cfg.ctrl_loss_tmo);
    +		len = sprintf(p, ",ctrl_loss_tmo=%d", cfg.ctrl_loss_tmo);
     		if (len < 0)
     			return -EINVAL;
     		p += len;
     	}
     
     	if (cfg.keep_alive_tmo && !discover) {
    -		len = sprintf(p, ",keep_alive_tmo=%s", cfg.keep_alive_tmo);
    +		len = sprintf(p, ",keep_alive_tmo=%d", cfg.keep_alive_tmo);
     		if (len < 0)
     			return -EINVAL;
     		p += len;
    @@ -835,9 +851,9 @@ 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" },
    -		{"keep-alive-tmo",  'k', "LIST", CFG_STRING, &cfg.keep_alive_tmo,  required_argument, "keep alive timeout period in seconds" },
    -		{"reconnect-delay", 'c', "LIST", CFG_STRING, &cfg.reconnect_delay, required_argument, "reconnect timeout period in seconds" },
    -		{"ctrl-loss-tmo",   'l', "LIST", CFG_STRING, &cfg.ctrl_loss_tmo,   required_argument, "controller loss timeout period in seconds" },
    +		{"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" },
     		{NULL},
     	};
     
    @@ -872,11 +888,11 @@ int connect(const char *desc, int argc, char **argv)
     		{"host-traddr",     'w', "LIST", CFG_STRING, &cfg.host_traddr,     required_argument, "host traddr (e.g. FC WWN's)" },
     		{"hostnqn",         'q', "LIST", CFG_STRING, &cfg.hostnqn,         required_argument, "user-defined hostnqn" },
     		{"hostid",          'I', "LIST", CFG_STRING, &cfg.hostid,      required_argument, "user-defined hostid (if default not used)"},
    -		{"nr-io-queues",    'i', "LIST", CFG_STRING, &cfg.nr_io_queues,    required_argument, "number of io queues to use (default is core count)" },
    -		{"queue-size",      'Q', "LIST", CFG_STRING, &cfg.queue_size,      required_argument, "number of io queue elements to use (default 128)" },
    -		{"keep-alive-tmo",  'k', "LIST", CFG_STRING, &cfg.keep_alive_tmo,  required_argument, "keep alive timeout period in seconds" },
    -		{"reconnect-delay", 'c', "LIST", CFG_STRING, &cfg.reconnect_delay, required_argument, "reconnect timeout period in seconds" },
    -		{"ctrl-loss-tmo",   'l', "LIST", CFG_STRING, &cfg.ctrl_loss_tmo,   required_argument, "controller loss timeout period in seconds" },
    +		{"nr-io-queues",    'i', "LIST", CFG_INT, &cfg.nr_io_queues,    required_argument, "number of io queues to use (default is core count)" },
    +		{"queue-size",      'Q', "LIST", CFG_INT, &cfg.queue_size,      required_argument, "number of io queue elements to use (default 128)" },
    +		{"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" },
     		{"duplicate_connect", 'D', "", CFG_NONE, &cfg.duplicate_connect, no_argument, "allow duplicate connections between same transport host and subsystem port" },
     		{NULL},
     	};
    -- 
    2.17.1
    
    
    _______________________________________________
    Linux-nvme mailing list
    Linux-nvme at lists.infradead.org
    http://lists.infradead.org/mailman/listinfo/linux-nvme
    

[PATCH nvme-cli 2/4] fabrics: don't fail empty discovery log page

[Chaitanya Kulkarni]

Looks good to me. (not sure if it matters but we can go up to 72 columns for commit log,
if it does can be fixed at the time of applying the patch).

Reviewed-by : Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>.
?On 8/31/18, 6:37 PM, "Linux-nvme on behalf of Sagi Grimberg" <linux-nvme-bounces@lists.infradead.org on behalf of sagi@grimberg.me> wrote:

    It can be possible that discovery subsystem will not return
    any valid discovery records.
    
    Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
    ---
     fabrics.c | 3 ++-
     1 file changed, 2 insertions(+), 1 deletion(-)
    
    diff --git a/fabrics.c b/fabrics.c
    index 28a3e3d9ac82..09cb97c5b880 100644
    --- a/fabrics.c
    +++ b/fabrics.c
    @@ -762,7 +762,8 @@ static int do_discover(char *argstr, bool connect)
     		fprintf(stderr, "Get discovery log entries failed.\n");
     		break;
     	case DISC_NO_LOG:
    -		fprintf(stderr, "No discovery log entries to fetch.\n");
    +		fprintf(stdout, "No discovery log entries to fetch.\n");
    +		ret = DISC_OK;
     		break;
     	case DISC_NOT_EQUAL:
     		fprintf(stderr,
    -- 
    2.17.1
    
    
    _______________________________________________
    Linux-nvme mailing list
    Linux-nvme at lists.infradead.org
    http://lists.infradead.org/mailman/listinfo/linux-nvme
    

[PATCH nvme-cli 3/4] nvme: commonize subsystems info in a helper

[Chaitanya Kulkarni]

Looks good to me. (not sure if it matters but we can go up to 72 columns for commit log,
if it does can be fixed at the time of applying the patch).

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

On 8/31/18, 6:37 PM, "Linux-nvme on behalf of Sagi Grimberg" <linux-nvme-bounces@lists.infradead.org on behalf of sagi@grimberg.me> wrote:

    We will want that to reuse for other ops that
    will require to scan subsystems or controllers
    
    Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
    ---
     nvme.c | 77 ++++++++++++++++++++++++++++++++--------------------------
     nvme.h |  2 ++
     2 files changed, 44 insertions(+), 35 deletions(-)
    
    diff --git a/nvme.c b/nvme.c
    index d7be2e04c9f4..80422516e6ad 100644
    --- a/nvme.c
    +++ b/nvme.c
    @@ -1498,7 +1498,7 @@ static void free_subsys_list_item(struct subsys_list_item *item)
     	free(item->name);
     }
     
    -static void free_subsys_list(struct subsys_list_item *slist, int n)
    +void free_subsys_list(struct subsys_list_item *slist, int n)
     {
     	int i;
     
    @@ -1508,13 +1508,50 @@ static void free_subsys_list(struct subsys_list_item *slist, int n)
     	free(slist);
     }
     
    -static int list_subsys(int argc, char **argv, struct command *cmd,
    -		       struct plugin *plugin)
    +struct subsys_list_item *get_subsys_list(int *subcnt)
     {
     	char path[310];
     	struct dirent **subsys;
     	struct subsys_list_item *slist;
    -	int fmt, n, i, ret = 0, subcnt = 0;
    +	int n, i, ret = 0;
    +
    +	n = scandir(subsys_dir, &subsys, scan_subsys_filter, alphasort);
    +	if (n < 0) {
    +		fprintf(stderr, "no NVMe subsystem(s) detected.\n");
    +		return NULL;
    +	}
    +
    +	slist = calloc(n, sizeof(struct subsys_list_item));
    +	if (!slist)
    +		goto free_subsys;
    +
    +	for (i = 0; i < n; i++) {
    +		snprintf(path, sizeof(path), "%s%s", subsys_dir,
    +			subsys[i]->d_name);
    +		ret = get_nvme_subsystem_info(subsys[i]->d_name, path,
    +				&slist[*subcnt]);
    +		if (ret) {
    +			fprintf(stderr,
    +				"%s: failed to get subsystem info: %s\n",
    +				path, strerror(errno));
    +			free_subsys_list_item(&slist[*subcnt]);
    +		} else
    +			(*subcnt)++;
    +	}
    +
    +free_subsys:
    +	for (i = 0; i < n; i++)
    +		free(subsys[i]);
    +	free(subsys);
    +
    +	return slist;
    +}
    +
    +static int list_subsys(int argc, char **argv, struct command *cmd,
    +		struct plugin *plugin)
    +{
    +	struct subsys_list_item *slist;
    +	int fmt, ret, subcnt = 0;
     	const char *desc = "Retrieve information for subsystems";
     	struct config {
     		char *output_format;
    @@ -1535,47 +1572,17 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
     		return ret;
     
     	fmt = validate_output_format(cfg.output_format);
    -
     	if (fmt != JSON && fmt != NORMAL)
     		return -EINVAL;
    -	n = scandir(subsys_dir, &subsys, scan_subsys_filter, alphasort);
    -	if (n < 0) {
    -		fprintf(stderr, "no NVMe subsystem(s) detected.\n");
    -		return n;
    -	}
     
    -	slist = calloc(n, sizeof(struct subsys_list_item));
    -	if (!slist) {
    -		ret = ENOMEM;
    -		goto free_subsys;
    -	}
    -
    -	for (i = 0; i < n; i++) {
    -		snprintf(path, sizeof(path), "%s%s", subsys_dir,
    -			subsys[i]->d_name);
    -		ret = get_nvme_subsystem_info(subsys[i]->d_name, path,
    -				&slist[subcnt]);
    -		if (ret) {
    -			fprintf(stderr,
    -				"%s: failed to get subsystem info: %s\n",
    -				path, strerror(errno));
    -			free_subsys_list_item(&slist[subcnt]);
    -		} else
    -			subcnt++;
    -	}
    +	slist = get_subsys_list(&subcnt);
     
     	if (fmt == JSON)
     		json_print_nvme_subsystem_list(slist, subcnt);
     	else
     		show_nvme_subsystem_list(slist, subcnt);
     
    -free_subsys:
     	free_subsys_list(slist, subcnt);
    -
    -	for (i = 0; i < n; i++)
    -		free(subsys[i]);
    -	free(subsys);
    -
     	return ret;
     }
     
    diff --git a/nvme.h b/nvme.h
    index 5098b0ef9c7c..e509e9c64f05 100644
    --- a/nvme.h
    +++ b/nvme.h
    @@ -155,4 +155,6 @@ extern const char *devicename;
     int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin, void (*vs)(__u8 *vs, struct json_object *root));
     int	validate_output_format(char *format);
     
    +struct subsys_list_item *get_subsys_list(int *subcnt);
    +void free_subsys_list(struct subsys_list_item *slist, int n);
     #endif /* _NVME_H */
    -- 
    2.17.1
    
    
    _______________________________________________
    Linux-nvme mailing list
    Linux-nvme at lists.infradead.org
    http://lists.infradead.org/mailman/listinfo/linux-nvme
    

[PATCH nvme-cli 4/4] fabrics: add disconnect-all command

[Chaitanya Kulkarni]

Just one minor comment, otherwise looks good to me.

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

On 8/31/18, 6:38 PM, "Linux-nvme on behalf of Sagi Grimberg" <linux-nvme-bounces@lists.infradead.org on behalf of sagi@grimberg.me> wrote:

    Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
    ---
     Documentation/nvme-disconnect-all.txt | 34 +++++++++++++++++++++++++++
     fabrics.c                             | 29 +++++++++++++++++++++++
     fabrics.h                             |  1 +
     nvme-builtin.h                        |  1 +
     nvme.c                                |  6 +++++
     5 files changed, 71 insertions(+)
     create mode 100644 Documentation/nvme-disconnect-all.txt
    
    diff --git a/Documentation/nvme-disconnect-all.txt b/Documentation/nvme-disconnect-all.txt
    new file mode 100644
    index 000000000000..6be7e414dea7
    --- /dev/null
    +++ b/Documentation/nvme-disconnect-all.txt
    @@ -0,0 +1,34 @@
    +nvme-disconnect-all(1)
    +======================
    +
    +NAME
    +----
    +nvme-disconnect-all - Disconnect from all connected Fabrics controllers.
    +
    +SYNOPSIS
    +--------
    +[verse]
    +'nvme disconnect-all'
    +
    +DESCRIPTION
    +-----------
    +Disconnects and removes all existing NVMe over Fabrics controllers.
    +
    +See the documentation for the nvme-disconnect(1) command for further
    +background.
    +
    +EXAMPLES
    +--------
    +* Disconnect all existing nvme controllers:
[CK] Even though it's implied may be add "NVMe over Fabrics" in the above line ?
    ++
    +------------
    +# nvme disconnect-all
    +------------
    +
    +SEE ALSO
    +--------
    +nvme-disconnect(1)
    +
    +NVME
    +----
    +Part of the nvme-user suite
    diff --git a/fabrics.c b/fabrics.c
    index 09cb97c5b880..029105b227ff 100644
    --- a/fabrics.c
    +++ b/fabrics.c
    @@ -1048,3 +1048,32 @@ int disconnect(const char *desc, int argc, char **argv)
     
     	return ret;
     }
    +
    +int disconnect_all(const char *desc, int argc, char **argv)
    +{
    +	struct subsys_list_item *slist;
    +	int i, j, ret = 0, subcnt = 0;
    +	const struct argconfig_commandline_options command_line_options[] = {
    +		{NULL},
    +	};
    +
    +	ret = argconfig_parse(argc, argv, desc, command_line_options, &cfg,
    +			sizeof(cfg));
    +	if (ret)
    +		return ret;
    +
    +	slist = get_subsys_list(&subcnt);
    +	for (i = 0; i < subcnt; i++) {
    +		struct subsys_list_item *subsys = &slist[i];
    +
    +		for (j = 0; j < subsys->nctrls; j++) {
    +			struct ctrl_list_item *ctrl = &subsys->ctrls[j];
    +
    +			ret = disconnect_by_device(ctrl->name);
    +			if (ret)
    +				goto out;
    +		}
    +	}
    +out:
    +	return ret;
    +}
    diff --git a/fabrics.h b/fabrics.h
    index 91aec90f0ab0..988f3ef2fbc4 100644
    --- a/fabrics.h
    +++ b/fabrics.h
    @@ -4,5 +4,6 @@
     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);
    +extern int disconnect_all(const char *desc, int argc, char **argv);
     
     #endif
    diff --git a/nvme-builtin.h b/nvme-builtin.h
    index f9e47b33803c..afd12b055910 100644
    --- a/nvme-builtin.h
    +++ b/nvme-builtin.h
    @@ -63,6 +63,7 @@ COMMAND_LIST(
     	ENTRY("connect-all", "Discover and Connect to NVMeoF subsystems", connect_all_cmd)
     	ENTRY("connect", "Connect to NVMeoF subsystem", connect_cmd)
     	ENTRY("disconnect", "Disconnect from NVMeoF subsystem", disconnect_cmd)
    +	ENTRY("disconnect-all", "Disconnect from all connected NVMeoF subsystems", disconnect_all_cmd)
     	ENTRY("gen-hostnqn", "Generate NVMeoF host NQN", gen_hostnqn_cmd)
     	ENTRY("dir-receive", "Submit a Directive Receive command, return results", dir_receive)
     	ENTRY("dir-send", "Submit a Directive Send command, return results", dir_send)
    diff --git a/nvme.c b/nvme.c
    index 80422516e6ad..a17a4f9393dc 100644
    --- a/nvme.c
    +++ b/nvme.c
    @@ -4624,6 +4624,12 @@ static int disconnect_cmd(int argc, char **argv, struct command *command, struct
     	return disconnect(desc, argc, argv);
     }
     
    +static int disconnect_all_cmd(int argc, char **argv, struct command *command, struct plugin *plugin)
    +{
    +	const char *desc = "Disconnect from all connected NVMeoF subsystems";
    +	return disconnect_all(desc, argc, argv);
    +}
    +
     void register_extension(struct plugin *plugin)
     {
     	plugin->parent = &nvme;
    -- 
    2.17.1
    
    
    _______________________________________________
    Linux-nvme mailing list
    Linux-nvme at lists.infradead.org
    http://lists.infradead.org/mailman/listinfo/linux-nvme
    

[PATCH nvme-cli 0/4] Some useful fabrics patches

[Keith Busch]

On Fri, Aug 31, 2018@06:36:01PM -0700, Sagi Grimberg wrote:
> patch #1 is cleanup
> patch #2 is a bug fix
> patch #3,#4 are are adding a useful fabrics command to disconnect
> all the existing controllers.
> 
> Sagi Grimberg (4):
>   fabrics: make some arguments integers
>   fabrics: don't fail empty discovery log page
>   nvme: commonize subsystems info in a helper
>   fabrics: add disconnect-all command
> 
>  Documentation/nvme-disconnect-all.txt | 34 ++++++++++
>  fabrics.c                             | 92 ++++++++++++++++++++-------
>  fabrics.h                             |  1 +
>  nvme-builtin.h                        |  1 +
>  nvme.c                                | 83 ++++++++++++++----------
>  nvme.h                                |  2 +
>  6 files changed, 155 insertions(+), 58 deletions(-)
>  create mode 100644 Documentation/nvme-disconnect-all.txt

Looks good, applied with Chaitanya's reviews and regenerated
documentation.