fabrics: use context at controller creation time

The controller is created from the provided fabrics context, which
contains all the information required to fully construct it. The
libnvmf_add_ctrl function should only perform the connect call.

Signed-off-by: Daniel Wagner <[email protected]>

Author: igaw

libnvme/examples/discover-loop.c

@@ -67,6 +67,11 @@ int main()
 	if (ret)
 		goto free_ctx;
 
+	ret = libnvmf_context_set_connection(fctx, NVME_DISC_SUBSYS_NAME,
+		"loop", NULL, NULL, NULL, NULL);
+	if (ret)
+		goto free_ctx;
+
 	ret = libnvme_scan_topology(ctx, NULL, NULL);
 	if (ret)
 		goto free_fctx;
@@ -77,14 +82,13 @@ int main()
 		goto free_fctx;
 	}
 
-	ret = libnvmf_create_ctrl(ctx, NVME_DISC_SUBSYS_NAME, "loop",
-			       NULL, NULL, NULL, NULL, &c);
+	ret = libnvmf_create_ctrl(ctx, fctx, &c);
 	if (ret) {
 		fprintf(stderr, "Failed to allocate memory\n");
 		goto free_fctx;
 	}
 
-	ret = libnvmf_add_ctrl(h, c, fctx);
+	ret = libnvmf_add_ctrl(h, c);
 	if (ret) {
 		fprintf(stderr, "no controller found\n");
 		goto free_fctx;

libnvme/libnvme/nvme.i

@@ -737,15 +737,9 @@ struct libnvmf_context {};
     self.__host = h  # Keep a reference to parent to ensure ctrl obj gets GCed before host}
 %extend libnvme_ctrl {
 	libnvme_ctrl(struct libnvme_global_ctx *ctx,
-		  const char *subsysnqn,
-		  const char *transport,
-		  const char *traddr = NULL,
-		  const char *host_traddr = NULL,
-		  const char *host_iface = NULL,
-		  const char *trsvcid = NULL) {
+		  struct libnvmf_context *fctx) {
 		struct libnvme_ctrl *c;
-		if (libnvmf_create_ctrl(ctx, subsysnqn, transport, traddr,
-					host_traddr, host_iface, trsvcid, &c))
+		if (libnvmf_create_ctrl(ctx, fctx, &c))
 			return NULL;
 		return c;
 	}
@@ -765,19 +759,18 @@ struct libnvmf_context {};
 		return libnvme_init_ctrl(h, $self, instance) == 0;
 	}
 
-	void connect(struct libnvme_host *h,
-		     struct libnvmf_context *fctx = NULL) {
+	void connect(struct libnvme_host *h) {
 		int ret;
 		const char *dev;
 
 		dev = libnvme_ctrl_get_name($self);
-		if (dev && !(fctx && fctx->cfg.duplicate_connect)) {
+		if (dev && $self->cfg.duplicate_connect) {
 			connect_err = -ENVME_CONNECT_ALREADY;
 			return;
 		}
 
 		Py_BEGIN_ALLOW_THREADS  /* Release Python GIL */
-		ret = libnvmf_add_ctrl(h, $self, fctx);
+		ret = libnvmf_add_ctrl(h, $self);
 		Py_END_ALLOW_THREADS    /* Reacquire Python GIL */
 
 		if (ret) {

libnvme/libnvme/tests/create-ctrl-obj.py

@@ -1,15 +1,18 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: LGPL-2.1-or-later
-import sys
-import pprint
+
+
 from libnvme import nvme
 
 ctx = nvme.global_ctx()      
-ctx.log_level('debug') 
-
-host = nvme.host(ctx)  
-subsysnqn = nvme.NVME_DISC_SUBSYS_NAME
-transport = 'loop'
-traddr    = '127.0.0.1'
-trsvcid   = '8009'
-ctrl = nvme.ctrl(ctx, subsysnqn=subsysnqn, transport=transport, traddr=traddr, trsvcid=trsvcid)
+ctx.log_level('debug')
+
+fctx = nvme.fabrics_context(ctx)
+fctx.set_connection(
+    subsysnqn=nvme.NVME_DISC_SUBSYS_NAME,
+    transport='loop',
+    traddr='127.0.0.1',
+    trsvcid='8009',
+)
+
+ctrl = nvme.ctrl(ctx, fctx)

libnvme/libnvme/tests/gc.py

@@ -1,8 +1,8 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: LGPL-2.1-or-later
+
+
 import gc
-import sys
-import pprint
 from libnvme import nvme
 
 ctx = nvme.global_ctx()
@@ -12,13 +12,16 @@
 host = nvme.host(ctx)
 print(f'host: {host}')
 
+fctx = nvme.fabrics_context(ctx)
+fctx.set_connection(
+    subsysnqn=nvme.NVME_DISC_SUBSYS_NAME,
+    transport='loop',
+)
+print(f'fctx: {fctx}')
+
 ctrls = []
 for i in range(10):
-    ctrl = nvme.ctrl(
-        ctx,
-        subsysnqn=nvme.NVME_DISC_SUBSYS_NAME,
-        transport='loop',
-    )
+    ctrl = nvme.ctrl(ctx, fctx)
     ctrls.append(ctrl)
     print(f'ctrl {i}: {ctrl}')
 

libnvme/libnvme/tests/test-objects.py

@@ -118,20 +118,26 @@ def tearDown(self):
         gc.collect()
 
     def _make_loop_ctrl(self):
-        return nvme.ctrl(self.ctx, subsysnqn=self.subsysnqn, transport='loop')
+        fctx = nvme.fabrics_context(self.ctx)
+        fctx.set_connection(
+            subsysnqn=self.subsysnqn,
+            transport='loop'
+        )
+        return nvme.ctrl(self.ctx, fctx)
 
     def test_creation_loop_transport(self):
         ctrl = self._make_loop_ctrl()
         self.assertIsNotNone(ctrl)
 
     def test_creation_tcp_transport_with_traddr(self):
-        ctrl = nvme.ctrl(
-            self.ctx,
+        fctx = nvme.fabrics_context(self.ctx)
+        fctx.set_connection(
             subsysnqn=self.subsysnqn,
             transport='tcp',
             traddr='192.168.1.1',
-            trsvcid='4420',
+            trsvcid='4420'
         )
+        ctrl = nvme.ctrl(self.ctx, fctx)
         self.assertIsNotNone(ctrl)
 
     def test_transport_property(self):
@@ -143,22 +149,24 @@ def test_subsysnqn_property(self):
         self.assertEqual(ctrl.subsysnqn, self.subsysnqn)
 
     def test_traddr_property(self):
-        ctrl = nvme.ctrl(
-            self.ctx,
+        fctx = nvme.fabrics_context(self.ctx)
+        fctx.set_connection(
             subsysnqn=self.subsysnqn,
             transport='tcp',
             traddr='10.0.0.1',
         )
+        ctrl = nvme.ctrl(self.ctx, fctx)
         self.assertEqual(ctrl.traddr, '10.0.0.1')
 
     def test_trsvcid_property(self):
-        ctrl = nvme.ctrl(
-            self.ctx,
+        fctx = nvme.fabrics_context(self.ctx)
+        fctx.set_connection(
             subsysnqn=self.subsysnqn,
             transport='tcp',
             traddr='10.0.0.1',
             trsvcid='8009',
         )
+        ctrl = nvme.ctrl(self.ctx, fctx)
         self.assertEqual(ctrl.trsvcid, '8009')
 
     def test_connected_returns_false_before_connect(self):
@@ -175,7 +183,12 @@ def test_str_contains_transport(self):
         self.assertIn('loop', s)
 
     def test_context_manager(self):
-        with nvme.ctrl(self.ctx, subsysnqn=self.subsysnqn, transport='loop') as c:
+        fctx = nvme.fabrics_context(self.ctx)
+        fctx.set_connection(
+            subsysnqn=self.subsysnqn,
+            transport='loop'
+        )
+        with nvme.ctrl(self.ctx, fctx) as c:
             self.assertIsNotNone(c)
 
     def test_namespaces_iterator_returns_list(self):
@@ -221,11 +234,12 @@ class TestCtrlErrorHandling(unittest.TestCase):
 
     def setUp(self):
         self.ctx = nvme.global_ctx()
-        self.ctrl = nvme.ctrl(
-            self.ctx,
+        fctx = nvme.fabrics_context(self.ctx)
+        fctx.set_connection(
             subsysnqn=nvme.NVME_DISC_SUBSYS_NAME,
             transport='loop',
         )
+        self.ctrl = nvme.ctrl(self.ctx, fctx)
 
     def tearDown(self):
         self.ctrl = None

libnvme/src/nvme/fabrics.c

@@ -215,6 +215,8 @@ __public int libnvmf_context_create(struct libnvme_global_ctx *ctx,
 
 	fctx->ctx = ctx;
 
+	libnvmf_default_config(&fctx->cfg);
+
 	fctx->decide_retry = decide_retry;
 	fctx->connected = connected;
 	fctx->already_connected = already_connected;
@@ -1069,34 +1071,18 @@ static const char *lookup_context(struct libnvme_global_ctx *ctx, libnvme_ctrl_t
 }
 
 __public int libnvmf_create_ctrl(struct libnvme_global_ctx *ctx,
-		const char *subsysnqn, const char *transport,
-		const char *traddr, const char *host_traddr,
-		const char *host_iface, const char *trsvcid,
-		libnvme_ctrl_t *cp)
-{
-	struct libnvmf_context fctx = {
-		.transport = transport,
-		.traddr = traddr,
-		.host_traddr = host_traddr,
-		.host_iface = host_iface,
-		.trsvcid = trsvcid,
-		.subsysnqn = subsysnqn,
-	};
-
-	return _libnvme_create_ctrl(ctx, &fctx, cp);
+		struct libnvmf_context *fctx, libnvme_ctrl_t *cp)
+{
+	return _libnvme_create_ctrl(ctx, fctx, cp);
 }
 
-__public int libnvmf_add_ctrl(libnvme_host_t h, libnvme_ctrl_t c,
-		  const struct libnvmf_context *fctx)
+__public int libnvmf_add_ctrl(libnvme_host_t h, libnvme_ctrl_t c)
 {
 	libnvme_subsystem_t s;
 	const char *root_app, *app;
 	__cleanup_free char *argstr = NULL;
 	int ret;
 
-	/* highest prio have configs from command line */
-	merge_config(c, &fctx->cfg);
-
 	/* apply configuration from config file (JSON) */
 	s = libnvme_lookup_subsystem(h, NULL, libnvme_ctrl_get_subsysnqn(c));
 	if (s) {
@@ -1339,7 +1325,7 @@ static int nvmf_connect_disc_entry(libnvme_host_t h,
 	/* update tls or concat */
 	nvmf_update_tls_concat(e, c, h);
 
-	ret = libnvmf_add_ctrl(h, c, fctx);
+	ret = libnvmf_add_ctrl(h, c);
 	if (!ret) {
 		*cp = c;
 		return 0;
@@ -1350,7 +1336,7 @@ static int nvmf_connect_disc_entry(libnvme_host_t h,
 		libnvme_msg(h->ctx, LIBNVME_LOG_INFO, "failed to connect controller, "
 			 "retry with disabling SQ flow control\n");
 		c->cfg.disable_sqflow = false;
-		ret = libnvmf_add_ctrl(h, c, fctx);
+		ret = libnvmf_add_ctrl(h, c);
 		if (!ret) {
 			*cp = c;
 			return 0;
@@ -2207,7 +2193,7 @@ static int libnvme_add_ctrl(struct libnvmf_context *fctx,
 	int err;
 
 retry:
-	err = libnvmf_add_ctrl(h, c, fctx);
+	err = libnvmf_add_ctrl(h, c);
 	if (!err)
 		return 0;
 	if (fctx->decide_retry(fctx, err, fctx->user_data))
@@ -2705,7 +2691,7 @@ static int nbft_connect(struct libnvme_global_ctx *ctx,
 	/* Update tls or concat */
 	nvmf_update_tls_concat(e, c, h);
 
-	ret = libnvmf_add_ctrl(h, c, fctx);
+	ret = libnvmf_add_ctrl(h, c);
 
 	/* Resume logging */
 	if (ss && ss->unavailable && saved_log_level < 1)

libnvme/src/nvme/fabrics.h

@@ -218,16 +218,14 @@ void libnvmf_update_config(libnvme_ctrl_t c,
  * libnvmf_add_ctrl() - Connect a controller and update topology
  * @h:		Host to which the controller should be attached
  * @c:		Controller to be connected
- * @fctx:	Fabrics context
  *
  * Issues a 'connect' command to the NVMe-oF controller and inserts @c
  * into the topology using @h as parent.
  * @c must be initialized and not connected to the topology.
  *
  * Return: 0 on success, or an error code on failure.
  */
-int libnvmf_add_ctrl(libnvme_host_t h, libnvme_ctrl_t c,
-		  const struct libnvmf_context *fctx);
+int libnvmf_add_ctrl(libnvme_host_t h, libnvme_ctrl_t c);
 
 /**
  * libnvmf_connect_ctrl() - Connect a controller
@@ -572,23 +570,15 @@ int libnvmf_discovery_nbft(struct libnvme_global_ctx *ctx,
 /**
  * libnvmf_create_ctrl() - Allocate an unconnected NVMe controller
  * @ctx:		struct libnvme_global_ctx object
- * @subsysnqn:		Subsystem NQN
- * @transport:		Transport type
- * @traddr:		Transport address
- * @host_traddr:	Host transport address
- * @host_iface:		Host interface name
- * @trsvcid:		Transport service ID
+ * @fctx:		Fabrics context
  * @c:			@libnvme_ctrl_t object to return
  *
  * Creates an unconnected controller to be used for libnvme_add_ctrl().
  *
  * Return: 0 on success or negative error code otherwise
  */
 int libnvmf_create_ctrl(struct libnvme_global_ctx *ctx,
-		     const char *subsysnqn, const char *transport,
-		     const char *traddr, const char *host_traddr,
-		     const char *host_iface, const char *trsvcid,
-		     libnvme_ctrl_t *c);
+		struct libnvmf_context *fctx, libnvme_ctrl_t *c);
 
 /**
  * libnvmf_connect() - Connect to fabrics subsystem

libnvme/src/nvme/no-fabrics.c

@@ -13,3 +13,6 @@ bool traddr_is_hostname(struct libnvme_global_ctx *ctx,
 {
 	return false;
 }
+__public void libnvmf_default_config(struct libnvme_fabrics_config *cfg)
+{
+}

libnvme/src/nvme/tree.c

@@ -1048,7 +1048,6 @@ __public void libnvme_free_ctrl(libnvme_ctrl_t c)
 
 __public void libnvmf_default_config(struct libnvme_fabrics_config *cfg)
 {
-	memset(cfg, 0, sizeof(*cfg));
 	cfg->tos = -1;
 	cfg->ctrl_loss_tmo = NVMF_DEF_CTRL_LOSS_TMO;
 }
@@ -1079,7 +1078,7 @@ int _libnvme_create_ctrl(struct libnvme_global_ctx *ctx,
 
 	c->ctx = ctx;
 	c->hdl = NULL;
-	libnvmf_default_config(&c->cfg);
+	c->cfg = fctx->cfg;
 	list_head_init(&c->namespaces);
 	list_head_init(&c->paths);
 	list_node_init(&c->entry);
@@ -1515,6 +1514,7 @@ libnvme_ctrl_t libnvme_lookup_ctrl(libnvme_subsystem_t s,
 	ctx = s->h ? s->h->ctx : NULL;
 	/* Set the NQN to the subsystem the controller should be created in */
 	fctx->subsysnqn = s->subsysnqn;
+	libnvmf_default_config(&fctx->cfg);
 	ret = _libnvme_create_ctrl(ctx, fctx, &c);
 	/* And restore NQN to avoid issues with repetitive calls */
 	fctx->subsysnqn = subsysnqn;