linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] nvme-topology: have list-subsys print only controllers with attached namespace
@ 2021-03-15 20:52 Sagi Grimberg
  2021-03-15 21:13 ` Chaitanya Kulkarni
  0 siblings, 1 reply; 3+ messages in thread
From: Sagi Grimberg @ 2021-03-15 20:52 UTC (permalink / raw)
  To: linux-nvme, Keith Busch

When running list-subsys on a specific namespace, we output all the
controllers that belong to the subsystem regardless if the requested
namespace is actually attached to them.

Example:
$ nvme list-subsys /dev/nvme0n1
nvme-subsys0 - NQN=nqn.2016-01.com.lightbitslabs:uuid:07cfffe6-5a4f-4151-b663-e13cf835609b
\
 +- nvme0 tcp traddr=10.113.5.1 trsvcid=4420 live optimized
 +- nvme1 tcp traddr=10.133.3.1 trsvcid=4420 live
 +- nvme2 tcp traddr=10.133.1.1 trsvcid=4420 live
 +- nvme3 tcp traddr=10.113.1.1 trsvcid=4420 live inaccessible
 +- nvme4 tcp traddr=10.133.5.1 trsvcid=4420 live
 +- nvme5 tcp traddr=10.113.4.1 trsvcid=4420 live
 +- nvme6 tcp traddr=10.133.4.1 trsvcid=4420 live
 +- nvme7 tcp traddr=10.113.2.1 trsvcid=4420 live
 +- nvme8 tcp traddr=10.133.2.1 trsvcid=4420 live
 +- nvme9 tcp traddr=10.113.3.1 trsvcid=4420 live

This output is somewhat confusing and makes the user think the namespace
is actually attached to all of these controllers, instead we want the
output which provided by the change introduced here:

$ nvme list-subsys /dev/nvme0n1
nvme-subsys0 - NQN=nqn.2016-01.com.lightbitslabs:uuid:07cfffe6-5a4f-4151-b663-e13cf835609b
\
 +- nvme0 tcp traddr=10.113.5.1 trsvcid=4420 live optimized
 +- nvme3 tcp traddr=10.113.1.1 trsvcid=4420 live inaccessible

Do the same trick as we do in scan_subsystems, we add the ctrl to the
topology if either ns_instance wasn't passed (see all controllers) or
it was pased _and_ the controller has this namespace is attached to
the controller (to do that we add nsid down the call chain for that).

Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
 fabrics.c       |  2 +-
 nvme-print.c    |  2 +-
 nvme-topology.c | 30 +++++++++++++++++++++++++-----
 nvme.c          | 24 ++++++++++++++++++++----
 nvme.h          |  2 +-
 5 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/fabrics.c b/fabrics.c
index f84c45b5f724..ff34c3039faf 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -1777,7 +1777,7 @@ int fabrics_disconnect_all(const char *desc, int argc, char **argv)
 	if (err)
 		goto out;
 
-	err = scan_subsystems(&t, NULL, 0, NULL);
+	err = scan_subsystems(&t, NULL, 0, 0, NULL);
 	if (err) {
 		fprintf(stderr, "Failed to scan namespaces\n");
 		goto out;
diff --git a/nvme-print.c b/nvme-print.c
index b2a55af699ff..f66c3808a909 100755
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2514,7 +2514,7 @@ void nvme_show_relatives(const char *name)
 			free(path);
 			return;
 		}
-		err = scan_subsystems(&t, subsysnqn, 0, NULL);
+		err = scan_subsystems(&t, subsysnqn, 0, 0, NULL);
 		if (err || t.nr_subsystems != 1) {
 			free(subsysnqn);
 			free(path);
diff --git a/nvme-topology.c b/nvme-topology.c
index c0e2d5100b99..63e433d70f01 100644
--- a/nvme-topology.c
+++ b/nvme-topology.c
@@ -11,6 +11,7 @@
 
 static const char *dev = "/dev/";
 static const char *subsys_dir = "/sys/class/nvme-subsystem/";
+static void free_ctrl(struct nvme_ctrl *c);
 
 char *get_nvme_subsnqn(char *path)
 {
@@ -230,6 +231,19 @@ static char *get_nvme_ctrl_path_ana_state(char *path, int nsid)
 	return ana_state;
 }
 
+static bool ns_attached_to_ctrl(int nsid, struct nvme_ctrl *ctrl)
+{
+	struct nvme_namespace *n;
+	int i;
+
+	for (i = 0; i < ctrl->nr_namespaces; i++) {
+		n = &ctrl->namespaces[i];
+		if (nsid == n->nsid)
+			return true;
+	}
+	return false;
+}
+
 static int scan_ctrl(struct nvme_ctrl *c, char *p, __u32 ns_instance)
 {
 	struct nvme_namespace *n;
@@ -315,12 +329,12 @@ free:
 	return 0;
 }
 
-static int scan_subsystem(struct nvme_subsystem *s, __u32 ns_instance)
+static int scan_subsystem(struct nvme_subsystem *s, __u32 ns_instance, int nsid)
 {
 	struct dirent **ctrls, **ns;
 	struct nvme_namespace *n;
 	struct nvme_ctrl *c;
-	int i, ret;
+	int i, j = 0, ret;
 	char *path;
 
 	ret = asprintf(&path, "%s%s", subsys_dir, s->name);
@@ -336,12 +350,18 @@ static int scan_subsystem(struct nvme_subsystem *s, __u32 ns_instance)
 	s->nr_ctrls = ret;
 	s->ctrls = calloc(s->nr_ctrls, sizeof(*c));
 	for (i = 0; i < s->nr_ctrls; i++) {
-		c = &s->ctrls[i];
+		c = &s->ctrls[j];
 		c->name = strdup(ctrls[i]->d_name);
 		c->path = strdup(dev);
 		c->subsys = s;
 		scan_ctrl(c, path, ns_instance);
+
+		if (!ns_instance || ns_attached_to_ctrl(nsid, c))
+			j++;
+		else
+			free_ctrl(c);
 	}
+	s->nr_ctrls = j;
 
 	while (i--)
 		free(ctrls[i]);
@@ -550,7 +570,7 @@ static int scan_subsystem_dir(struct nvme_topology *t, char *dev_dir)
 }
 
 int scan_subsystems(struct nvme_topology *t, const char *subsysnqn,
-		    __u32 ns_instance, char *dev_dir)
+		    __u32 ns_instance, int nsid, char *dev_dir)
 {
 	struct nvme_subsystem *s;
 	struct dirent **subsys;
@@ -568,7 +588,7 @@ int scan_subsystems(struct nvme_topology *t, const char *subsysnqn,
 		for (i = 0; i < t->nr_subsystems; i++) {
 			s = &t->subsystems[j];
 			s->name = strdup(subsys[i]->d_name);
-			scan_subsystem(s, ns_instance);
+			scan_subsystem(s, ns_instance, nsid);
 
 			if (!subsysnqn || !strcmp(s->subsysnqn, subsysnqn))
 				j++;
diff --git a/nvme.c b/nvme.c
index 7ede27509bf7..2a3d0ac5da49 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1685,7 +1685,7 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 	const char *desc = "Retrieve information for subsystems";
 	const char *verbose = "Increase output verbosity";
 	__u32 ns_instance = 0;
-	int err;
+	int err, nsid = 0;
 
 	struct config {
 		char *output_format;
@@ -1710,7 +1710,7 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 	devicename = NULL;
 	if (optind < argc) {
 		char path[512];
-		int id;
+		int id, fd;
 
 		devicename = basename(argv[optind]);
 		if (sscanf(devicename, "nvme%dn%d", &id, &ns_instance) != 2) {
@@ -1719,6 +1719,22 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 			err = -EINVAL;
 			goto ret;
 		}
+		sprintf(path, "/dev/%s", devicename);
+		fd = open(path, O_RDONLY);
+		if (fd < 0) {
+			fprintf(stderr, "Cannot read nsid from %s\n",
+				devicename);
+			err = -EINVAL;
+			goto ret;
+		}
+		nsid = nvme_get_nsid(fd);
+		close(fd);
+		if (nsid < 0) {
+			fprintf(stderr, "Cannot read nsid from %s\n",
+				devicename);
+			err = -EINVAL;
+			goto ret;
+		}
 		sprintf(path, "/sys/block/%s/device", devicename);
 		subsysnqn = get_nvme_subsnqn(path);
 		if (!subsysnqn) {
@@ -1740,7 +1756,7 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 	if (cfg.verbose)
 		flags |= VERBOSE;
 
-	err = scan_subsystems(&t, subsysnqn, ns_instance, NULL);
+	err = scan_subsystems(&t, subsysnqn, ns_instance, nsid, NULL);
 	if (err) {
 		fprintf(stderr, "Failed to scan namespaces\n");
 		goto free;
@@ -1796,7 +1812,7 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
 	if (cfg.verbose)
 		flags |= VERBOSE;
 
-	err = scan_subsystems(&t, NULL, 0, cfg.device_dir);
+	err = scan_subsystems(&t, NULL, 0, 0, cfg.device_dir);
 	if (err) {
 		fprintf(stderr, "Failed to scan namespaces\n");
 		return err;
diff --git a/nvme.h b/nvme.h
index 3fb1060760ee..8501754c6502 100644
--- a/nvme.h
+++ b/nvme.h
@@ -106,7 +106,7 @@ int scan_subsys_filter(const struct dirent *d);
 int scan_dev_filter(const struct dirent *d);
 
 int scan_subsystems(struct nvme_topology *t, const char *subsysnqn,
-		    __u32 ns_instance, char *dev_dir);
+		    __u32 ns_instance, int nsid, char *dev_dir);
 void free_topology(struct nvme_topology *t);
 char *get_nvme_subsnqn(char *path);
 char *nvme_get_ctrl_attr(char *path, const char *attr);
-- 
2.27.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: [PATCH] nvme-topology: have list-subsys print only controllers with attached namespace
  2021-03-15 20:52 [PATCH] nvme-topology: have list-subsys print only controllers with attached namespace Sagi Grimberg
@ 2021-03-15 21:13 ` Chaitanya Kulkarni
  2021-03-15 21:40   ` Keith Busch
  0 siblings, 1 reply; 3+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-15 21:13 UTC (permalink / raw)
  To: Sagi Grimberg; +Cc: linux-nvme, Keith Busch

On 3/15/21 14:01, Sagi Grimberg wrote:
> When running list-subsys on a specific namespace, we output all the
> controllers that belong to the subsystem regardless if the requested
> namespace is actually attached to them.
>
> Example:
> $ nvme list-subsys /dev/nvme0n1
> nvme-subsys0 - NQN=nqn.2016-01.com.lightbitslabs:uuid:07cfffe6-5a4f-4151-b663-e13cf835609b
> \
>  +- nvme0 tcp traddr=10.113.5.1 trsvcid=4420 live optimized
>  +- nvme1 tcp traddr=10.133.3.1 trsvcid=4420 live
>  +- nvme2 tcp traddr=10.133.1.1 trsvcid=4420 live
>  +- nvme3 tcp traddr=10.113.1.1 trsvcid=4420 live inaccessible
>  +- nvme4 tcp traddr=10.133.5.1 trsvcid=4420 live
>  +- nvme5 tcp traddr=10.113.4.1 trsvcid=4420 live
>  +- nvme6 tcp traddr=10.133.4.1 trsvcid=4420 live
>  +- nvme7 tcp traddr=10.113.2.1 trsvcid=4420 live
>  +- nvme8 tcp traddr=10.133.2.1 trsvcid=4420 live
>  +- nvme9 tcp traddr=10.113.3.1 trsvcid=4420 live
>
> This output is somewhat confusing and makes the user think the namespace
> is actually attached to all of these controllers, instead we want the
> output which provided by the change introduced here:
>
> $ nvme list-subsys /dev/nvme0n1
> nvme-subsys0 - NQN=nqn.2016-01.com.lightbitslabs:uuid:07cfffe6-5a4f-4151-b663-e13cf835609b
> \
>  +- nvme0 tcp traddr=10.113.5.1 trsvcid=4420 live optimized
>  +- nvme3 tcp traddr=10.113.1.1 trsvcid=4420 live inaccessible
>
> Do the same trick as we do in scan_subsystems, we add the ctrl to the
> topology if either ns_instance wasn't passed (see all controllers) or
> it was pased _and_ the controller has this namespace is attached to
> the controller (to do that we add nsid down the call chain for that).
>
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>

This is much clear than what we have currently, looks good.

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






_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: [PATCH] nvme-topology: have list-subsys print only controllers with attached namespace
  2021-03-15 21:13 ` Chaitanya Kulkarni
@ 2021-03-15 21:40   ` Keith Busch
  0 siblings, 0 replies; 3+ messages in thread
From: Keith Busch @ 2021-03-15 21:40 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: Sagi Grimberg, linux-nvme

Thank you, patch applied.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

end of thread, other threads:[~2021-03-15 21:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 20:52 [PATCH] nvme-topology: have list-subsys print only controllers with attached namespace Sagi Grimberg
2021-03-15 21:13 ` Chaitanya Kulkarni
2021-03-15 21:40   ` Keith Busch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).