libnvmf: replace nvmf_get_discovery_log() with opaque-args API

Introduce struct nvmf_discovery_args as an ABI-stable opaque object
with auto-generated setter/getter accessors.  The mandatory controller
argument moves to the function signature; optional parameters (timeout,
max_retries, lsp) are set via accessors.  Remove the old
nvmf_get_discovery_wargs() API entirely.

Split the accessor generator output into a common NVMe triplet
(accessors.{h,c,ld}) and an NVMe-oF-specific triplet
(nvmf-accessors.{h,c,ld}) so that non-fabrics builds can exclude all
fabrics code.  The generator script is made generic; two meson targets
(update-common-accessors, update-fabrics-accessors) are exposed under the
update-accessors alias.

Fix a bug in the generator where //!accessors:readonly members were
correctly suppressed in the .h/.c output but still emitted as
setters in the .ld version script.

Signed-off-by: Martin Belanger <[email protected]>

Author: Martin Belanger

libnvme/examples/discover-loop.c

@@ -86,7 +86,16 @@ int main()
 		return 1;
 	}
 
-	ret = nvmf_get_discovery_log(c, &log, 4);
+	{
+		struct nvmf_discovery_args *dargs;
+
+		ret = nvmf_discovery_args_create(&dargs);
+		if (!ret) {
+			nvmf_discovery_args_set_max_retries(dargs, 4);
+			ret = nvmf_get_discovery_log(c, dargs, &log);
+			nvmf_discovery_args_free(dargs);
+		}
+	}
 	libnvme_disconnect_ctrl(c);
 	libnvme_free_ctrl(c);
 

libnvme/libnvme/nvme.i

@@ -777,25 +777,22 @@ struct libnvme_ns {
 
 	%newobject discover;
 	struct nvmf_discovery_log *discover(int lsp = 0, int max_retries = 6) {
-		const char *dev;
 		struct nvmf_discovery_log *logp = NULL;
-		struct libnvme_get_discovery_args args = {
-			.c = $self,
-			.args_size = sizeof(args),
-			.max_retries = max_retries,
-			.result = NULL,
-			.timeout = NVME_DEFAULT_IOCTL_TIMEOUT,
-			.lsp = lsp,
-		};
+		struct nvmf_discovery_args *args = NULL;
 
-		dev = libnvme_ctrl_get_name($self);
-		if (!dev) {
+		if (!libnvme_ctrl_get_name($self)) {
 			discover_err = 1;
 			return NULL;
 		}
+		discover_err = nvmf_discovery_args_create(&args);
+		if (discover_err)
+			return NULL;
+		nvmf_discovery_args_set_lsp(args, lsp);
+		nvmf_discovery_args_set_max_retries(args, max_retries);
 		Py_BEGIN_ALLOW_THREADS  /* Release Python GIL */
-		    discover_err = nvmf_get_discovery_wargs(&args, &logp);
+		    discover_err = nvmf_get_discovery_log($self, args, &logp);
 		Py_END_ALLOW_THREADS    /* Reacquire Python GIL */
+		nvmf_discovery_args_free(args);
 
 		if (logp == NULL) discover_err = 2;
 		return logp;

libnvme/src/accessors.ld

@@ -61,6 +61,7 @@ LIBNVME_ACCESSORS_3 {
 		libnvme_ctrl_get_cntrltype;
 		libnvme_ctrl_get_cntlid;
 		libnvme_ctrl_get_dctype;
+		libnvme_ctrl_get_phy_slot;
 		libnvme_ctrl_get_host_traddr;
 		libnvme_ctrl_get_host_iface;
 		libnvme_ctrl_get_discovery_ctrl;
@@ -71,7 +72,6 @@ LIBNVME_ACCESSORS_3 {
 		libnvme_ctrl_set_discovered;
 		libnvme_ctrl_get_persistent;
 		libnvme_ctrl_set_persistent;
-		libnvme_ctrl_get_phy_slot;
 		libnvme_subsystem_get_name;
 		libnvme_subsystem_get_sysfs_dir;
 		libnvme_subsystem_get_subsysnqn;

libnvme/src/libnvmf.ld

@@ -28,8 +28,9 @@ LIBNVMF_3 {
 		nvmf_exat_ptr_next;
 		nvmf_free_uri;
 		nvmf_get_default_trsvcid;
+		nvmf_discovery_args_create;
+		nvmf_discovery_args_free;
 		nvmf_get_discovery_log;
-		nvmf_get_discovery_wargs;
 		nvmf_is_registration_supported;
 		nvmf_nbft_free;
 		nvmf_nbft_read_files;

libnvme/src/meson.build

@@ -45,10 +45,12 @@ if want_fabrics
     sources += [
         'nvme/fabrics.c',
         'nvme/nbft.c',
+        'nvme/nvmf-accessors.c',
     ]
     headers += [
         'nvme/fabrics.h',
         'nvme/nbft.h',
+        'nvme/nvmf-accessors.h',
     ]
 endif
 
@@ -85,6 +87,7 @@ endif
 nvme_ld = meson.current_source_dir() / 'libnvme.ld'
 nvmf_ld = meson.current_source_dir() / 'libnvmf.ld'
 accessors_ld = meson.current_source_dir() / 'accessors.ld'
+nvmf_accessors_ld = meson.current_source_dir() / 'nvmf-accessors.ld'
 
 link_args = [
     '-Wl,--version-script=@0@'.format(nvme_ld),
@@ -93,9 +96,12 @@ link_args = [
 
 libconf = configuration_data()
 if want_fabrics
-    link_args += '-Wl,--version-script=@0@'.format(nvmf_ld)
+    link_args += [
+        '-Wl,--version-script=@0@'.format(nvmf_ld),
+        '-Wl,--version-script=@0@'.format(nvmf_accessors_ld),
+    ]
     libconf.set('FABRICS_INCLUDE',
-                '#include <nvme/fabrics.h>\n#include <nvme/nbft.h>')
+                '#include <nvme/fabrics.h>\n#include <nvme/nbft.h>\n#include <nvme/nvmf-accessors.h>')
 else
     libconf.set('FABRICS_INCLUDE', '')
 endif

libnvme/src/nvme/fabrics.c

@@ -36,6 +36,7 @@
 
 #include "cleanup.h"
 #include "private.h"
+#include "private-fabrics.h"
 #include "compiler_attributes.h"
 
 const char *nvmf_dev = "/dev/nvme-fabrics";
@@ -1297,16 +1298,17 @@ static int nvmf_connect_disc_entry(libnvme_host_t h,
  */
 #define DISCOVERY_HEADER_LEN 20
 
-static int nvme_discovery_log(const struct libnvme_get_discovery_args *args,
+static int nvme_discovery_log(libnvme_ctrl_t ctrl,
+			      const struct nvmf_discovery_args *args,
 			      struct nvmf_discovery_log **logp)
 {
-	struct libnvme_global_ctx *ctx = args->c->ctx;
+	struct libnvme_global_ctx *ctx = ctrl->ctx;
 	struct nvmf_discovery_log *log;
 	int retries = 0;
 	int err;
-	const char *name = libnvme_ctrl_get_name(args->c);
+	const char *name = libnvme_ctrl_get_name(ctrl);
 	uint64_t genctr, numrec;
-	struct libnvme_transport_handle *hdl = libnvme_ctrl_get_transport_handle(args->c);
+	struct libnvme_transport_handle *hdl = libnvme_ctrl_get_transport_handle(ctrl);
 	struct libnvme_passthru_cmd cmd;
 
 	log = __libnvme_alloc(sizeof(*log));
@@ -1319,6 +1321,7 @@ static int nvme_discovery_log(const struct libnvme_get_discovery_args *args,
 	libnvme_msg(ctx, LOG_DEBUG, "%s: get header (try %d/%d)\n",
 		 name, retries, args->max_retries);
 	nvme_init_get_log_discovery(&cmd, 0, log, DISCOVERY_HEADER_LEN);
+	cmd.timeout_ms = args->timeout;
 	err = libnvme_get_log(hdl, &cmd, false, DISCOVERY_HEADER_LEN);
 	if (err) {
 		libnvme_msg(ctx, LOG_INFO,
@@ -1350,6 +1353,7 @@ static int nvme_discovery_log(const struct libnvme_get_discovery_args *args,
 			 name, numrec, genctr);
 
 		nvme_init_get_log_discovery(&cmd, sizeof(*log), log->entries, entries_size);
+		cmd.timeout_ms = args->timeout;
 		cmd.cdw10 |= NVME_FIELD_ENCODE(args->lsp,
 					       NVME_LOG_CDW10_LSP_SHIFT,
 					       NVME_LOG_CDW10_LSP_MASK);
@@ -1368,6 +1372,7 @@ static int nvme_discovery_log(const struct libnvme_get_discovery_args *args,
 		libnvme_msg(ctx, LOG_DEBUG, "%s: get header again\n", name);
 
 		nvme_init_get_log_discovery(&cmd, 0, log, DISCOVERY_HEADER_LEN);
+		cmd.timeout_ms = args->timeout;
 		err = libnvme_get_log(hdl, &cmd, false, DISCOVERY_HEADER_LEN);
 		if (err) {
 			libnvme_msg(ctx, LOG_INFO,
@@ -1414,27 +1419,38 @@ static void sanitize_discovery_log_entry(struct nvmf_disc_log_entry *e)
 	}
 }
 
-__public int nvmf_get_discovery_log(libnvme_ctrl_t c, struct nvmf_discovery_log **logp,
-			   int max_retries)
+__public int nvmf_discovery_args_create(struct nvmf_discovery_args **argsp)
 {
-	struct libnvme_get_discovery_args args = {
-		.c = c,
-		.max_retries = max_retries,
-		.timeout = NVME_DEFAULT_IOCTL_TIMEOUT,
-		.lsp = NVMF_LOG_DISC_LSP_NONE,
-	};
+	struct nvmf_discovery_args *args;
+
+	if (!argsp)
+		return -EINVAL;
+
+	args = calloc(1, sizeof(*args));
+	if (!args)
+		return -ENOMEM;
 
+	args->max_retries = 6;
+	args->timeout = NVME_DEFAULT_IOCTL_TIMEOUT;
+	args->lsp = NVMF_LOG_DISC_LSP_NONE;
 
-	return nvmf_get_discovery_wargs(&args, logp);
+	*argsp = args;
+	return 0;
 }
 
-__public int nvmf_get_discovery_wargs(struct libnvme_get_discovery_args *args,
-			     struct nvmf_discovery_log **logp)
+__public void nvmf_discovery_args_free(struct nvmf_discovery_args *args)
+{
+	free(args);
+}
+
+__public int nvmf_get_discovery_log(libnvme_ctrl_t ctrl,
+				    struct nvmf_discovery_args *args,
+				    struct nvmf_discovery_log **logp)
 {
 	struct nvmf_discovery_log *log;
 	int err;
 
-	err = nvme_discovery_log(args, &log);
+	err = nvme_discovery_log(ctrl, args, &log);
 	if (err)
 		return err;
 
@@ -1445,6 +1461,7 @@ __public int nvmf_get_discovery_wargs(struct libnvme_get_discovery_args *args,
 	return 0;
 }
 
+
 /**
  * nvmf_get_tel() - Calculate the amount of memory needed for a DIE.
  * @hostsymname:	Symbolic name (may be NULL)
@@ -1978,15 +1995,13 @@ static int _nvmf_discovery(struct libnvme_global_ctx *ctx,
 	uint64_t numrec;
 	int err;
 
-	struct libnvme_get_discovery_args args = {
-		.c = c,
-		.args_size = sizeof(args),
+	struct nvmf_discovery_args args = {
 		.max_retries = fctx->default_max_discovery_retries,
-		.result = 0,
-		.lsp = 0,
+		.timeout = NVME_DEFAULT_IOCTL_TIMEOUT,
+		.lsp = NVMF_LOG_DISC_LSP_NONE,
 	};
 
-	err = nvmf_get_discovery_wargs(&args, &log);
+	err = nvme_discovery_log(c, &args, &log);
 	if (err) {
 		libnvme_msg(ctx, LOG_ERR, "failed to get discovery log: %s\n",
 			libnvme_strerror(err));
@@ -2661,16 +2676,13 @@ static int nbft_discovery(struct libnvme_global_ctx *ctx,
 	int ret;
 	int i;
 
-	struct libnvme_get_discovery_args args = {
-		.c = c,
-		.args_size = sizeof(args),
+	struct nvmf_discovery_args args = {
 		.max_retries = 10 /* MAX_DISC_RETRIES */,
-		.result = 0,
 		.timeout = NVME_DEFAULT_IOCTL_TIMEOUT,
-		.lsp = 0,
+		.lsp = NVMF_LOG_DISC_LSP_NONE,
 	};
 
-	ret = nvmf_get_discovery_wargs(&args, &log);
+	ret = nvme_discovery_log(c, &args, &log);
 	if (ret) {
 		libnvme_msg(ctx, LOG_ERR,
 			"Discovery Descriptor %d: failed to get discovery log: %s\n",

libnvme/src/nvme/fabrics.h

@@ -232,57 +232,47 @@ int nvmf_add_ctrl(libnvme_host_t h, libnvme_ctrl_t c,
  */
 int nvmf_connect_ctrl(libnvme_ctrl_t c);
 
-/**
- * nvmf_get_discovery_log() - Return the discovery log page
- * @c:			Discovery controller to use
- * @logp:		Log page object to return
- * @max_retries:	Number of retries in case of failure
+/*
+ * struct nvmf_discovery_args - Opaque arguments for nvmf_get_discovery_log()
  *
- * The memory allocated for the log page and returned in @logp
- * must be freed by the caller using free().
+ * Allocate with nvmf_discovery_args_create() and release with
+ * nvmf_discovery_args_free(). Use the setter/getter accessors to configure
+ * fields; do not access members directly.
+ */
+struct nvmf_discovery_args;
+
+/**
+ * nvmf_discovery_args_create() - Allocate a discovery args object
+ * @argsp:	On success, set to the newly allocated object
  *
- * Note: Consider using nvmf_get_discovery_wargs() instead.
+ * Allocates and initialises a &struct nvmf_discovery_args with sensible
+ * defaults. The caller must release it with nvmf_discovery_args_free().
  *
- * Return: 0 on success, or an error code on failure.
+ * Return: 0 on success, or a negative error code on failure.
  */
-int nvmf_get_discovery_log(libnvme_ctrl_t c, struct nvmf_discovery_log **logp,
-			   int max_retries);
-
-/**
- * struct libnvme_get_discovery_args - Arguments for nvmf_get_discovery_wargs()
- * @c:			Discovery controller
- * @args_size:		Length of the structure
- * @max_retries:	Number of retries in case of failure
- * @result:		The command completion result from CQE dword0
- * @timeout:		Timeout in ms (default: NVME_DEFAULT_IOCTL_TIMEOUT)
- * @lsp:		Log specific field (See enum nvmf_log_discovery_lsp)
- */
-struct libnvme_get_discovery_args {
-	libnvme_ctrl_t c;
-	int args_size;
-	int max_retries;
-	__u32 *result;
-	__u32 timeout;
-	__u8 lsp;
-};
+int nvmf_discovery_args_create(struct nvmf_discovery_args **argsp);
 
 /**
- * nvmf_get_discovery_wargs() - Get the discovery log page with args
- * @args:	Argument structure
- * @log:	Discovery log page object to return
- *
- * This function is similar to nvmf_get_discovery_log(), but
- * takes an extensible @args parameter. @args provides more
- * options than nvmf_get_discovery_log().
+ * nvmf_discovery_args_free() - Release a discovery args object
+ * @args:	Object previously returned by nvmf_discovery_args_create()
+ */
+void nvmf_discovery_args_free(struct nvmf_discovery_args *args);
+
+/**
+ * nvmf_get_discovery_log() - Fetch the NVMe-oF discovery log page
+ * @ctrl:	Discovery controller
+ * @args:	Optional arguments (pass NULL for defaults)
+ * @logp:	On success, set to the allocated log page (caller must free())
  *
- * This function performs a get discovery log page (DLP) command
- * and returns the DLP. The memory allocated for the returned
- * DLP must be freed by the caller using free().
+ * Issues the three-phase Get Log Page protocol against @ctrl, validates
+ * generation-counter atomicity, and normalises each log entry.
  *
- * Return: 0 on success, or an error code on failure.
+ * Return: 0 on success, or a negative error code on failure.
  */
-int nvmf_get_discovery_wargs(struct libnvme_get_discovery_args *args,
-			     struct nvmf_discovery_log **log);
+int nvmf_get_discovery_log(libnvme_ctrl_t ctrl,
+			   struct nvmf_discovery_args *args,
+			   struct nvmf_discovery_log **logp);
+
 
 /**
  * nvmf_is_registration_supported - check whether registration can be performed.