[PATCH nvme-cli 0/3] Print out controller and ANA state for list-subsys

[PATCH nvme-cli 0/3] Print out controller and ANA state for list-subsys

[Hannes Reinecke]

Hi all,

this patchset implements the ability to check the path / controller state
on each namespace. It does so by enhancing the 'list-subsys' command to
optionally accept a device name for the namespace to check; with that we
are able to print out the controller and the ANA state per controller leading
up to that namespace.

As usual, comments and reviews are welcome.

Hannes Reinecke (3):
  nvme: introduce get_nvme_ctrl_attr()
  nvme: print out controller state for 'list-subsys'
  nvme-list-subsys: Add device name argument and print out ANA state

 Documentation/nvme-list-subsys.txt |   4 +-
 fabrics.c                          |   2 +-
 nvme-print.c                       |  13 ++-
 nvme.c                             | 226 ++++++++++++++++++++++++++-----------
 nvme.h                             |   4 +-
 5 files changed, 175 insertions(+), 74 deletions(-)

-- 
2.13.7

[PATCH nvme-cli 1/3] nvme: introduce get_nvme_ctrl_attr()

[Hannes Reinecke]

Use a generic function 'get_nvme_ctrl_attr()' instead of coding
the same function for each sysfs attribute.

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 nvme.c | 98 ++++++++++++++++++------------------------------------------------
 1 file changed, 27 insertions(+), 71 deletions(-)

diff --git a/nvme.c b/nvme.c
index 1df3a10..f15d372 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1290,103 +1290,58 @@ close_fd:
 	return subsysnqn;
 }
 
-static char *get_nvme_ctrl_transport(char *path)
+static char *get_nvme_ctrl_attr(char *path, const char *attr)
 {
-	char *trpath;
-	char *transport;
-	int fd;
-	ssize_t ret;
-
-	ret = asprintf(&trpath, "%s/transport", path);
-	if (ret < 0)
-		return NULL;
-
-	transport = calloc(1, 1024);
-	if (!transport)
-		goto err_free_trpath;
-
-	fd = open(trpath, O_RDONLY);
-	if (fd < 0) {
-		fprintf(stderr, "Failed to open %s: %s\n",
-				trpath, strerror(errno));
-		goto err_free_tr;
-	}
-
-	ret = read(fd, transport, 1024);
-	if (ret < 0) {
-		fprintf(stderr, "read :%s :%s\n", trpath, strerror(errno));
-		goto err_close_fd;
-	}
-
-	if (transport[strlen(transport) - 1] == '\n')
-		transport[strlen(transport) - 1] = '\0';
-
-	close(fd);
-	free(trpath);
-
-	return transport;
-
-err_close_fd:
-	close(fd);
-err_free_tr:
-	free(transport);
-err_free_trpath:
-	free(trpath);
-
-	return NULL;
-}
-
-static char *get_nvme_ctrl_address(char *path)
-{
-	char *addrpath;
-	char *address;
+	char *attrpath;
+	char *value;
 	int fd;
 	ssize_t ret;
 	int i;
 
-	ret = asprintf(&addrpath, "%s/address", path);
+	ret = asprintf(&attrpath, "%s/%s", path, attr);
 	if (ret < 0)
 		return NULL;
 
-	address = calloc(1, 1024);
-	if (!address)
-		goto err_free_addrpath;
+	value = calloc(1, 1024);
+	if (!value)
+		goto err_free_path;
 
-	fd = open(addrpath, O_RDONLY);
+	fd = open(attrpath, O_RDONLY);
 	if (fd < 0) {
 		fprintf(stderr, "Failed to open %s: %s\n",
-				addrpath, strerror(errno));
-		goto err_free_addr;
+				attrpath, strerror(errno));
+		goto err_free_value;
 	}
 
-	ret = read(fd, address, 1024);
+	ret = read(fd, value, 1024);
 	if (ret < 0) {
-		fprintf(stderr, "read :%s :%s\n", addrpath, strerror(errno));
+		fprintf(stderr, "read :%s :%s\n", attrpath, strerror(errno));
 		goto err_close_fd;
 	}
 
-	if (address[strlen(address) - 1] == '\n')
-		address[strlen(address) - 1] = '\0';
+	if (value[strlen(value) - 1] == '\n')
+		value[strlen(value) - 1] = '\0';
 
-	for (i = 0; i < strlen(address); i++) {
-		if (address[i] == ',' )
-			address[i] = ' ';
+	for (i = 0; i < strlen(value); i++) {
+		if (value[i] == ',' )
+			value[i] = ' ';
 	}
 
 	close(fd);
-	free(addrpath);
+	free(attrpath);
 
-	return address;
+	return value;
 
 err_close_fd:
 	close(fd);
-err_free_addr:
-	free(address);
-err_free_addrpath:
-	free(addrpath);
+err_free_value:
+	free(value);
+err_free_path:
+	free(attrpath);
 
 	return NULL;
 }
+
 static int scan_ctrls_filter(const struct dirent *d)
 {
 	int id, nsid;
@@ -1445,7 +1400,8 @@ static int get_nvme_subsystem_info(char *name, char *path,
 		snprintf(ctrl_path, sizeof(ctrl_path), "%s/%s", path,
 			 item->ctrls[ccnt].name);
 
-		item->ctrls[ccnt].address = get_nvme_ctrl_address(ctrl_path);
+		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]);
@@ -1453,7 +1409,7 @@ static int get_nvme_subsystem_info(char *name, char *path,
 		}
 
 		item->ctrls[ccnt].transport =
-				get_nvme_ctrl_transport(ctrl_path);
+				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]);
-- 
2.13.7

[PATCH nvme-cli 2/3] nvme: print out controller state for 'list-subsys'

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 nvme-print.c | 7 +++++--
 nvme.c       | 9 +++++++++
 nvme.h       | 1 +
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/nvme-print.c b/nvme-print.c
index b728975..5cdfabe 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2772,9 +2772,10 @@ static void show_nvme_subsystem(struct subsys_list_item *item)
 	printf("\\\n");
 
 	for (i = 0; i < item->nctrls; i++) {
-		printf(" +- %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].address,
+				item->ctrls[i].state);
 	}
 
 }
@@ -2820,6 +2821,8 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
 					slist[i].ctrls[j].transport);
 			json_object_add_value_string(path_attrs, "Address",
 					slist[i].ctrls[j].address);
+			json_object_add_value_string(path_attrs, "State",
+					slist[i].ctrls[j].state);
 			json_array_add_value_object(paths, path_attrs);
 		}
 		if (j) {
diff --git a/nvme.c b/nvme.c
index f15d372..c3295c5 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1363,6 +1363,7 @@ static void free_ctrl_list_item(struct ctrl_list_item *ctrls)
 	free(ctrls->name);
 	free(ctrls->transport);
 	free(ctrls->address);
+	free(ctrls->state);
 }
 
 static int get_nvme_subsystem_info(char *name, char *path,
@@ -1416,6 +1417,14 @@ static int get_nvme_subsystem_info(char *name, char *path,
 			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++;
 	}
 
diff --git a/nvme.h b/nvme.h
index 32ac96d..668c6fd 100644
--- a/nvme.h
+++ b/nvme.h
@@ -129,6 +129,7 @@ struct ctrl_list_item {
 	char *name;
 	char *address;
 	char *transport;
+	char *state;
 };
 
 struct subsys_list_item {
-- 
2.13.7

[PATCH nvme-cli 3/3] nvme-list-subsys: Add device name argument and print out ANA state

[Hannes Reinecke]

Update the 'nvme list-subsys' command to accept a device name and
print out the ANA state for all paths to that device.

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 Documentation/nvme-list-subsys.txt |   4 +-
 fabrics.c                          |   2 +-
 nvme-print.c                       |  10 ++-
 nvme.c                             | 135 +++++++++++++++++++++++++++++++++++--
 nvme.h                             |   3 +-
 5 files changed, 143 insertions(+), 11 deletions(-)

diff --git a/Documentation/nvme-list-subsys.txt b/Documentation/nvme-list-subsys.txt
index c7de7ef..c40b708 100644
--- a/Documentation/nvme-list-subsys.txt
+++ b/Documentation/nvme-list-subsys.txt
@@ -8,12 +8,14 @@ nvme-list-subsys - List all NVMe subsystems
 SYNOPSIS
 --------
 [verse]
-'nvme list-subsys' [-o <fmt> | --output-format=<fmt>]
+'nvme list-subsys' [-o <fmt> | --output-format=<fmt>] <device>
 
 DESCRIPTION
 -----------
 Scan the sysfs tree for NVM Express subsystems and return the controllers
 for those subsystems as well as some pertinent information about them.
+If a device is given, print out only the values for the controllers
+and subsystems leading to the device.
 
 OPTIONS
 -------
diff --git a/fabrics.c b/fabrics.c
index 711a755..f2bbd92 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -1102,7 +1102,7 @@ int disconnect_all(const char *desc, int argc, char **argv)
 	if (ret)
 		return ret;
 
-	slist = get_subsys_list(&subcnt);
+	slist = get_subsys_list(&subcnt, NULL, NVME_NSID_ALL);
 	for (i = 0; i < subcnt; i++) {
 		struct subsys_list_item *subsys = &slist[i];
 
diff --git a/nvme-print.c b/nvme-print.c
index 5cdfabe..ee3ceb3 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2772,10 +2772,12 @@ static void show_nvme_subsystem(struct subsys_list_item *item)
 	printf("\\\n");
 
 	for (i = 0; i < item->nctrls; i++) {
-		printf(" +- %s %s %s %s\n", item->ctrls[i].name,
+		printf(" +- %s %s %s %s %s\n", item->ctrls[i].name,
 				item->ctrls[i].transport,
 				item->ctrls[i].address,
-				item->ctrls[i].state);
+				item->ctrls[i].state,
+				item->ctrls[i].ana_state ?
+					item->ctrls[i].ana_state : "");
 	}
 
 }
@@ -2823,6 +2825,10 @@ 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 c3295c5..0e53472 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1342,6 +1342,85 @@ 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;
@@ -1364,10 +1443,11 @@ 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)
+				struct subsys_list_item *item, __u32 nsid)
 {
 	char ctrl_path[512];
 	struct dirent **ctrls;
@@ -1425,6 +1505,9 @@ 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++;
 	}
 
@@ -1489,7 +1572,8 @@ void free_subsys_list(struct subsys_list_item *slist, int n)
 	free(slist);
 }
 
-struct subsys_list_item *get_subsys_list(int *subcnt)
+struct subsys_list_item *get_subsys_list(int *subcnt, char *subsysnqn,
+					 __u32 nsid)
 {
 	char path[310];
 	struct dirent **subsys;
@@ -1510,13 +1594,16 @@ struct subsys_list_item *get_subsys_list(int *subcnt)
 		snprintf(path, sizeof(path), "%s%s", subsys_dir,
 			subsys[i]->d_name);
 		ret = get_nvme_subsystem_info(subsys[i]->d_name, path,
-				&slist[*subcnt]);
+				&slist[*subcnt], nsid);
 		if (ret) {
 			fprintf(stderr,
 				"%s: failed to get subsystem info: %s\n",
 				path, strerror(errno));
 			free_subsys_list_item(&slist[*subcnt]);
-		} else
+		} else if (subsysnqn &&
+			   strncmp(slist[*subcnt].subsysnqn, subsysnqn, 255))
+			free_subsys_list_item(&slist[*subcnt]);
+		else
 			(*subcnt)++;
 	}
 
@@ -1533,12 +1620,15 @@ 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";
 	struct config {
+		__u32 namespace_id;
 		char *output_format;
 	};
 
 	struct config cfg = {
+		.namespace_id  = NVME_NSID_ALL,
 		.output_format = "normal",
 	};
 
@@ -1552,11 +1642,42 @@ 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 (fmt != JSON && fmt != NORMAL) {
+		if (subsysnqn)
+			free(subsysnqn);
 		return -EINVAL;
+	}
 
-	slist = get_subsys_list(&subcnt);
+	slist = get_subsys_list(&subcnt, subsysnqn, cfg.namespace_id);
 
 	if (fmt == JSON)
 		json_print_nvme_subsystem_list(slist, subcnt);
@@ -1564,6 +1685,8 @@ 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 668c6fd..685d179 100644
--- a/nvme.h
+++ b/nvme.h
@@ -130,6 +130,7 @@ struct ctrl_list_item {
 	char *address;
 	char *transport;
 	char *state;
+	char *ana_state;
 };
 
 struct subsys_list_item {
@@ -156,7 +157,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);
+struct subsys_list_item *get_subsys_list(int *subcnt, char *subsysnqn, __u32 nsid);
 void free_subsys_list(struct subsys_list_item *slist, int n);
 char *nvme_char_from_block(char *block);
 #endif /* _NVME_H */
-- 
2.13.7

[PATCH nvme-cli 1/3] nvme: introduce get_nvme_ctrl_attr()

[Keith Busch]

Looks good, I've applied all three. I also fixed some unrelated
documentation format for the list-subsys manpage and regenerated all
the documentation.