[PATCH 2/2] nvme-cli: extend show-topology tabular output for multipath subsys

[Nilay Shroff <nilay@linux.ibm.com>]

The current tabular output of the show-topology command does not
display a controller if it has no associated nvme path or namespaces.
However, it is valid for a controller within a subsystem to have no
namespaces or paths attached.

In such cases, it is still useful to display controller information
such as the controller name, transport type, address, and state, while
leaving NVMe path and namespace-related fields (e.g., nsid, nshead,
nspath, anastate etc.) as "--".

Update the tabular output of the show-topology command to include
controllers and their associated fields regardless of whether any
namespaces are present. While we are at it, also dispaly the error
message using nvme_show_error() instead of using printf().

This change applies to NVMe multipath subsystems.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 nvme-print-stdout.c | 156 +++++++++++++++++++++++++++++++-------------
 1 file changed, 111 insertions(+), 45 deletions(-)

diff --git a/nvme-print-stdout.c b/nvme-print-stdout.c
index 769afa695..064956138 100644
--- a/nvme-print-stdout.c
+++ b/nvme-print-stdout.c
@@ -5945,13 +5945,69 @@ static bool subsystem_iopolicy_filter(const char *name, void *arg)
 	return true;
 }
 
+static int subsystem_topology_multipath_add_row(struct table *t,
+						const char *iopolicy,
+						const char *nshead,
+						const char *nsid,
+						const char *nspath,
+						const char *anastate,
+						const char *iopolicy_info,
+						const char *ctrl,
+						const char *trtype,
+						const char *address,
+						const char *state)
+{
+	int row;
+	int col = -1;
+
+	row = table_get_row_id(t);
+	if (row < 0) {
+		nvme_show_error("Failed to add subsys topology multipath row");
+		return row;
+	}
+
+	/* col 0: NSHead */
+	table_set_value_str(t, ++col, row, nshead, CENTERED);
+
+	/* col 1: NSID */
+	table_set_value_str(t, ++col, row, nsid, CENTERED);
+
+	/* col 2: NSPath */
+	table_set_value_str(t, ++col, row, nspath, CENTERED);
+
+	/* col 3: ANAState */
+	table_set_value_str(t, ++col, row, anastate, CENTERED);
+
+	/* col 4: Nodes/Qdepth */
+	if (!strcmp(iopolicy, "numa") || !strcmp(iopolicy, "queue-depth"))
+		table_set_value_str(t, ++col, row, iopolicy_info, CENTERED);
+
+	/* col 5: Controller */
+	table_set_value_str(t, ++col, row, ctrl, CENTERED);
+
+	/* col 6: TrType */
+	table_set_value_str(t, ++col, row, trtype, CENTERED);
+
+	/* col 7: Address */
+	table_set_value_str(t, ++col, row, address, CENTERED);
+
+	/* col 8: State */
+	table_set_value_str(t, ++col, row, state, CENTERED);
+
+	table_add_row(t, row);
+
+	return 0;
+}
+
 static void stdout_tabular_subsystem_topology_multipath(nvme_subsystem_t s)
 {
 	nvme_ns_t n;
 	nvme_path_t p;
 	nvme_ctrl_t c;
-	int row, col;
 	bool first;
+	char nshead[32], nsid[32];
+	char iopolicy_info[256];
+	int ret, num_path;
 	struct table *t;
 	const char *iopolicy = nvme_subsystem_get_iopolicy(s);
 	struct table_column columns[] = {
@@ -5969,13 +6025,13 @@ static void stdout_tabular_subsystem_topology_multipath(nvme_subsystem_t s)
 
 	t = table_create();
 	if (!t) {
-		printf("Failed to init table\n");
+		nvme_show_error("Failed to init subsys topology multipath table");
 		return;
 	}
 
 	if (table_add_columns_filter(t, columns, ARRAY_SIZE(columns),
 			subsystem_iopolicy_filter, (void *)s) < 0) {
-		printf("Failed to add columns\n");
+		nvme_show_error("Failed to add subsys topology multipath columns");
 		goto free_tbl;
 	}
 
@@ -5984,61 +6040,71 @@ static void stdout_tabular_subsystem_topology_multipath(nvme_subsystem_t s)
 		nvme_namespace_for_each_path(n, p) {
 			c = nvme_path_get_ctrl(p);
 
-			row = table_get_row_id(t);
-			if (row < 0) {
-				printf("Failed to add row\n");
-				goto free_tbl;
-			}
-			/* For the first row we print actual NSHead name,
+			/*
+			 * For the first row we print actual NSHead name,
 			 * however, for the subsequent rows we print "arrow"
 			 * ("-->") symbol for NSHead. This "arrow" style makes
 			 * it visually obvious that susequenet entries (if
 			 * present) are a path under the first NSHead.
 			 */
-			col = -1;
-			/* col 0: NSHead */
 			if (first) {
-				table_set_value_str(t, ++col, row,
-						nvme_ns_get_name(n), LEFT);
+				snprintf(nshead, sizeof(nshead), "%s",
+						nvme_ns_get_name(n));
 				first = false;
 			} else
-				table_set_value_str(t, ++col, row,
-						"-->", CENTERED);
-			/* col 1: NSID */
-			table_set_value_int(t, ++col, row,
-					nvme_ns_get_nsid(n), CENTERED);
-			/* col 2: NSPath */
-			table_set_value_str(t, ++col, row,
-					nvme_path_get_name(p), LEFT);
-			/* col 3: ANAState */
-			table_set_value_str(t, ++col, row,
-					nvme_path_get_ana_state(p), LEFT);
+				snprintf(nshead, sizeof(nshead), "%s", "-->");
+
+			snprintf(nsid, sizeof(nsid), "%u", nvme_ns_get_nsid(n));
 
 			if (!strcmp(iopolicy, "numa"))
-				/* col 4: Nodes */
-				table_set_value_str(t, ++col, row,
-					nvme_path_get_numa_nodes(p), CENTERED);
+				snprintf(iopolicy_info, sizeof(iopolicy_info),
+					"%s", nvme_path_get_numa_nodes(p));
 			else if (!strcmp(iopolicy, "queue-depth"))
-				/* col 4 : Qdepth */
-				table_set_value_int(t, ++col, row,
-					nvme_path_get_queue_depth(p), CENTERED);
-
-			/* col 5: Controller */
-			table_set_value_str(t, ++col, row,
-					nvme_ctrl_get_name(c), LEFT);
-			/* col 6: TrType */
-			table_set_value_str(t, ++col, row,
-					nvme_ctrl_get_transport(c), LEFT);
-			/* col 7: Address */
-			table_set_value_str(t, ++col, row,
-					nvme_ctrl_get_traddr(c), LEFT);
-			/* col 8: State */
-			table_set_value_str(t, ++col, row,
-					nvme_ctrl_get_state(c), LEFT);
-
-			table_add_row(t, row);
+				snprintf(iopolicy_info, sizeof(iopolicy_info),
+					"%d", nvme_path_get_queue_depth(p));
+
+			ret = subsystem_topology_multipath_add_row(t,
+						    iopolicy,
+						    nshead,
+						    nsid,
+						    nvme_path_get_name(p),
+						    nvme_path_get_ana_state(p),
+						    iopolicy_info,
+						    nvme_ctrl_get_name(c),
+						    nvme_ctrl_get_transport(c),
+						    nvme_ctrl_get_traddr(c),
+						    nvme_ctrl_get_state(c));
+			if (ret < 0)
+				goto free_tbl;
 		}
 	}
+
+	/*
+	 * Next we print controller in the subsystem which may not have any
+	 * nvme path associated to it.
+	 */
+	nvme_subsystem_for_each_ctrl(s, c) {
+		num_path = 0;
+		nvme_ctrl_for_each_path(c, p)
+			num_path++;
+
+		if (!num_path) {
+			ret = subsystem_topology_multipath_add_row(t,
+					iopolicy,
+					"--", 	/* NSHead */
+					"--", 	/* NSID */
+					"--", 	/* NSPath */
+					"--", 	/* ANAState */
+					"--", 	/* Nodes/Qdepth */
+					nvme_ctrl_get_name(c),
+					nvme_ctrl_get_transport(c),
+					nvme_ctrl_get_traddr(c),
+					nvme_ctrl_get_state(c));
+			if (ret < 0)
+				goto free_tbl;
+		}
+	}
+
 	table_print(t);
 free_tbl:
 	table_free(t);
-- 
2.53.0