Allow listing namespaces for all subsystems.

Fixes #943

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>

Author: gbregman

.github/workflows/build-container.yml

@@ -322,6 +322,7 @@ jobs:
               cephnvmf listener list --subsystem $sub
               cephnvmf host list --subsystem $sub
           done
+          cephnvmf namespace list
 
       - name: Run bdevperf
         run: |

control/cli.py

@@ -1898,6 +1898,9 @@ def ns_list(self, args):
         if args.nsid is not None and args.nsid <= 0:
             self.cli.parser.error("nsid value must be positive")
 
+        if not args.subsystem:
+            args.subsystem = "*"
+
         try:
             namespaces_info = self.stub.list_namespaces(pb2.list_namespaces_req(
                 subsystem=args.subsystem,
@@ -1909,12 +1912,32 @@ def ns_list(self, args):
 
         if args.format == "text" or args.format == "plain":
             if namespaces_info.status == 0:
-                if args.nsid and len(namespaces_info.namespaces) > 1:
+                if args.nsid and len(namespaces_info.namespaces) > 1 and args.subsystem != "*":
                     err_func(f"Got more than one namespace for NSID {args.nsid}")
-                if args.uuid and len(namespaces_info.namespaces) > 1:
+                if args.uuid and len(namespaces_info.namespaces) > 1 and args.subsystem != "*":
                     err_func(f"Got more than one namespace for UUID {args.uuid}")
+                if args.subsystem != "*":
+                    if namespaces_info.subsystem_nqn != args.subsystem:
+                        err_func(f"Got namespaces in subsystem "
+                                 f"{namespaces_info.subsystem_nqn} which is different than the "
+                                 f"requested one {args.subsystem}")
+                        return errno.ENODEV
                 namespaces_list = []
                 for ns in namespaces_info.namespaces:
+                    if args.subsystem == "*":
+                        if not ns.subsystem_nqn:
+                            err_func(f"Got namespace with ID {ns.nsid} on an unknown subsystem")
+                            subsys_nqn = "<n/a>"
+                        else:
+                            subsys_nqn = ns.subsystem_nqn
+                    else:
+                        if ns.subsystem_nqn and ns.subsystem_nqn != args.subsystem:
+                            err_func(f"Got a namespace with ID {ns.nsid} in subsystem "
+                                     f"{ns.subsystem_nqn} which is different than the "
+                                     f"requested one {args.subsystem}")
+                            return errno.ENODEV
+                        subsys_nqn = namespaces_info.subsystem_nqn
+
                     if args.nsid and args.nsid != ns.nsid:
                         err_func(f"Failure listing namespace {args.nsid}: "
                                  f"Got namespace {ns.nsid} instead")
@@ -1937,7 +1960,8 @@ def ns_list(self, args):
                         else:
                             visibility = "Restrictive"
 
-                    namespaces_list.append([ns.nsid,
+                    namespaces_list.append([subsys_nqn,
+                                            ns.nsid,
                                             break_string(ns.bdev_name, "-", 2),
                                             f"{ns.rbd_pool_name}/{ns.rbd_image_name}",
                                             self.format_size(ns.rbd_image_size),
@@ -1956,7 +1980,8 @@ def ns_list(self, args):
                     else:
                         table_format = "plain"
                     namespaces_out = tabulate(namespaces_list,
-                                              headers=["NSID",
+                                              headers=["NQN",
+                                                       "NSID",
                                                        "Bdev\nName",
                                                        "RBD\nImage",
                                                        "Image\nSize",
@@ -1975,15 +2000,27 @@ def ns_list(self, args):
                         prefix = f"Namespace with UUID {args.uuid} in"
                     else:
                         prefix = "Namespaces in"
-                    out_func(f"{prefix} subsystem {args.subsystem}:\n{namespaces_out}")
+                    if args.subsystem == "*":
+                        out_func(f"{prefix} all subsystems:\n{namespaces_out}")
+                    else:
+                        out_func(f"{prefix} subsystem {args.subsystem}:\n{namespaces_out}")
                 else:
                     if args.nsid:
-                        out_func(f"No namespace {args.nsid} in subsystem {args.subsystem}")
+                        if args.subsystem == "*":
+                            out_func(f"No namespace {args.nsid} in any subsystem")
+                        else:
+                            out_func(f"No namespace {args.nsid} in subsystem {args.subsystem}")
                     elif args.uuid:
-                        out_func(f"No namespace with UUID {args.uuid} in subsystem "
-                                 f"{args.subsystem}")
+                        if args.subsystem == "*":
+                            out_func(f"No namespace with UUID {args.uuid} in any subsystem")
+                        else:
+                            out_func(f"No namespace with UUID {args.uuid} in subsystem "
+                                     f"{args.subsystem}")
                     else:
-                        out_func(f"No namespaces in subsystem {args.subsystem}")
+                        if args.subsystem == "*":
+                            out_func("No namespaces in any subsystem")
+                        else:
+                            out_func(f"No namespaces in subsystem {args.subsystem}")
             else:
                 err_func(f"{namespaces_info.error_message}")
         elif args.format == "json" or args.format == "yaml":
@@ -2433,7 +2470,11 @@ def ns_change_visibility(self, args):
                  help="Size in bytes or specified unit (K, KB, M, MB, G, GB, T, TB, P, PB)",
                  required=True),
     ]
-    ns_list_args_list = ns_common_args + [
+    ns_list_args_list = [
+        argument("--subsystem",
+                 "-n",
+                 help="Subsystem NQN",
+                 required=False),
         argument("--nsid",
                  help="Namespace ID",
                  type=int),

control/grpc.py

@@ -2290,14 +2290,14 @@ def list_namespaces(self, request, context=None):
                          f"context: {context}{peer_msg}")
 
         if not request.subsystem:
-            errmsg = "Failure listing namespaces, missing subsystem NQN"
-            self.logger.error(errmsg)
-            return pb2.namespaces_info(status=errno.EINVAL, error_message=errmsg,
-                                       subsystem_nqn=request.subsystem, namespaces=[])
+            request.subsystem = "*"
 
         with self.rpc_lock:
             try:
-                ret = rpc_nvmf.nvmf_get_subsystems(self.spdk_rpc_client, nqn=request.subsystem)
+                if request.subsystem == "*":
+                    ret = rpc_nvmf.nvmf_get_subsystems(self.spdk_rpc_client)
+                else:
+                    ret = rpc_nvmf.nvmf_get_subsystems(self.spdk_rpc_client, nqn=request.subsystem)
                 self.logger.debug(f"list_namespaces: {ret}")
             except Exception as ex:
                 errmsg = "Failure listing namespaces"
@@ -2314,8 +2314,9 @@ def list_namespaces(self, request, context=None):
         namespaces = []
         for s in ret:
             try:
-                if s["nqn"] != request.subsystem:
-                    self.logger.warning(f'Got subsystem {s["nqn"]} instead of '
+                subsys_nqn = s["nqn"]
+                if request.subsystem != "*" and subsys_nqn != request.subsystem:
+                    self.logger.warning(f'Got subsystem {subsys_nqn} instead of '
                                         f'{request.subsystem}, ignore')
                     continue
                 try:
@@ -2324,7 +2325,7 @@ def list_namespaces(self, request, context=None):
                     ns_list = []
                     pass
                 if not ns_list:
-                    self.subsystem_nsid_bdev_and_uuid.remove_namespace(request.subsystem)
+                    self.subsystem_nsid_bdev_and_uuid.remove_namespace(subsys_nqn)
                 for n in ns_list:
                     nsid = n["nsid"]
                     bdev_name = n["bdev_name"]
@@ -2341,18 +2342,19 @@ def list_namespaces(self, request, context=None):
                         lb_group = n["anagrpid"]
                     except KeyError:
                         pass
-                    find_ret = self.subsystem_nsid_bdev_and_uuid.find_namespace(request.subsystem,
+                    find_ret = self.subsystem_nsid_bdev_and_uuid.find_namespace(subsys_nqn,
                                                                                 nsid)
                     if find_ret.empty():
                         self.logger.warning(f"Can't find info of namesapce {nsid} in "
-                                            f"{request.subsystem}. Visibility status "
+                                            f"{subsys_nqn}. Visibility status "
                                             f"will be inaccurate")
                     one_ns = pb2.namespace_cli(nsid=nsid,
                                                bdev_name=bdev_name,
                                                uuid=n["uuid"],
                                                load_balancing_group=lb_group,
                                                auto_visible=find_ret.auto_visible,
-                                               hosts=find_ret.host_list)
+                                               hosts=find_ret.host_list,
+                                               subsystem_nqn=subsys_nqn)
                     with self.rpc_lock:
                         ns_bdev = self.get_bdev_info(bdev_name)
                     if ns_bdev is None:
@@ -2379,7 +2381,6 @@ def list_namespaces(self, request, context=None):
                             self.logger.exception(f"{ns_bdev=} parse error")
                             pass
                     namespaces.append(one_ns)
-                break
             except Exception:
                 self.logger.exception(f"{s=} parse error")
                 pass

control/proto/gateway.proto

@@ -523,6 +523,7 @@ message namespace_cli {
 	uint64 w_mbytes_per_second = 12;
 	bool auto_visible = 13;
 	repeated string hosts = 14;
+	optional string subsystem_nqn = 15;
 }
 
 message namespaces_info {

tests/test_cli.py

@@ -24,6 +24,10 @@
 image12 = "mytestdevimage12"
 image13 = "mytestdevimage13"
 image14 = "mytestdevimage14"
+image15 = "mytestdevimage15"
+image16 = "mytestdevimage16"
+image17 = "mytestdevimage17"
+image18 = "mytestdevimage18"
 pool = "rbd"
 subsystem = "nqn.2016-06.io.spdk:cnode1"
 subsystem2 = "nqn.2016-06.io.spdk:cnode2"
@@ -33,6 +37,7 @@
 subsystem6 = "nqn.2016-06.io.spdk:cnode6"
 subsystem7 = "nqn.2016-06.io.spdk:cnode7"
 subsystem8 = "nqn.2016-06.io.spdk:cnode8"
+subsystem9 = "nqn.2016-06.io.spdk:cnode9"
 subsystemX = "nqn.2016-06.io.spdk:cnodeX"
 discovery_nqn = "nqn.2014-08.org.nvmexpress.discovery"
 serial = "Ceph00000000000001"
@@ -80,7 +85,7 @@ def gateway(config):
     config.config["gateway"]["group"] = group_name
     config.config["gateway"]["max_namespaces_with_netmask"] = "3"
     config.config["gateway"]["max_hosts_per_namespace"] = "3"
-    config.config["gateway"]["max_subsystems"] = "3"
+    config.config["gateway"]["max_subsystems"] = "4"
     config.config["gateway"]["max_namespaces"] = "12"
     config.config["gateway"]["max_namespaces_per_subsystem"] = "11"
     config.config["gateway"]["max_hosts_per_subsystem"] = "4"
@@ -170,7 +175,7 @@ def test_get_gateway_info(self, caplog, gateway):
         assert gw_info.spdk_version == spdk_ver
         assert gw_info.name == gw.gateway_name
         assert gw_info.hostname == gw.host_name
-        assert gw_info.max_subsystems == 3
+        assert gw_info.max_subsystems == 4
         assert gw_info.max_namespaces == 12
         assert gw_info.max_namespaces_per_subsystem == 11
         assert gw_info.max_hosts_per_subsystem == 4
@@ -1238,6 +1243,78 @@ def test_create_listener_on_discovery(self, caplog, listener, gateway):
         cli(["listener", "add", "--host-name", host_name] + listener)
         assert "Can't create a listener for a discovery subsystem" in caplog.text
 
+    def test_list_namespaces_all_subsystems(self, caplog):
+        caplog.clear()
+        cli(["subsystem", "add", "--subsystem", subsystem9, "--no-group-append"])
+        assert f"Adding subsystem {subsystem9}: Successful" in caplog.text
+        caplog.clear()
+        cli(["namespace", "del", "--subsystem", subsystem, "--nsid", "10"])
+        assert f"Deleting namespace 10 from {subsystem}: Successful" in caplog.text
+        caplog.clear()
+        cli(["namespace", "del", "--subsystem", subsystem, "--nsid", "11"])
+        assert f"Deleting namespace 11 from {subsystem}: Successful" in caplog.text
+        caplog.clear()
+        cli(["namespace", "del", "--subsystem", subsystem, "--nsid", "12"])
+        assert f"Deleting namespace 12 from {subsystem}: Successful" in caplog.text
+        caplog.clear()
+        cli(["namespace", "add", "--subsystem", subsystem9, "--rbd-pool", pool,
+             "--rbd-image", image15, "--size", "16MB", "--rbd-create-image"])
+        assert f"Adding namespace 1 to {subsystem9}: Successful" in caplog.text
+        caplog.clear()
+        cli(["namespace", "add", "--subsystem", subsystem9, "--rbd-pool", pool,
+             "--rbd-image", image16, "--size", "16MB", "--rbd-create-image"])
+        assert f"Adding namespace 2 to {subsystem9}: Successful" in caplog.text
+        caplog.clear()
+        cli(["--format", "json", "namespace", "list"])
+        assert '"subsystem_nqn": "*"' in caplog.text
+        assert f'"subsystem_nqn": "{subsystem}"' in caplog.text
+        assert f'"subsystem_nqn": "{subsystem9}"' in caplog.text
+        assert '"nsid": 1' in caplog.text
+        assert '"nsid": 2' in caplog.text
+        assert '"nsid": 6' in caplog.text
+        caplog.clear()
+        cli(["--format", "json", "namespace", "list", "--nsid", "1"])
+        assert '"subsystem_nqn": "*"' in caplog.text
+        assert f'"subsystem_nqn": "{subsystem}"' in caplog.text
+        assert f'"subsystem_nqn": "{subsystem9}"' in caplog.text
+        assert '"nsid": 1' in caplog.text
+        assert '"nsid": 2' not in caplog.text
+        caplog.clear()
+        cli(["--format", "json", "namespace", "list", "--nsid", "6"])
+        assert '"subsystem_nqn": "*"' in caplog.text
+        assert f'"subsystem_nqn": "{subsystem}"' in caplog.text
+        assert f'"subsystem_nqn": "{subsystem9}"' not in caplog.text
+        assert '"nsid": 6' in caplog.text
+        assert '"nsid": 1' not in caplog.text
+        assert '"nsid": 2' not in caplog.text
+        caplog.clear()
+        cli(["--format", "json", "namespace", "list", "--uuid", uuid])
+        assert '"subsystem_nqn": "*"' in caplog.text
+        assert f'"subsystem_nqn": "{subsystem}"' in caplog.text
+        assert f'"subsystem_nqn": "{subsystem9}"' not in caplog.text
+        assert f'"uuid": "{uuid}"' in caplog.text
+
+    def test_namespace_count_updated(self, caplog):
+        caplog.clear()
+        cli(["subsystem", "add", "--subsystem", subsystem5, "--no-group-append"])
+        assert f"Adding subsystem {subsystem5}: Successful" in caplog.text
+        caplog.clear()
+        cli(["namespace", "add", "--subsystem", subsystem5, "--rbd-pool", pool,
+             "--rbd-image", image17, "--size", "16MB", "--rbd-create-image"])
+        assert f"Adding namespace 1 to {subsystem5}: Successful" in caplog.text
+        caplog.clear()
+        cli(["namespace", "add", "--subsystem", subsystem5, "--rbd-pool", pool,
+             "--rbd-image", image18, "--size", "16MB", "--rbd-create-image"])
+        assert f"Failure adding namespace to {subsystem5}: Maximal number of namespaces (12) " \
+               f"has already been reached" in caplog.text
+        caplog.clear()
+        cli(["subsystem", "del", "--subsystem", subsystem9, "--force"])
+        assert f"Deleting subsystem {subsystem9}: Successful" in caplog.text
+        caplog.clear()
+        cli(["namespace", "add", "--subsystem", subsystem5, "--rbd-pool", pool,
+             "--rbd-image", image18, "--size", "16MB", "--rbd-create-image"])
+        assert f"Adding namespace 2 to {subsystem5}: Successful" in caplog.text
+
 
 class TestDelete:
     @pytest.mark.parametrize("host", host_list)
@@ -1392,6 +1469,9 @@ def test_delete_subsystem(self, caplog, gateway):
         cli(["subsystem", "del", "--subsystem", subsystem2])
         assert f"Deleting subsystem {subsystem2}: Successful" in caplog.text
         caplog.clear()
+        cli(["subsystem", "del", "--subsystem", subsystem5, "--force"])
+        assert f"Deleting subsystem {subsystem5}: Successful" in caplog.text
+        caplog.clear()
         cli(["subsystem", "list"])
         assert "No subsystems" in caplog.text
 
@@ -1493,8 +1573,14 @@ def test_add_too_many_subsystem(self, caplog, gateway):
         assert f"Adding subsystem {subsystem6}: Successful" in caplog.text
         caplog.clear()
         cli(["subsystem", "add", "--subsystem", subsystem7, "--no-group-append"])
-        assert f"Failure creating subsystem {subsystem7}: Maximal number of subsystems (3) has " \
+        assert f"Adding subsystem {subsystem7}: Successful" in caplog.text
+        caplog.clear()
+        cli(["subsystem", "add", "--subsystem", subsystem8, "--no-group-append"])
+        assert f"Failure creating subsystem {subsystem8}: Maximal number of subsystems (4) has " \
                f"already been reached" in caplog.text
+        caplog.clear()
+        cli(["subsystem", "del", "--subsystem", subsystem7])
+        assert f"Deleting subsystem {subsystem7}: Successful" in caplog.text
 
     def test_too_many_hosts(self, caplog, gateway):
         caplog.clear()