All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli)
@ 2019-05-23  9:00 Max Gurtovoy
  2019-05-23  9:00 ` [PATCH 1/9] nvme: update list-ns nsid option Max Gurtovoy
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  9:00 UTC (permalink / raw)


This patchset introduce few small bug fixes for memory leaks, improve
coding style, improve code readability and mainly focus on the "nvme list-subsys"
command. This command will show the whole NVM subsystem list (same as "nvme list"
does for namespaces).
The patchset ends with adding namespaces entry per each ctrl in the system.
Actually, at first stage, we'll print only the namespace handle (nvme0n1 for
example). Later on, we can add more attribute per demand (uuid, nguid, capacity,
etc...).

Changes from v1:
 - Added Reviewed-by sign (thanks Minwoo Im)
 - Added check for invalid nsid (proposed by Minwoo Im)
 - Fixed the output for native NVMe multipath

Max Gurtovoy (9):
  nvme: update list-ns nsid option
  nvme: update description for "nvme list" command
  fabrics: Fix memory leak of subsys list
  nvme-print: fix json object memory leak
  nvme: fix coding style issue
  nvme: update list-subsys command to show the entire list
  nvme-print: Introduce show_nvme_ctrl helper
  nvme-print: Rename "Paths" --> "Ctrls" for json output in list-subsys
  nvme: Retrieve namespaces during list-subsys cmd

 fabrics.c    |   6 +-
 nvme-print.c |  52 +++++++----
 nvme.c       | 281 ++++++++++++++++++++++++-----------------------------------
 nvme.h       |   9 +-
 4 files changed, 158 insertions(+), 190 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/9] nvme: update list-ns nsid option
  2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
@ 2019-05-23  9:00 ` Max Gurtovoy
  2019-05-23 17:27   ` Heitke, Kenneth
  2019-05-23  9:00 ` [PATCH 2/9] nvme: update description for "nvme list" command Max Gurtovoy
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  9:00 UTC (permalink / raw)


This commit updates the optional nsid argument to define the wanted
nsid for start, instead of starting from nsid + 1. E.g. in case we've
wanted to get the list of namespaces starting from 1, before this
commit, we used the "--namespace-id=0" option. Nsid 0 is not valid in
NVMe spec, thus change it to start counting from the given nsid.

Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 nvme.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/nvme.c b/nvme.c
index 9819fcb..3c1bcb1 100644
--- a/nvme.c
+++ b/nvme.c
@@ -950,9 +950,9 @@ close_fd:
 
 static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
 {
-	const char *desc = "For the specified device, show the "\
-		"namespace list in a NVMe subsystem, optionally starting with a given namespace";
-	const char *namespace_id = "namespace number returned list should to start after";
+	const char *desc = "For the specified controller handle, show the "\
+		"namespace list in the associated NVMe subsystem, optionally starting with a given nsid.";
+	const char *namespace_id = "first nsid returned list should start from";
 	const char *all = "show all namespaces in the subsystem, whether attached or inactive";
 	int err, i, fd;
 	__u32 ns_list[1024];
@@ -963,7 +963,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
 	};
 
 	struct config cfg = {
-		.namespace_id = 0,
+		.namespace_id = 1,
 	};
 
 	const struct argconfig_commandline_options command_line_options[] = {
@@ -976,7 +976,11 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
 	if (fd < 0)
 		return fd;
 
-	err = nvme_identify_ns_list(fd, cfg.namespace_id, !!cfg.all, ns_list);
+	if (!cfg.namespace_id)
+		return -EINVAL;
+
+	err = nvme_identify_ns_list(fd, cfg.namespace_id - 1, !!cfg.all,
+				    ns_list);
 	if (!err) {
 		for (i = 0; i < 1024; i++)
 			if (ns_list[i])
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/9] nvme: update description for "nvme list" command
  2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
  2019-05-23  9:00 ` [PATCH 1/9] nvme: update list-ns nsid option Max Gurtovoy
@ 2019-05-23  9:00 ` Max Gurtovoy
  2019-05-23  9:00 ` [PATCH 3/9] fabrics: Fix memory leak of subsys list Max Gurtovoy
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  9:00 UTC (permalink / raw)


The "nvme list" command doesn't get any device handle as an input
argument. This operation prints out a basic information for all NVMe
namespaces in the system.

Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nvme.c b/nvme.c
index 3c1bcb1..fd86779 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1797,7 +1797,7 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
 	struct list_item *list_items;
 	unsigned int list_cnt = 0;
 	int fmt, ret, fd, i, n;
-	const char *desc = "Retrieve basic information for the given device";
+	const char *desc = "Retrieve basic information for all NVMe namespaces";
 	struct config {
 		char *output_format;
 	};
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/9] fabrics: Fix memory leak of subsys list
  2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
  2019-05-23  9:00 ` [PATCH 1/9] nvme: update list-ns nsid option Max Gurtovoy
  2019-05-23  9:00 ` [PATCH 2/9] nvme: update description for "nvme list" command Max Gurtovoy
@ 2019-05-23  9:00 ` Max Gurtovoy
  2019-05-23  9:00 ` [PATCH 4/9] nvme-print: fix json object memory leak Max Gurtovoy
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  9:00 UTC (permalink / raw)


Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 fabrics.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fabrics.c b/fabrics.c
index 511de06..b42a3ce 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -1203,5 +1203,6 @@ int disconnect_all(const char *desc, int argc, char **argv)
 		}
 	}
 out:
+	free_subsys_list(slist, subcnt);
 	return ret;
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/9] nvme-print: fix json object memory leak
  2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
                   ` (2 preceding siblings ...)
  2019-05-23  9:00 ` [PATCH 3/9] fabrics: Fix memory leak of subsys list Max Gurtovoy
@ 2019-05-23  9:00 ` Max Gurtovoy
  2019-05-23  9:00 ` [PATCH 5/9] nvme: fix coding style issue Max Gurtovoy
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  9:00 UTC (permalink / raw)


Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 nvme-print.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nvme-print.c b/nvme-print.c
index 2c4822e..6f85e73 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2918,6 +2918,7 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
 		json_object_add_value_array(root, "Subsystems", subsystems);
 	json_print_object(root, NULL);
 	printf("\n");
+	json_free_object(root);
 }
 
 static void show_registers_cap(struct nvme_bar_cap *cap)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/9] nvme: fix coding style issue
  2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
                   ` (3 preceding siblings ...)
  2019-05-23  9:00 ` [PATCH 4/9] nvme-print: fix json object memory leak Max Gurtovoy
@ 2019-05-23  9:00 ` Max Gurtovoy
  2019-05-23  9:00 ` [PATCH 6/9] nvme: update list-subsys command to show the entire list Max Gurtovoy
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  9:00 UTC (permalink / raw)


It's more common to use the following coding style:
if (condition) {
   do_that;
   do_this;
} else if (condition2) {
   do_this;
} else {
   do_that;
}

Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 nvme.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/nvme.c b/nvme.c
index fd86779..8edf398 100644
--- a/nvme.c
+++ b/nvme.c
@@ -985,11 +985,11 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
 		for (i = 0; i < 1024; i++)
 			if (ns_list[i])
 				printf("[%4u]:%#x\n", i, le32_to_cpu(ns_list[i]));
-	}
-	else if (err > 0)
+	} else if (err > 0) {
 		show_nvme_status(err);
-	else
+	} else {
 		perror("id namespace list");
+	}
 
 	close(fd);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 6/9] nvme: update list-subsys command to show the entire list
  2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
                   ` (4 preceding siblings ...)
  2019-05-23  9:00 ` [PATCH 5/9] nvme: fix coding style issue Max Gurtovoy
@ 2019-05-23  9:00 ` Max Gurtovoy
  2019-05-23  9:00 ` [PATCH 7/9] nvme-print: Introduce show_nvme_ctrl helper Max Gurtovoy
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  9:00 UTC (permalink / raw)


Make it similar to "nvme list" command. For that, remove the
unnecessary ana_state parsing (was introduced only if specific namespace
handle was given).

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 fabrics.c    |   5 +-
 nvme-print.c |  10 +---
 nvme.c       | 149 +++++++----------------------------------------------------
 nvme.h       |   3 +-
 4 files changed, 23 insertions(+), 144 deletions(-)

diff --git a/fabrics.c b/fabrics.c
index b42a3ce..3dadb4e 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -1190,7 +1190,10 @@ int disconnect_all(const char *desc, int argc, char **argv)
 	if (ret)
 		return ret;
 
-	slist = get_subsys_list(&subcnt, NULL, NVME_NSID_ALL);
+	ret = get_subsys_list(&subcnt, &slist);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < subcnt; i++) {
 		struct subsys_list_item *subsys = &slist[i];
 
diff --git a/nvme-print.c b/nvme-print.c
index 6f85e73..7316117 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2851,12 +2851,10 @@ static void show_nvme_subsystem(struct subsys_list_item *item)
 	printf("\\\n");
 
 	for (i = 0; i < item->nctrls; i++) {
-		printf(" +- %s %s %s %s %s\n", item->ctrls[i].name,
+		printf(" +- %s %s %s %s\n", item->ctrls[i].name,
 				item->ctrls[i].transport,
 				item->ctrls[i].address,
-				item->ctrls[i].state,
-				item->ctrls[i].ana_state ?
-					item->ctrls[i].ana_state : "");
+				item->ctrls[i].state);
 	}
 
 }
@@ -2902,10 +2900,6 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
 					slist[i].ctrls[j].address);
 			json_object_add_value_string(path_attrs, "State",
 					slist[i].ctrls[j].state);
-			if (slist[i].ctrls[j].ana_state)
-				json_object_add_value_string(path_attrs,
-						"ANAState",
-						slist[i].ctrls[j].ana_state);
 			json_array_add_value_object(paths, path_attrs);
 		}
 		if (j) {
diff --git a/nvme.c b/nvme.c
index 8edf398..227615e 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1397,85 +1397,6 @@ err_free_path:
 	return NULL;
 }
 
-static int scan_ctrl_paths_filter(const struct dirent *d)
-{
-	int id, cntlid, nsid;
-
-	if (d->d_name[0] == '.')
-		return 0;
-
-	if (strstr(d->d_name, "nvme")) {
-		if (sscanf(d->d_name, "nvme%dc%dn%d", &id, &cntlid, &nsid) == 3)
-			return 1;
-		if (sscanf(d->d_name, "nvme%dn%d", &id, &nsid) == 2)
-			return 1;
-	}
-
-	return 0;
-}
-
-static char *get_nvme_ctrl_path_ana_state(char *path, int nsid)
-{
-	struct dirent **paths;
-	char *ana_state;
-	int i, n;
-
-	ana_state = calloc(1, 16);
-	if (!ana_state)
-		return NULL;
-
-	n = scandir(path, &paths, scan_ctrl_paths_filter, alphasort);
-	if (n <= 0) {
-		free(ana_state);
-		return NULL;
-	}
-	for (i = 0; i < n; i++) {
-		int id, cntlid, ns, fd;
-		ssize_t ret;
-		char *ctrl_path;
-
-		if (sscanf(paths[i]->d_name, "nvme%dc%dn%d",
-			   &id, &cntlid, &ns) != 3) {
-			if (sscanf(paths[i]->d_name, "nvme%dn%d",
-				   &id, &ns) != 2) {
-				continue;
-			}
-		}
-		if (ns != nsid)
-			continue;
-
-		ret = asprintf(&ctrl_path, "%s/%s/ana_state",
-			       path, paths[i]->d_name);
-		if (ret < 0) {
-			free(ana_state);
-			ana_state = NULL;
-			break;
-		}
-		fd = open(ctrl_path, O_RDONLY);
-		if (fd < 0) {
-			free(ctrl_path);
-			free(ana_state);
-			ana_state = NULL;
-			break;
-		}
-		ret = read(fd, ana_state, 16);
-		if (ret < 0) {
-			fprintf(stderr, "Failed to read ANA state from %s\n",
-				ctrl_path);
-			free(ana_state);
-			ana_state = NULL;
-		} else if (ana_state[strlen(ana_state) - 1] == '\n')
-			ana_state[strlen(ana_state) - 1] = '\0';
-		close(fd);
-		free(ctrl_path);
-		break;
-	}
-	for (i = 0; i < n; i++)
-		free(paths[i]);
-	free(paths);
-	return ana_state;
-}
-
 static int scan_ctrls_filter(const struct dirent *d)
 {
 	int id, nsid;
@@ -1498,11 +1419,10 @@ static void free_ctrl_list_item(struct ctrl_list_item *ctrls)
 	free(ctrls->transport);
 	free(ctrls->address);
 	free(ctrls->state);
-	free(ctrls->ana_state);
 }
 
 static int get_nvme_subsystem_info(char *name, char *path,
-				struct subsys_list_item *item, __u32 nsid)
+				   struct subsys_list_item *item)
 {
 	char ctrl_path[512];
 	struct dirent **ctrls;
@@ -1560,9 +1480,6 @@ static int get_nvme_subsystem_info(char *name, char *path,
 			continue;
 		}
 
-		if (nsid != NVME_NSID_ALL)
-			item->ctrls[ccnt].ana_state =
-				get_nvme_ctrl_path_ana_state(ctrl_path, nsid);
 		ccnt++;
 	}
 
@@ -1627,8 +1544,7 @@ void free_subsys_list(struct subsys_list_item *slist, int n)
 	free(slist);
 }
 
-struct subsys_list_item *get_subsys_list(int *subcnt, char *subsysnqn,
-					 __u32 nsid)
+int get_subsys_list(int *subcnt, struct subsys_list_item **pslist)
 {
 	char path[310];
 	struct dirent **subsys;
@@ -1638,36 +1554,37 @@ struct subsys_list_item *get_subsys_list(int *subcnt, char *subsysnqn,
 	n = scandir(subsys_dir, &subsys, scan_subsys_filter, alphasort);
 	if (n < 0) {
 		fprintf(stderr, "no NVMe subsystem(s) detected.\n");
-		return NULL;
+		return -EINVAL;
 	}
 
 	slist = calloc(n, sizeof(struct subsys_list_item));
-	if (!slist)
+	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], nsid);
+				&slist[*subcnt]);
 		if (ret) {
 			fprintf(stderr,
 				"%s: failed to get subsystem info: %s\n",
 				path, strerror(errno));
 			free_subsys_list_item(&slist[*subcnt]);
-		} else if (subsysnqn &&
-			   strncmp(slist[*subcnt].subsysnqn, subsysnqn, 255))
-			free_subsys_list_item(&slist[*subcnt]);
-		else
+		} else {
 			(*subcnt)++;
+		}
 	}
 
+	*pslist = slist;
 free_subsys:
 	for (i = 0; i < n; i++)
 		free(subsys[i]);
 	free(subsys);
 
-	return slist;
+	return ret;
 }
 
 static int list_subsys(int argc, char **argv, struct command *cmd,
@@ -1675,15 +1592,12 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 {
 	struct subsys_list_item *slist;
 	int fmt, ret, subcnt = 0;
-	char *subsysnqn = NULL;
-	const char *desc = "Retrieve information for subsystems";
+	const char *desc = "Retrieve information for all NVMe subsystems.";
 	struct config {
-		__u32 namespace_id;
 		char *output_format;
 	};
 
 	struct config cfg = {
-		.namespace_id  = NVME_NSID_ALL,
 		.output_format = "normal",
 	};
 
@@ -1697,42 +1611,13 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 	if (ret < 0)
 		return ret;
 
-	devicename = NULL;
-	if (optind < argc) {
-		char path[512];
-		int id;
-
-		devicename = basename(argv[optind]);
-		if (sscanf(devicename, "nvme%dn%d", &id,
-			   &cfg.namespace_id) != 2) {
-			fprintf(stderr, "%s is not a NVMe namespace device\n",
-				argv[optind]);
-			return -EINVAL;
-		}
-		sprintf(path, "/sys/block/%s/device", devicename);
-		subsysnqn = get_nvme_subsnqn(path);
-		if (!subsysnqn) {
-			fprintf(stderr, "Cannot read subsys NQN from %s\n",
-				devicename);
-			return -EINVAL;
-		}
-		optind++;
-	}
-
-	if (ret < 0) {
-		argconfig_print_help(desc, opts);
-		if (subsysnqn)
-			free(subsysnqn);
-		return ret;
-	}
 	fmt = validate_output_format(cfg.output_format);
-	if (fmt != JSON && fmt != NORMAL) {
-		if (subsysnqn)
-			free(subsysnqn);
+	if (fmt != JSON && fmt != NORMAL)
 		return -EINVAL;
-	}
 
-	slist = get_subsys_list(&subcnt, subsysnqn, cfg.namespace_id);
+	ret = get_subsys_list(&subcnt, &slist);
+	if (ret)
+		return ret;
 
 	if (fmt == JSON)
 		json_print_nvme_subsystem_list(slist, subcnt);
@@ -1740,8 +1625,6 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 		show_nvme_subsystem_list(slist, subcnt);
 
 	free_subsys_list(slist, subcnt);
-	if (subsysnqn)
-		free(subsysnqn);
 	return ret;
 }
 
diff --git a/nvme.h b/nvme.h
index a149005..7c444a4 100644
--- a/nvme.h
+++ b/nvme.h
@@ -147,7 +147,6 @@ struct ctrl_list_item {
 	char *address;
 	char *transport;
 	char *state;
-	char *ana_state;
 };
 
 struct subsys_list_item {
@@ -174,7 +173,7 @@ 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, char *subsysnqn, __u32 nsid);
+int get_subsys_list(int *subcnt, struct subsys_list_item **pslist);
 void free_subsys_list(struct subsys_list_item *slist, int n);
 char *nvme_char_from_block(char *block);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 7/9] nvme-print: Introduce show_nvme_ctrl helper
  2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
                   ` (5 preceding siblings ...)
  2019-05-23  9:00 ` [PATCH 6/9] nvme: update list-subsys command to show the entire list Max Gurtovoy
@ 2019-05-23  9:00 ` Max Gurtovoy
  2019-05-23 15:12   ` Minwoo Im
  2019-05-23  9:00 ` [PATCH 8/9] nvme-print: Rename "Paths" --> "Ctrls" for json output in list-subsys Max Gurtovoy
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  9:00 UTC (permalink / raw)


This is a preparation patch for adding namespaces attribute for each
controller.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 nvme-print.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/nvme-print.c b/nvme-print.c
index 7316117..7e1f9ea 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2843,6 +2843,13 @@ void json_sanitize_log(struct nvme_sanitize_log_page *sanitize_log, const char *
 	json_free_object(root);
 }
 
+static void show_nvme_ctrl(struct ctrl_list_item *item)
+{
+	printf(" +- %s %s %s %s\n", item->name, item->transport, item->address,
+	       item->state);
+}
+
+
 static void show_nvme_subsystem(struct subsys_list_item *item)
 {
 	int i;
@@ -2850,13 +2857,8 @@ static void show_nvme_subsystem(struct subsys_list_item *item)
 	printf("%s - NQN=%s\n", item->name, item->subsysnqn);
 	printf("\\\n");
 
-	for (i = 0; i < item->nctrls; i++) {
-		printf(" +- %s %s %s %s\n", item->ctrls[i].name,
-				item->ctrls[i].transport,
-				item->ctrls[i].address,
-				item->ctrls[i].state);
-	}
-
+	for (i = 0; i < item->nctrls; i++)
+		show_nvme_ctrl(&item->ctrls[i]);
 }
 
 void show_nvme_subsystem_list(struct subsys_list_item *slist, int n)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 8/9] nvme-print: Rename "Paths" --> "Ctrls" for json output in list-subsys
  2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
                   ` (6 preceding siblings ...)
  2019-05-23  9:00 ` [PATCH 7/9] nvme-print: Introduce show_nvme_ctrl helper Max Gurtovoy
@ 2019-05-23  9:00 ` Max Gurtovoy
  2019-05-23  9:00 ` [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd Max Gurtovoy
  2019-05-24  7:35 ` [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Sagi Grimberg
  9 siblings, 0 replies; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  9:00 UTC (permalink / raw)


Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 nvme-print.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/nvme-print.c b/nvme-print.c
index 7e1f9ea..b3f08cf 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2904,10 +2904,8 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
 					slist[i].ctrls[j].state);
 			json_array_add_value_object(paths, path_attrs);
 		}
-		if (j) {
-			json_object_add_value_array(subsystem_attrs, "Paths", paths);
-		}
-
+		if (j)
+			json_object_add_value_array(subsystem_attrs, "Ctrls", paths);
 	}
 
 	if (i)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd
  2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
                   ` (7 preceding siblings ...)
  2019-05-23  9:00 ` [PATCH 8/9] nvme-print: Rename "Paths" --> "Ctrls" for json output in list-subsys Max Gurtovoy
@ 2019-05-23  9:00 ` Max Gurtovoy
  2019-05-23 15:11   ` Minwoo Im
  2019-05-24  7:35 ` [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Sagi Grimberg
  9 siblings, 1 reply; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  9:00 UTC (permalink / raw)


Add an association between subsystems/ctrls/namespaces using
"nvme list-subsys" command. Now this command will show the following:

nvme-subsys3 - NQN=nqn.2014.08.org.nvmexpress:80868086CVCQ5234001C400AGN INTEL SSDPEDMW400G4
\
 +- nvme0 pcie 0000:08:00.0 live
 \
  +- nvme3n1
nvme-subsys4 - NQN=testsubsystem_0
\
 +- nvme4 rdma traddr=1.212.99.85 trsvcid=4420 live
 \
  +- nvme4c1n1
  +- nvme4c1n2
  +- nvme4c1n3
 +- nvme5 rdma traddr=1.212.99.85 trsvcid=4420 live
 \
  +- nvme4c2n1
  +- nvme4c2n2
  +- nvme4c2n3

Instead of:
----------------
nvme-subsys3 - NQN=nqn.2014.08.org.nvmexpress:80868086CVCQ5234001C400AGN INTEL SSDPEDMW400G4
\
 +- nvme0 pcie 0000:08:00.0
nvme-subsys4 - NQN=testsubsystem_0
\
 +- nvme4 rdma traddr=1.212.99.85 trsvcid=4420
 +- nvme5 rdma traddr=1.212.99.85 trsvcid=4420

The new json output is (partial):
--------------------------
{
      "Name" : "nvme-subsys4",
      "NQN" : "testsubsystem_0",
      "Ctrls" : [
        {
          "Name" : "nvme4",
          "Transport" : "rdma",
          "Address" : "traddr=1.212.99.85 trsvcid=4420",
          "State" : "live",
          "Namespaces" : [
            {
              "Name" : "nvme4c1n1"
            },
            {
              "Name" : "nvme4c1n2"
            },
            {
              "Name" : "nvme4c1n3"
            }
          ]
        },
        {
          "Name" : "nvme5",
          "Transport" : "rdma",
          "Address" : "traddr=1.212.99.85 trsvcid=4420",
          "State" : "live",
          "Namespaces" : [
            {
              "Name" : "nvme4c2n1"
            },
            {
              "Name" : "nvme4c2n2"
            },
            {
              "Name" : "nvme4c2n3"
            }
          ]
        }
      ]
}

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 nvme-print.c |  23 ++++++++++++-
 nvme.c       | 110 ++++++++++++++++++++++++++++++++++++++++++++---------------
 nvme.h       |   6 ++++
 3 files changed, 111 insertions(+), 28 deletions(-)

diff --git a/nvme-print.c b/nvme-print.c
index b3f08cf..7625559 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2845,8 +2845,15 @@ void json_sanitize_log(struct nvme_sanitize_log_page *sanitize_log, const char *
 
 static void show_nvme_ctrl(struct ctrl_list_item *item)
 {
+	int i;
+
 	printf(" +- %s %s %s %s\n", item->name, item->transport, item->address,
 	       item->state);
+	for (i = 0; i < item->ns_num; i++) {
+		if (!i)
+			printf(" \\\n");
+		printf("  +- %s\n", item->namespaces[i].name);
+	}
 }
 
 
@@ -2875,7 +2882,9 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
 	struct json_object *subsystem_attrs;
 	struct json_array *paths;
 	struct json_object *path_attrs;
-	int i, j;
+	struct json_array *namespaces;
+	struct json_object *ns_attrs;
+	int i, j, k;
 
 	root = json_create_object();
 	subsystems = json_create_array();
@@ -2902,6 +2911,18 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
 					slist[i].ctrls[j].address);
 			json_object_add_value_string(path_attrs, "State",
 					slist[i].ctrls[j].state);
+
+			namespaces = json_create_array();
+			for (k = 0; k < slist[i].ctrls[j].ns_num; k++) {
+				ns_attrs = json_create_object();
+				json_object_add_value_string(ns_attrs, "Name",
+							     slist[i].ctrls[j].namespaces[k].name);
+				json_array_add_value_object(namespaces, ns_attrs);
+			}
+			if (k)
+				json_object_add_value_array(path_attrs, "Namespaces",
+							    namespaces);
+
 			json_array_add_value_object(paths, path_attrs);
 		}
 		if (j)
diff --git a/nvme.c b/nvme.c
index 227615e..0ab63e8 100644
--- a/nvme.c
+++ b/nvme.c
@@ -60,6 +60,9 @@ const char *devicename;
 
 static const char nvme_version_string[] = NVME_VERSION;
 
+static void free_ctrl_list_item(struct ctrl_list_item *ctrls);
+static char *get_nvme_ctrl_attr(char *path, const char *attr);
+
 #define CREATE_CMD
 #include "nvme-builtin.h"
 
@@ -1309,6 +1312,76 @@ static void *get_registers(void)
 
 static const char *subsys_dir = "/sys/class/nvme-subsystem/";
 
+static int scan_ns_filter(const struct dirent *d)
+{
+	int id, cntlid, nsid;
+
+	if (strstr(d->d_name, "nvme")) {
+		if (sscanf(d->d_name, "nvme%dc%dn%d", &id, &cntlid, &nsid) == 3)
+			return 1;
+		if (sscanf(d->d_name, "nvme%dn%d", &id, &nsid) == 2)
+			return 1;
+	}
+
+	return 0;
+}
+
+static int get_nvme_ctrl_info(char *name, char *ctrl_path,
+			      struct ctrl_list_item *item)
+{
+	struct dirent **namespaces;
+	int ret = 0, n, i;
+
+	item->name = strdup(name);
+
+	n = scandir(ctrl_path, &namespaces, scan_ns_filter, alphasort);
+	if (n < 0) {
+		fprintf(stderr, "failed to scan namespace(s).\n");
+		return n;
+	}
+
+	item->namespaces = calloc(n, sizeof(struct ns_list_item));
+	if (!item->namespaces) {
+		ret = -ENOMEM;
+		fprintf(stderr, "failed to allocate controller namespace(s)\n");
+		goto free_namespaces;
+	}
+
+	item->ns_num = n;
+
+	for (i = 0; i < n; i++)
+		item->namespaces[i].name = strdup(namespaces[i]->d_name);
+
+	item->address = get_nvme_ctrl_attr(ctrl_path, "address");
+	if (!item->address) {
+		fprintf(stderr, "failed to get %s address.\n", name);
+		free_ctrl_list_item(item);
+		ret = -EAGAIN;
+		goto free_namespaces;
+	}
+	item->transport = get_nvme_ctrl_attr(ctrl_path, "transport");
+	if (!item->transport) {
+		fprintf(stderr, "failed to get %s transport.\n", name);
+		free_ctrl_list_item(item);
+		ret = -EAGAIN;
+		goto free_namespaces;
+	}
+	item->state = get_nvme_ctrl_attr(ctrl_path, "state");
+	if (!item->state) {
+		fprintf(stderr, "failed to get %s state.\n", name);
+		free_ctrl_list_item(item);
+		ret = -EAGAIN;
+		goto free_namespaces;
+	}
+
+free_namespaces:
+	for (i = 0; i < n; i++)
+		free(namespaces[i]);
+	free(namespaces);
+
+	return ret;
+}
+
 static char *get_nvme_subsnqn(char *path)
 {
 	char sspath[320];
@@ -1415,10 +1488,15 @@ static int scan_ctrls_filter(const struct dirent *d)
 
 static void free_ctrl_list_item(struct ctrl_list_item *ctrls)
 {
+	int i;
+
 	free(ctrls->name);
 	free(ctrls->transport);
 	free(ctrls->address);
 	free(ctrls->state);
+	for (i = 0; i < ctrls->ns_num; i++)
+		free(ctrls->namespaces[i].name);
+	free(ctrls->namespaces);
 }
 
 static int get_nvme_subsystem_info(char *name, char *path,
@@ -1451,36 +1529,14 @@ 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;
-		}
-
-		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;
-		}
+			 ctrls[i]->d_name);
 
-		ccnt++;
+		ret = get_nvme_ctrl_info(ctrls[i]->d_name, ctrl_path,
+					 &(item->ctrls[ccnt]));
+		if (!ret)
+			ccnt++;
 	}
 
 	item->nctrls = ccnt;
diff --git a/nvme.h b/nvme.h
index 7c444a4..84eaa9f 100644
--- a/nvme.h
+++ b/nvme.h
@@ -142,11 +142,17 @@ struct list_item {
 	unsigned            block;
 };
 
+struct ns_list_item {
+	char *name;
+};
+
 struct ctrl_list_item {
 	char *name;
 	char *address;
 	char *transport;
 	char *state;
+	int ns_num;
+	struct ns_list_item *namespaces;
 };
 
 struct subsys_list_item {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd
  2019-05-23  9:00 ` [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd Max Gurtovoy
@ 2019-05-23 15:11   ` Minwoo Im
  0 siblings, 0 replies; 22+ messages in thread
From: Minwoo Im @ 2019-05-23 15:11 UTC (permalink / raw)


On 19-05-23 12:00:58, Max Gurtovoy wrote:
> Add an association between subsystems/ctrls/namespaces using
> "nvme list-subsys" command. Now this command will show the following:
> 
> nvme-subsys3 - NQN=nqn.2014.08.org.nvmexpress:80868086CVCQ5234001C400AGN INTEL SSDPEDMW400G4
> \
>  +- nvme0 pcie 0000:08:00.0 live
>  \
>   +- nvme3n1
> nvme-subsys4 - NQN=testsubsystem_0
> \
>  +- nvme4 rdma traddr=1.212.99.85 trsvcid=4420 live
>  \
>   +- nvme4c1n1
>   +- nvme4c1n2
>   +- nvme4c1n3
>  +- nvme5 rdma traddr=1.212.99.85 trsvcid=4420 live
>  \
>   +- nvme4c2n1
>   +- nvme4c2n2
>   +- nvme4c2n3
> 

Tested-by: Minwoo Im <minwoo.im.dev at gmail.com>

> The new json output is (partial):
> --------------------------
> {
>       "Name" : "nvme-subsys4",
>       "NQN" : "testsubsystem_0",
>       "Ctrls" : [
>         {
>           "Name" : "nvme4",
>           "Transport" : "rdma",
>           "Address" : "traddr=1.212.99.85 trsvcid=4420",
>           "State" : "live",
>           "Namespaces" : [
>             {
>               "Name" : "nvme4c1n1"
>             },
>             {
>               "Name" : "nvme4c1n2"
>             },
>             {
>               "Name" : "nvme4c1n3"
>             }
>           ]
>         },
>         {
>           "Name" : "nvme5",
>           "Transport" : "rdma",
>           "Address" : "traddr=1.212.99.85 trsvcid=4420",
>           "State" : "live",
>           "Namespaces" : [
>             {
>               "Name" : "nvme4c2n1"
>             },
>             {
>               "Name" : "nvme4c2n2"
>             },
>             {
>               "Name" : "nvme4c2n3"
>             }
>           ]
>         }
>       ]
> }
> 
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 7/9] nvme-print: Introduce show_nvme_ctrl helper
  2019-05-23  9:00 ` [PATCH 7/9] nvme-print: Introduce show_nvme_ctrl helper Max Gurtovoy
@ 2019-05-23 15:12   ` Minwoo Im
  0 siblings, 0 replies; 22+ messages in thread
From: Minwoo Im @ 2019-05-23 15:12 UTC (permalink / raw)


On 19-05-23 12:00:56, Max Gurtovoy wrote:
> This is a preparation patch for adding namespaces attribute for each
> controller.
> 
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
> ---
>  nvme-print.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/nvme-print.c b/nvme-print.c
> index 7316117..7e1f9ea 100644
> --- a/nvme-print.c
> +++ b/nvme-print.c
> @@ -2843,6 +2843,13 @@ void json_sanitize_log(struct nvme_sanitize_log_page *sanitize_log, const char *
>  	json_free_object(root);
>  }
>  
> +static void show_nvme_ctrl(struct ctrl_list_item *item)
> +{
> +	printf(" +- %s %s %s %s\n", item->name, item->transport, item->address,
> +	       item->state);
> +}
> +
> +
>  static void show_nvme_subsystem(struct subsys_list_item *item)
>  {
>  	int i;
> @@ -2850,13 +2857,8 @@ static void show_nvme_subsystem(struct subsys_list_item *item)
>  	printf("%s - NQN=%s\n", item->name, item->subsysnqn);
>  	printf("\\\n");
>  
> -	for (i = 0; i < item->nctrls; i++) {
> -		printf(" +- %s %s %s %s\n", item->ctrls[i].name,
> -				item->ctrls[i].transport,
> -				item->ctrls[i].address,
> -				item->ctrls[i].state);
> -	}
> -
> +	for (i = 0; i < item->nctrls; i++)
> +		show_nvme_ctrl(&item->ctrls[i]);
>  }


This looks good to me.

Reviewed-by: Minwoo Im <minwoo.im.dev at gmail.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/9] nvme: update list-ns nsid option
  2019-05-23  9:00 ` [PATCH 1/9] nvme: update list-ns nsid option Max Gurtovoy
@ 2019-05-23 17:27   ` Heitke, Kenneth
  2019-05-24 19:17     ` Max Gurtovoy
  0 siblings, 1 reply; 22+ messages in thread
From: Heitke, Kenneth @ 2019-05-23 17:27 UTC (permalink / raw)




On 5/23/2019 3:00 AM, Max Gurtovoy wrote:
> This commit updates the optional nsid argument to define the wanted
> nsid for start, instead of starting from nsid + 1. E.g. in case we've
> wanted to get the list of namespaces starting from 1, before this
> commit, we used the "--namespace-id=0" option. Nsid 0 is not valid in
> NVMe spec, thus change it to start counting from the given nsid.
> 
> Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
> ---
>   nvme.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/nvme.c b/nvme.c
> index 9819fcb..3c1bcb1 100644
> --- a/nvme.c
> +++ b/nvme.c
> @@ -950,9 +950,9 @@ close_fd:
>   
>   static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
>   {
> -	const char *desc = "For the specified device, show the "\
> -		"namespace list in a NVMe subsystem, optionally starting with a given namespace";
> -	const char *namespace_id = "namespace number returned list should to start after";
> +	const char *desc = "For the specified controller handle, show the "\
> +		"namespace list in the associated NVMe subsystem, optionally starting with a given nsid.";
> +	const char *namespace_id = "first nsid returned list should start from";
>   	const char *all = "show all namespaces in the subsystem, whether attached or inactive";
>   	int err, i, fd;
>   	__u32 ns_list[1024];
> @@ -963,7 +963,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>   	};
>   
>   	struct config cfg = {
> -		.namespace_id = 0,
> +		.namespace_id = 1,
>   	};
>   
>   	const struct argconfig_commandline_options command_line_options[] = {
> @@ -976,7 +976,11 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>   	if (fd < 0)
>   		return fd;
>   
> -	err = nvme_identify_ns_list(fd, cfg.namespace_id, !!cfg.all, ns_list);
> +	if (!cfg.namespace_id)
> +		return -EINVAL;

Would it be useful to include an error message here? I don't like
playing the guessing game about what I did wrong when I just get the
help message back.

> +
> +	err = nvme_identify_ns_list(fd, cfg.namespace_id - 1, !!cfg.all,
> +				    ns_list);
>   	if (!err) {
>   		for (i = 0; i < 1024; i++)
>   			if (ns_list[i])
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli)
  2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
                   ` (8 preceding siblings ...)
  2019-05-23  9:00 ` [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd Max Gurtovoy
@ 2019-05-24  7:35 ` Sagi Grimberg
  9 siblings, 0 replies; 22+ messages in thread
From: Sagi Grimberg @ 2019-05-24  7:35 UTC (permalink / raw)



> This patchset introduce few small bug fixes for memory leaks, improve
> coding style, improve code readability and mainly focus on the "nvme list-subsys"
> command. This command will show the whole NVM subsystem list (same as "nvme list"
> does for namespaces).
> The patchset ends with adding namespaces entry per each ctrl in the system.
> Actually, at first stage, we'll print only the namespace handle (nvme0n1 for
> example). Later on, we can add more attribute per demand (uuid, nguid, capacity,
> etc...).

Can you split patches 1-5 to a separate fixes series? Would be easier to
review.

As for the list-subsys, I think that when passing the namespace handle
it should only output the corresponding namespace from each controller.

list-subsys in my mind should probably stay as is without a namespace
handle passed because it doesn't give any meaningful information without
the mpath-namespace to slaves mapping.

Instead, a more effective display would be the mapping of each mpath
namespace to the corresponding slave namespaces. Perhaps with a new
command "list-mpath"?

On a side question, what does this information give the user if the
slave devices don't have a device node?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/9] nvme: update list-ns nsid option
  2019-05-23 17:27   ` Heitke, Kenneth
@ 2019-05-24 19:17     ` Max Gurtovoy
  0 siblings, 0 replies; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-24 19:17 UTC (permalink / raw)



On 5/23/2019 8:27 PM, Heitke, Kenneth wrote:
> Would it be useful to include an error message here? I don't like
> playing the guessing game about what I did wrong when I just get the
> help message back. 

Yup good idea. I'll fix that in V3.

-Max.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd
  2019-05-27 10:01 ` [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd Max Gurtovoy
@ 2019-05-28 19:54   ` Sagi Grimberg
  0 siblings, 0 replies; 22+ messages in thread
From: Sagi Grimberg @ 2019-05-28 19:54 UTC (permalink / raw)



> Add an association between subsystems/ctrls/namespaces using
> "nvme list-subsys" command. Now this command will show the following
> (notice that nvme-subsys4 is a dual controller subsystem that runs a
> native multipath configuration. Hidden block devices not presented):

I'm having a problem with the removal of the ANA state and the namespace
handle removal.

If you insist of having this display, please introduce it in a separate
command, don't just change how list-subsys acts (which is actually kinda
useful).

> nvme-subsys3 - NQN=nqn.2014.08.org.nvmexpress:80868086CVCQ5234001C400AGN INTEL SSDPEDMW400G4
> \
>   +- nvme0 pcie 0000:08:00.0 live
>   \
>    +- nvme3n1
> nvme-subsys4 - NQN=testsubsystem_0
> \
>   +- nvme4 rdma traddr=1.212.99.85 trsvcid=4420 live
>   \
>    +- nvme4n1
>    +- nvme4n2
>    +- nvme4n3
>   +- nvme5 rdma traddr=1.212.99.85 trsvcid=4420 live
>   \
>    +- nvme4n1
>    +- nvme4n2
>    +- nvme4n3

This is definitely better than the hidden devices though...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd
  2019-05-27 10:01 [PATCH v3 " Max Gurtovoy
@ 2019-05-27 10:01 ` Max Gurtovoy
  2019-05-28 19:54   ` Sagi Grimberg
  0 siblings, 1 reply; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-27 10:01 UTC (permalink / raw)


Add an association between subsystems/ctrls/namespaces using
"nvme list-subsys" command. Now this command will show the following
(notice that nvme-subsys4 is a dual controller subsystem that runs a
native multipath configuration. Hidden block devices not presented):

nvme-subsys3 - NQN=nqn.2014.08.org.nvmexpress:80868086CVCQ5234001C400AGN INTEL SSDPEDMW400G4
\
 +- nvme0 pcie 0000:08:00.0 live
 \
  +- nvme3n1
nvme-subsys4 - NQN=testsubsystem_0
\
 +- nvme4 rdma traddr=1.212.99.85 trsvcid=4420 live
 \
  +- nvme4n1
  +- nvme4n2
  +- nvme4n3
 +- nvme5 rdma traddr=1.212.99.85 trsvcid=4420 live
 \
  +- nvme4n1
  +- nvme4n2
  +- nvme4n3

Instead of:
----------------
nvme-subsys3 - NQN=nqn.2014.08.org.nvmexpress:80868086CVCQ5234001C400AGN INTEL SSDPEDMW400G4
\
 +- nvme0 pcie 0000:08:00.0
nvme-subsys4 - NQN=testsubsystem_0
\
 +- nvme4 rdma traddr=1.212.99.85 trsvcid=4420
 +- nvme5 rdma traddr=1.212.99.85 trsvcid=4420

The new json output is (partial):
--------------------------
{
      "Name" : "nvme-subsys4",
      "NQN" : "testsubsystem_0",
      "Ctrls" : [
        {
          "Name" : "nvme4",
          "Transport" : "rdma",
          "Address" : "traddr=1.212.99.85 trsvcid=4420",
          "State" : "live",
          "Namespaces" : [
            {
              "Name" : "nvme4n1"
            },
            {
              "Name" : "nvme4n2"
            },
            {
              "Name" : "nvme4n3"
            }
          ]
        },
        {
          "Name" : "nvme5",
          "Transport" : "rdma",
          "Address" : "traddr=1.212.99.85 trsvcid=4420",
          "State" : "live",
          "Namespaces" : [
            {
              "Name" : "nvme4n1"
            },
            {
              "Name" : "nvme4n2"
            },
            {
              "Name" : "nvme4n3"
            }
          ]
        }
      ]
}

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 nvme-print.c |  23 ++++++++++-
 nvme.c       | 130 ++++++++++++++++++++++++++++++++++++++++++++++-------------
 nvme.h       |   6 +++
 3 files changed, 130 insertions(+), 29 deletions(-)

diff --git a/nvme-print.c b/nvme-print.c
index b3f08cf..7625559 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2845,8 +2845,15 @@ void json_sanitize_log(struct nvme_sanitize_log_page *sanitize_log, const char *
 
 static void show_nvme_ctrl(struct ctrl_list_item *item)
 {
+	int i;
+
 	printf(" +- %s %s %s %s\n", item->name, item->transport, item->address,
 	       item->state);
+	for (i = 0; i < item->ns_num; i++) {
+		if (!i)
+			printf(" \\\n");
+		printf("  +- %s\n", item->namespaces[i].name);
+	}
 }
 
 
@@ -2875,7 +2882,9 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
 	struct json_object *subsystem_attrs;
 	struct json_array *paths;
 	struct json_object *path_attrs;
-	int i, j;
+	struct json_array *namespaces;
+	struct json_object *ns_attrs;
+	int i, j, k;
 
 	root = json_create_object();
 	subsystems = json_create_array();
@@ -2902,6 +2911,18 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
 					slist[i].ctrls[j].address);
 			json_object_add_value_string(path_attrs, "State",
 					slist[i].ctrls[j].state);
+
+			namespaces = json_create_array();
+			for (k = 0; k < slist[i].ctrls[j].ns_num; k++) {
+				ns_attrs = json_create_object();
+				json_object_add_value_string(ns_attrs, "Name",
+							     slist[i].ctrls[j].namespaces[k].name);
+				json_array_add_value_object(namespaces, ns_attrs);
+			}
+			if (k)
+				json_object_add_value_array(path_attrs, "Namespaces",
+							    namespaces);
+
 			json_array_add_value_object(paths, path_attrs);
 		}
 		if (j)
diff --git a/nvme.c b/nvme.c
index 518a396..ef01ead 100644
--- a/nvme.c
+++ b/nvme.c
@@ -60,6 +60,9 @@ const char *devicename;
 
 static const char nvme_version_string[] = NVME_VERSION;
 
+static void free_ctrl_list_item(struct ctrl_list_item *ctrls);
+static char *get_nvme_ctrl_attr(char *path, const char *attr);
+
 #define CREATE_CMD
 #include "nvme-builtin.h"
 
@@ -1313,6 +1316,82 @@ static void *get_registers(void)
 
 static const char *subsys_dir = "/sys/class/nvme-subsystem/";
 
+static int scan_ns_filter(const struct dirent *d)
+{
+	int id, nsid;
+
+	if (strstr(d->d_name, "nvme")) {
+		if (sscanf(d->d_name, "nvme%dn%d", &id, &nsid) == 2)
+			return 1;
+	}
+
+	return 0;
+}
+
+static int get_nvme_ctrl_info(char *name, char *ctrl_path,
+			      struct ctrl_list_item *item, int heads_num,
+			      struct dirent **heads)
+{
+	struct dirent **namespaces;
+	int ret = 0, n, i;
+
+	item->name = strdup(name);
+
+	if (heads_num) {
+		n = heads_num;
+		namespaces = heads;
+	} else {
+		n = scandir(ctrl_path, &namespaces, scan_ns_filter, alphasort);
+		if (n < 0) {
+			fprintf(stderr, "failed to scan namespace(s).\n");
+			return n;
+		}
+	}
+
+	item->namespaces = calloc(n, sizeof(struct ns_list_item));
+	if (!item->namespaces) {
+		ret = -ENOMEM;
+		fprintf(stderr, "failed to allocate controller namespace(s)\n");
+		goto free_namespaces;
+	}
+
+	item->ns_num = n;
+
+	for (i = 0; i < n; i++)
+		item->namespaces[i].name = strdup(namespaces[i]->d_name);
+
+	item->address = get_nvme_ctrl_attr(ctrl_path, "address");
+	if (!item->address) {
+		fprintf(stderr, "failed to get %s address.\n", name);
+		free_ctrl_list_item(item);
+		ret = -EAGAIN;
+		goto free_namespaces;
+	}
+	item->transport = get_nvme_ctrl_attr(ctrl_path, "transport");
+	if (!item->transport) {
+		fprintf(stderr, "failed to get %s transport.\n", name);
+		free_ctrl_list_item(item);
+		ret = -EAGAIN;
+		goto free_namespaces;
+	}
+	item->state = get_nvme_ctrl_attr(ctrl_path, "state");
+	if (!item->state) {
+		fprintf(stderr, "failed to get %s state.\n", name);
+		free_ctrl_list_item(item);
+		ret = -EAGAIN;
+		goto free_namespaces;
+	}
+
+free_namespaces:
+	if (!heads) {
+		for (i = 0; i < n; i++)
+			free(namespaces[i]);
+		free(namespaces);
+	}
+
+	return ret;
+}
+
 static char *get_nvme_subsnqn(char *path)
 {
 	char sspath[320];
@@ -1419,10 +1498,15 @@ static int scan_ctrls_filter(const struct dirent *d)
 
 static void free_ctrl_list_item(struct ctrl_list_item *ctrls)
 {
+	int i;
+
 	free(ctrls->name);
 	free(ctrls->transport);
 	free(ctrls->address);
 	free(ctrls->state);
+	for (i = 0; i < ctrls->ns_num; i++)
+		free(ctrls->namespaces[i].name);
+	free(ctrls->namespaces);
 }
 
 static int get_nvme_subsystem_info(char *name, char *path,
@@ -1430,7 +1514,8 @@ static int get_nvme_subsystem_info(char *name, char *path,
 {
 	char ctrl_path[512];
 	struct dirent **ctrls;
-	int n, i, ret = 1, ccnt = 0;
+	struct dirent **heads;
+	int n, heads_num, i, ret = 1, ccnt = 0;
 
 	item->subsysnqn = get_nvme_subsnqn(path);
 	if (!item->subsysnqn) {
@@ -1454,43 +1539,32 @@ static int get_nvme_subsystem_info(char *name, char *path,
 
 	item->nctrls = n;
 
+	heads_num = scandir(path, &heads, scan_ns_filter, alphasort);
+	if (heads_num < 0) {
+		fprintf(stderr, "failed to scan heads(s).\n");
+		goto free_ctrls;
+	}
+
 	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;
-		}
+			 ctrls[i]->d_name);
 
-		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;
-		}
-
-		ccnt++;
+		ret = get_nvme_ctrl_info(ctrls[i]->d_name, ctrl_path,
+					 &(item->ctrls[ccnt]), heads_num, heads);
+		if (!ret)
+			ccnt++;
 	}
 
 	item->nctrls = ccnt;
 
 	ret = 0;
 
+	if (heads_num) {
+		for (i = 0; i < heads_num; i++)
+			free(heads[i]);
+		free(heads);
+	}
 free_ctrls:
 	for (i = 0; i < n; i++)
 		free(ctrls[i]);
diff --git a/nvme.h b/nvme.h
index 7c444a4..84eaa9f 100644
--- a/nvme.h
+++ b/nvme.h
@@ -142,11 +142,17 @@ struct list_item {
 	unsigned            block;
 };
 
+struct ns_list_item {
+	char *name;
+};
+
 struct ctrl_list_item {
 	char *name;
 	char *address;
 	char *transport;
 	char *state;
+	int ns_num;
+	struct ns_list_item *namespaces;
 };
 
 struct subsys_list_item {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd
  2019-05-23 13:39       ` Keith Busch
@ 2019-05-23 14:07         ` hch
  0 siblings, 0 replies; 22+ messages in thread
From: hch @ 2019-05-23 14:07 UTC (permalink / raw)


On Thu, May 23, 2019@07:39:46AM -0600, Keith Busch wrote:
> > if this if fine I'll send a V2 with a small fix.
> 
> I think we'd actually prefer not seeing the hidden block devices, but
> instead use the device name from the "head": nvme4n1, nvme4n2, etc...

Agreed.  The hidden devices are an artefact we should expose to the
user as little as possible.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd
  2019-05-23  8:01     ` Max Gurtovoy
@ 2019-05-23 13:39       ` Keith Busch
  2019-05-23 14:07         ` hch
  0 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2019-05-23 13:39 UTC (permalink / raw)


On Thu, May 23, 2019@01:01:54AM -0700, Max Gurtovoy wrote:
> 
> On 5/23/2019 1:49 AM, Keith Busch wrote:
> > On Wed, May 22, 2019@07:39:53PM +0300, Max Gurtovoy wrote:
> >> Add an association between subsystems/ctrls/namespaces using
> >> "nvme list-subsys" command. Now this command will show the following:
> >>
> >> nvme-subsys4 - NQN=testsubsystem_0
> >> \
> >>   +- nvme4 rdma traddr=12.212.99.85 trsvcid=4420 live
> >>   \
> >>    +- nvme4n1
> >>    +- nvme4n2
> >>    +- nvme4n3
> >>   +- nvme5 rdma traddr=12.212.99.85 trsvcid=4420 live
> >>   \
> >>    +- nvme5n1
> >>    +- nvme5n2
> >>    +- nvme5n3
> >>
> >> Instead of:
> >> ----------------
> >> nvme-subsys4 - NQN=testsubsystem_0
> >> \
> >>   +- nvme4 rdma traddr=12.212.99.85 trsvcid=4420
> >>   +- nvme5 rdma traddr=12.212.99.85 trsvcid=4420
> > This is a terrific start. Your output indicates you are not using
> > nvme native multipathing, though. Could you retry this command with
> > that enabled (I'll try it tomorrow as well)?
> 
> Yup I'll configure the native multipath. But what is expected ?
> 
> should we print nvme4c1n1/nvme4c1n2/nvme4c1n3 for nvme4
> 
> and nvme4c2n1/nvme4c2n2/nvme4c2n3 for nvme5 ?
> 
> if so, I guess the information on the "IO-ble" devices 
> nvme4n1/nvme4n2/nvme4n3 will not be printed.
> 
> something like:
> 
> nvme-subsys4 - NQN=testsubsystem_0
> \
>  ?+- nvme4 rdma traddr=12.212.99.85 trsvcid=4420 live
>  ?\
>  ? +- nvme4c1n1
>  ? +- nvme4c1n2
>  ? +- nvme4c1n3
>  ?+- nvme5 rdma traddr=12.212.99.85 trsvcid=4420 live
>  ?\
>  ? +- nvme4c2n1
>  ? +- nvme4c2n2
>  ? +- nvme4c2n3
> 
> 
> if this if fine I'll send a V2 with a small fix.

I think we'd actually prefer not seeing the hidden block devices, but
instead use the device name from the "head": nvme4n1, nvme4n2, etc...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd
  2019-05-22 22:49   ` Keith Busch
@ 2019-05-23  8:01     ` Max Gurtovoy
  2019-05-23 13:39       ` Keith Busch
  0 siblings, 1 reply; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-23  8:01 UTC (permalink / raw)



On 5/23/2019 1:49 AM, Keith Busch wrote:
> On Wed, May 22, 2019@07:39:53PM +0300, Max Gurtovoy wrote:
>> Add an association between subsystems/ctrls/namespaces using
>> "nvme list-subsys" command. Now this command will show the following:
>>
>> nvme-subsys4 - NQN=testsubsystem_0
>> \
>>   +- nvme4 rdma traddr=12.212.99.85 trsvcid=4420 live
>>   \
>>    +- nvme4n1
>>    +- nvme4n2
>>    +- nvme4n3
>>   +- nvme5 rdma traddr=12.212.99.85 trsvcid=4420 live
>>   \
>>    +- nvme5n1
>>    +- nvme5n2
>>    +- nvme5n3
>>
>> Instead of:
>> ----------------
>> nvme-subsys4 - NQN=testsubsystem_0
>> \
>>   +- nvme4 rdma traddr=12.212.99.85 trsvcid=4420
>>   +- nvme5 rdma traddr=12.212.99.85 trsvcid=4420
> This is a terrific start. Your output indicates you are not using
> nvme native multipathing, though. Could you retry this command with
> that enabled (I'll try it tomorrow as well)?

Yup I'll configure the native multipath. But what is expected ?

should we print nvme4c1n1/nvme4c1n2/nvme4c1n3 for nvme4

and nvme4c2n1/nvme4c2n2/nvme4c2n3 for nvme5 ?

if so, I guess the information on the "IO-ble" devices 
nvme4n1/nvme4n2/nvme4n3 will not be printed.

something like:

nvme-subsys4 - NQN=testsubsystem_0
\
 ?+- nvme4 rdma traddr=12.212.99.85 trsvcid=4420 live
 ?\
 ? +- nvme4c1n1
 ? +- nvme4c1n2
 ? +- nvme4c1n3
 ?+- nvme5 rdma traddr=12.212.99.85 trsvcid=4420 live
 ?\
 ? +- nvme4c2n1
 ? +- nvme4c2n2
 ? +- nvme4c2n3


if this if fine I'll send a V2 with a small fix.

-Max.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd
  2019-05-22 16:39 ` [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd Max Gurtovoy
@ 2019-05-22 22:49   ` Keith Busch
  2019-05-23  8:01     ` Max Gurtovoy
  0 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2019-05-22 22:49 UTC (permalink / raw)


On Wed, May 22, 2019@07:39:53PM +0300, Max Gurtovoy wrote:
> Add an association between subsystems/ctrls/namespaces using
> "nvme list-subsys" command. Now this command will show the following:
> 
> nvme-subsys4 - NQN=testsubsystem_0
> \
>  +- nvme4 rdma traddr=12.212.99.85 trsvcid=4420 live
>  \
>   +- nvme4n1
>   +- nvme4n2
>   +- nvme4n3
>  +- nvme5 rdma traddr=12.212.99.85 trsvcid=4420 live
>  \
>   +- nvme5n1
>   +- nvme5n2
>   +- nvme5n3
> 
> Instead of:
> ----------------
> nvme-subsys4 - NQN=testsubsystem_0
> \
>  +- nvme4 rdma traddr=12.212.99.85 trsvcid=4420
>  +- nvme5 rdma traddr=12.212.99.85 trsvcid=4420

This is a terrific start. Your output indicates you are not using
nvme native multipathing, though. Could you retry this command with
that enabled (I'll try it tomorrow as well)?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd
  2019-05-22 16:39 [PATCH 0/9] Add Subsystem/Ctrl/Namespace relations Max Gurtovoy
@ 2019-05-22 16:39 ` Max Gurtovoy
  2019-05-22 22:49   ` Keith Busch
  0 siblings, 1 reply; 22+ messages in thread
From: Max Gurtovoy @ 2019-05-22 16:39 UTC (permalink / raw)


Add an association between subsystems/ctrls/namespaces using
"nvme list-subsys" command. Now this command will show the following:

nvme-subsys4 - NQN=testsubsystem_0
\
 +- nvme4 rdma traddr=12.212.99.85 trsvcid=4420 live
 \
  +- nvme4n1
  +- nvme4n2
  +- nvme4n3
 +- nvme5 rdma traddr=12.212.99.85 trsvcid=4420 live
 \
  +- nvme5n1
  +- nvme5n2
  +- nvme5n3

Instead of:
----------------
nvme-subsys4 - NQN=testsubsystem_0
\
 +- nvme4 rdma traddr=12.212.99.85 trsvcid=4420
 +- nvme5 rdma traddr=12.212.99.85 trsvcid=4420

The new json output is:
--------------------------
{
      "Name" : "nvme-subsys4",
      "NQN" : "testsubsystem_0",
      "Ctrls" : [
        {
          "Name" : "nvme4",
          "Transport" : "rdma",
          "Address" : "traddr=12.212.99.85 trsvcid=4420",
          "State" : "live",
          "Namespaces" : [
            {
              "Name" : "nvme4n1"
            },
            {
              "Name" : "nvme4n2"
            },
            {
              "Name" : "nvme4n3"
            }
          ]
        },
        {
          "Name" : "nvme5",
          "Transport" : "rdma",
          "Address" : "traddr=12.212.99.85 trsvcid=4420",
          "State" : "live",
          "Namespaces" : [
            {
              "Name" : "nvme5n1"
            },
            {
              "Name" : "nvme5n2"
            },
            {
              "Name" : "nvme5n3"
            }
          ]
        }
      ]
}

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 nvme-print.c |  22 ++++++++++++-
 nvme.c       | 106 ++++++++++++++++++++++++++++++++++++++++++++---------------
 nvme.h       |   6 ++++
 3 files changed, 106 insertions(+), 28 deletions(-)

diff --git a/nvme-print.c b/nvme-print.c
index b3f08cf..e578d0b 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2845,8 +2845,14 @@ void json_sanitize_log(struct nvme_sanitize_log_page *sanitize_log, const char *
 
 static void show_nvme_ctrl(struct ctrl_list_item *item)
 {
+	int i;
+
 	printf(" +- %s %s %s %s\n", item->name, item->transport, item->address,
 	       item->state);
+	printf(" \\\n");
+	for (i = 0; i < item->ns_num; i++) {
+		printf("  +- %s\n", item->namespaces[i].name);
+	}
 }
 
 
@@ -2875,7 +2881,9 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
 	struct json_object *subsystem_attrs;
 	struct json_array *paths;
 	struct json_object *path_attrs;
-	int i, j;
+	struct json_array *namespaces;
+	struct json_object *ns_attrs;
+	int i, j, k;
 
 	root = json_create_object();
 	subsystems = json_create_array();
@@ -2902,6 +2910,18 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
 					slist[i].ctrls[j].address);
 			json_object_add_value_string(path_attrs, "State",
 					slist[i].ctrls[j].state);
+
+			namespaces = json_create_array();
+			for (k = 0; k < slist[i].ctrls[j].ns_num; k++) {
+				ns_attrs = json_create_object();
+				json_object_add_value_string(ns_attrs, "Name",
+							     slist[i].ctrls[j].namespaces[k].name);
+				json_array_add_value_object(namespaces, ns_attrs);
+			}
+			if (k)
+				json_object_add_value_array(path_attrs, "Namespaces",
+							    namespaces);
+
 			json_array_add_value_object(paths, path_attrs);
 		}
 		if (j)
diff --git a/nvme.c b/nvme.c
index 0fa141b..08d0cc2 100644
--- a/nvme.c
+++ b/nvme.c
@@ -60,6 +60,9 @@ const char *devicename;
 
 static const char nvme_version_string[] = NVME_VERSION;
 
+static void free_ctrl_list_item(struct ctrl_list_item *ctrls);
+static char *get_nvme_ctrl_attr(char *path, const char *attr);
+
 #define CREATE_CMD
 #include "nvme-builtin.h"
 
@@ -1306,6 +1309,72 @@ static void *get_registers(void)
 
 static const char *subsys_dir = "/sys/class/nvme-subsystem/";
 
+static int scan_ns_filter(const struct dirent *d)
+{
+	int id, nsid;
+
+	if (strstr(d->d_name, "nvme") &&
+	    sscanf(d->d_name, "nvme%dn%d", &id, &nsid) == 2)
+		return 1;
+	return 0;
+}
+
+static int get_nvme_ctrl_info(char *name, char *ctrl_path,
+			      struct ctrl_list_item *item)
+{
+	struct dirent **namespaces;
+	int ret = 0, n, i;
+
+	item->name = strdup(name);
+
+	n = scandir(ctrl_path, &namespaces, scan_ns_filter, alphasort);
+	if (n < 0) {
+		fprintf(stderr, "failed to scan namespace(s).\n");
+		return n;
+	}
+
+	item->namespaces = calloc(n, sizeof(struct ns_list_item));
+	if (!item->namespaces) {
+		ret = -ENOMEM;
+		fprintf(stderr, "failed to allocate controller namespace(s)\n");
+		goto free_namespaces;
+	}
+
+	item->ns_num = n;
+
+	for (i = 0; i < n; i++)
+		item->namespaces[i].name = strdup(namespaces[i]->d_name);
+
+	item->address = get_nvme_ctrl_attr(ctrl_path, "address");
+	if (!item->address) {
+		fprintf(stderr, "failed to get %s address.\n", name);
+		free_ctrl_list_item(item);
+		ret = -EAGAIN;
+		goto free_namespaces;
+	}
+	item->transport = get_nvme_ctrl_attr(ctrl_path, "transport");
+	if (!item->transport) {
+		fprintf(stderr, "failed to get %s transport.\n", name);
+		free_ctrl_list_item(item);
+		ret = -EAGAIN;
+		goto free_namespaces;
+	}
+	item->state = get_nvme_ctrl_attr(ctrl_path, "state");
+	if (!item->state) {
+		fprintf(stderr, "failed to get %s state.\n", name);
+		free_ctrl_list_item(item);
+		ret = -EAGAIN;
+		goto free_namespaces;
+	}
+
+free_namespaces:
+	for (i = 0; i < n; i++)
+		free(namespaces[i]);
+	free(namespaces);
+
+	return ret;
+}
+
 static char *get_nvme_subsnqn(char *path)
 {
 	char sspath[320];
@@ -1412,10 +1481,15 @@ static int scan_ctrls_filter(const struct dirent *d)
 
 static void free_ctrl_list_item(struct ctrl_list_item *ctrls)
 {
+	int i;
+
 	free(ctrls->name);
 	free(ctrls->transport);
 	free(ctrls->address);
 	free(ctrls->state);
+	for (i = 0; i < ctrls->ns_num; i++)
+		free(ctrls->namespaces[i].name);
+	free(ctrls->namespaces);
 }
 
 static int get_nvme_subsystem_info(char *name, char *path,
@@ -1448,36 +1522,14 @@ 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;
-		}
-
-		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;
-		}
+			 ctrls[i]->d_name);
 
-		ccnt++;
+		ret = get_nvme_ctrl_info(ctrls[i]->d_name, ctrl_path,
+					 &(item->ctrls[ccnt]));
+		if (!ret)
+			ccnt++;
 	}
 
 	item->nctrls = ccnt;
diff --git a/nvme.h b/nvme.h
index 7c444a4..84eaa9f 100644
--- a/nvme.h
+++ b/nvme.h
@@ -142,11 +142,17 @@ struct list_item {
 	unsigned            block;
 };
 
+struct ns_list_item {
+	char *name;
+};
+
 struct ctrl_list_item {
 	char *name;
 	char *address;
 	char *transport;
 	char *state;
+	int ns_num;
+	struct ns_list_item *namespaces;
 };
 
 struct subsys_list_item {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-05-28 19:54 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-23  9:00 [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Max Gurtovoy
2019-05-23  9:00 ` [PATCH 1/9] nvme: update list-ns nsid option Max Gurtovoy
2019-05-23 17:27   ` Heitke, Kenneth
2019-05-24 19:17     ` Max Gurtovoy
2019-05-23  9:00 ` [PATCH 2/9] nvme: update description for "nvme list" command Max Gurtovoy
2019-05-23  9:00 ` [PATCH 3/9] fabrics: Fix memory leak of subsys list Max Gurtovoy
2019-05-23  9:00 ` [PATCH 4/9] nvme-print: fix json object memory leak Max Gurtovoy
2019-05-23  9:00 ` [PATCH 5/9] nvme: fix coding style issue Max Gurtovoy
2019-05-23  9:00 ` [PATCH 6/9] nvme: update list-subsys command to show the entire list Max Gurtovoy
2019-05-23  9:00 ` [PATCH 7/9] nvme-print: Introduce show_nvme_ctrl helper Max Gurtovoy
2019-05-23 15:12   ` Minwoo Im
2019-05-23  9:00 ` [PATCH 8/9] nvme-print: Rename "Paths" --> "Ctrls" for json output in list-subsys Max Gurtovoy
2019-05-23  9:00 ` [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd Max Gurtovoy
2019-05-23 15:11   ` Minwoo Im
2019-05-24  7:35 ` [PATCH v2 0/9] Add Subsystem/Ctrl/Namespace relations (nvme-cli) Sagi Grimberg
  -- strict thread matches above, loose matches on Subject: below --
2019-05-27 10:01 [PATCH v3 " Max Gurtovoy
2019-05-27 10:01 ` [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd Max Gurtovoy
2019-05-28 19:54   ` Sagi Grimberg
2019-05-22 16:39 [PATCH 0/9] Add Subsystem/Ctrl/Namespace relations Max Gurtovoy
2019-05-22 16:39 ` [PATCH 9/9] nvme: Retrieve namespaces during list-subsys cmd Max Gurtovoy
2019-05-22 22:49   ` Keith Busch
2019-05-23  8:01     ` Max Gurtovoy
2019-05-23 13:39       ` Keith Busch
2019-05-23 14:07         ` hch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.