nvmf: 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 (nvmf-accessors.{h,c,ld}).
The mandatory controller argument moves to the function signature;
optional parameters (timeout, max_retries, lsp) are set via accessors.
Remove nvmf_get_discovery_wargs() 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 PCIe/embedded 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.

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

Author: Martin Belanger

libnvme/examples/discover-loop.c

@@ -51,7 +51,7 @@ static void print_discover_log(struct nvmf_discovery_log *log)
 
 int main()
 {
-	struct nvmf_discovery_log *log;
+	struct nvmf_discovery_log *log = NULL;
 	struct libnvme_global_ctx *ctx;
 	libnvme_host_t h;
 	libnvme_ctrl_t 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/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,19 @@ 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;
+
+	hdl = libnvme_ctrl_get_transport_handle(ctrl);
 	struct libnvme_passthru_cmd cmd;
 
 	log = __libnvme_alloc(sizeof(*log));
@@ -1421,37 +1425,57 @@ static void sanitize_discovery_log_entry(struct libnvme_global_ctx *ctx,
 	}
 }
 
-__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 void nvmf_discovery_args_free(struct nvmf_discovery_args *args)
+{
+	free(args);
 }
 
-__public int nvmf_get_discovery_wargs(struct libnvme_get_discovery_args *args,
-			     struct nvmf_discovery_log **logp)
+__public int nvmf_get_discovery_log(libnvme_ctrl_t ctrl,
+				    const struct nvmf_discovery_args *args,
+				    struct nvmf_discovery_log **logp)
 {
+	static const struct nvmf_discovery_args defaults = {
+		.max_retries = 6,
+		.timeout     = NVME_DEFAULT_IOCTL_TIMEOUT,
+		.lsp         = NVMF_LOG_DISC_LSP_NONE,
+	};
 	struct nvmf_discovery_log *log;
 	int err;
 
-	err = nvme_discovery_log(args, &log);
+	if (!args)
+		args = &defaults;
+
+	err = nvme_discovery_log(ctrl, args, &log);
 	if (err)
 		return err;
 
 	for (int i = 0; i < le64_to_cpu(log->numrec); i++)
-		sanitize_discovery_log_entry(args->c->ctx, &log->entries[i]);
+		sanitize_discovery_log_entry(ctrl->ctx, &log->entries[i]);
 
 	*logp = log;
 	return 0;
 }
 
+
 /**
  * nvmf_get_tel() - Calculate the amount of memory needed for a DIE.
  * @hostsymname:	Symbolic name (may be NULL)
@@ -1985,15 +2009,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,
 		.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));
@@ -2668,15 +2690,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 */,
 		.timeout = NVME_DEFAULT_IOCTL_TIMEOUT,
 		.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,
+			   const struct nvmf_discovery_args *args,
+			   struct nvmf_discovery_log **logp);
+
 
 /**
  * nvmf_is_registration_supported - check whether registration can be performed.