From 6ff8d6296646259bfbda10c1d6542bc0da3d6bb8 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Thu, 9 Apr 2026 13:00:18 -0400 Subject: [PATCH 1/4] nvmf: fix timeout not applied to discovery log commands args->timeout was set in the discovery args struct but never copied into cmd.timeout_ms before issuing the log page commands, so the caller-specified timeout had no effect. Signed-off-by: Martin Belanger --- libnvme/src/nvme/fabrics.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libnvme/src/nvme/fabrics.c b/libnvme/src/nvme/fabrics.c index 903033c2ed..6e59dfd382 100644 --- a/libnvme/src/nvme/fabrics.c +++ b/libnvme/src/nvme/fabrics.c @@ -1319,6 +1319,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 +1351,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 +1370,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, From 1a67a593002a3a8d70a4a6e90b6be2e805bd9fad Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Thu, 9 Apr 2026 13:01:00 -0400 Subject: [PATCH 2/4] nvmf: fix generator emitting setters for read-only members in .ld files generate_ld() always emitted both getter and setter entries regardless of the member's is_const flag, unlike generate_hdr() and generate_src() which both gate the setter on not is_const. As a result, accessors.ld exported setter symbols that had no __public definition in accessors.c. Also introduce LD_BANNER (a compact copyright block without the ASCII art) for generated .ld files, since the "Generated Code" art is only useful in .h/.c files that editors open regularly. Regenerate accessors.ld to remove the spurious setter entries and update the banner. Signed-off-by: Martin Belanger --- libnvme/src/accessors.ld | 2 +- libnvme/tools/generator/generate-accessors.py | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/libnvme/src/accessors.ld b/libnvme/src/accessors.ld index 58aa702356..474d8dbf8c 100644 --- a/libnvme/src/accessors.ld +++ b/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; diff --git a/libnvme/tools/generator/generate-accessors.py b/libnvme/tools/generator/generate-accessors.py index e8085eedb7..7e6ce48a13 100755 --- a/libnvme/tools/generator/generate-accessors.py +++ b/libnvme/tools/generator/generate-accessors.py @@ -74,6 +74,16 @@ " */" ) +LD_BANNER = ( + "/**\n" + " * This file is part of libnvme.\n" + " *\n" + " * Copyright (c) 2025, Dell Technologies Inc. or its subsidiaries.\n" + " * Authors: Martin Belanger \n" + " *\n" + " */" +) + # --------------------------------------------------------------------------- # Regular expressions # --------------------------------------------------------------------------- @@ -500,10 +510,9 @@ def generate_src(f, prefix, struct_name, members): def generate_ld(f, prefix, struct_name, members): """Write linker version-script entries for all members of one struct.""" for member in members: - f.write( - f'\t\t{_get_name(prefix, struct_name, member.name)};\n' - f'\t\t{_set_name(prefix, struct_name, member.name)};\n' - ) + f.write(f'\t\t{_get_name(prefix, struct_name, member.name)};\n') + if not member.is_const: + f.write(f'\t\t{_set_name(prefix, struct_name, member.name)};\n') # --------------------------------------------------------------------------- @@ -656,7 +665,7 @@ def main(): f.write( f'{SPDX_LD}\n' f'\n' - f'{BANNER}\n' + f'{LD_BANNER}\n' f'\n' f'LIBNVME_ACCESSORS_3 {{\n' f'\tglobal:\n' From eb36af744844973fec34b395d31e896818ba907f Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Thu, 9 Apr 2026 13:01:29 -0400 Subject: [PATCH 3/4] nvmf: clean up discovery args struct initialization Replace literal zero initializers with the proper named constants: .result = 0 -> removed (zero is the default for unspecified fields) .lsp = 0 -> .lsp = NVMF_LOG_DISC_LSP_NONE Also add the missing .timeout = NVME_DEFAULT_IOCTL_TIMEOUT in _nvmf_discovery() to be consistent with nbft_discovery(). Signed-off-by: Martin Belanger --- libnvme/src/nvme/fabrics.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/libnvme/src/nvme/fabrics.c b/libnvme/src/nvme/fabrics.c index 6e59dfd382..bd2b87c7f0 100644 --- a/libnvme/src/nvme/fabrics.c +++ b/libnvme/src/nvme/fabrics.c @@ -1989,8 +1989,8 @@ static int _nvmf_discovery(struct libnvme_global_ctx *ctx, .c = c, .args_size = sizeof(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); @@ -2672,9 +2672,8 @@ static int nbft_discovery(struct libnvme_global_ctx *ctx, .c = c, .args_size = sizeof(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); From f55f1871e26ae664b119050f8408b3826fa79376 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Thu, 9 Apr 2026 13:02:07 -0400 Subject: [PATCH 4/4] 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 --- README.md | 88 ++--------- libnvme/README.md | 82 ++++++++++ libnvme/examples/discover-loop.c | 11 +- libnvme/libnvme/nvme.i | 21 ++- libnvme/src/libnvmf.ld | 3 +- libnvme/src/meson.build | 10 +- libnvme/src/nvme/fabrics.c | 73 +++++---- libnvme/src/nvme/fabrics.h | 74 ++++----- libnvme/src/nvme/nvmf-accessors.c | 55 +++++++ libnvme/src/nvme/nvmf-accessors.h | 68 +++++++++ libnvme/src/nvme/private-fabrics.h | 26 ++++ libnvme/src/nvmf-accessors.ld | 17 +++ libnvme/test/ioctl/discovery.c | 36 ++++- libnvme/tools/check-public-symbols.py | 1 + libnvme/tools/generator/meson.build | 25 ++- libnvme/tools/generator/update-accessors.sh | 160 ++++++++++++-------- 16 files changed, 506 insertions(+), 244 deletions(-) create mode 100644 libnvme/src/nvme/nvmf-accessors.c create mode 100644 libnvme/src/nvme/nvmf-accessors.h create mode 100644 libnvme/src/nvme/private-fabrics.h create mode 100644 libnvme/src/nvmf-accessors.ld diff --git a/README.md b/README.md index 1a587ab57f..1d6153717c 100644 --- a/README.md +++ b/README.md @@ -303,90 +303,24 @@ ENTRY, then append the object file name to the meson.build "sources". ### Updating the libnvme accessor functions -libnvme exposes a set of getter and setter functions (accessors) for its core -internal structs (`libnvme_path`, `libnvme_ns`, `libnvme_ctrl`, `libnvme_subsystem`, -`libnvme_host`, `libnvme_fabric_options`). These are generated from the struct -definitions in `libnvme/src/nvme/private.h` by the tool -`libnvme/src/nvme/generate-accessors.c`. +libnvme exposes auto-generated getter/setter accessor functions for its +ABI-stable opaque structs. Two sets of accessors are maintained: -The generated files are committed to the source tree and are **not** -regenerated during a normal build: +| Set | Input header | Generated files | +|-----|-------------|-----------------| +| Common NVMe | `libnvme/src/nvme/private.h` | `src/nvme/accessors.{h,c}`, `src/accessors.ld` | +| NVMe-oF | `libnvme/src/nvme/private-fabrics.h` | `src/nvme/nvmf-accessors.{h,c}`, `src/nvmf-accessors.ld` | -``` -libnvme/src/nvme/accessors.h # public API declarations (with Doxygen stubs) -libnvme/src/nvme/accessors.c # implementations -libnvme/src/nvme/accessors.ld # linker version script (manually maintained) -``` - -#### When to regenerate - -Regeneration is needed when a struct member is added, removed, or renamed in -`private.h`, or when a struct is added to or removed from -`generate-accessors-include.list` or excluded in `generate-accessors-exclude.list`. - -#### How to regenerate - -```shell -$ make update-accessors -``` - -or equivalently: +The generated `.h` and `.c` files are committed to the source tree and are +**not** regenerated during a normal build. To regenerate after modifying a +`/*!generate-accessors*/` struct: ```shell $ meson compile -C .build update-accessors ``` -The script compiles the generator, runs it, and atomically updates -`accessors.h` and `accessors.c` only when their content changes. -Commit the updated files afterward: - -```shell -$ git add libnvme/src/nvme/accessors.h libnvme/src/nvme/accessors.c -$ git commit -m "libnvme: regenerate accessors following changes" -``` - -#### Maintaining accessors.ld - -`accessors.ld` is a GNU linker version script that controls which accessor -symbols are exported from `libnvme.so` and under which ABI version label they -were introduced (e.g. `LIBNVME_ACCESSORS_3`). - -This file is **not** updated automatically, because each symbol must be placed -in the correct version section by the maintainer. Adding a symbol to an -already-published version section would break binary compatibility for -existing users of the library. - -When `make update-accessors` detects that the symbol list has drifted from -`accessors.ld`, it prints a report like the following: - -``` -WARNING: accessors.ld needs manual attention. - - Symbols to ADD (place in a new version section, e.g. LIBNVME_ACCESSORS_X_Y): - libnvme_ctrl_get_new_field - libnvme_ctrl_set_new_field -``` - -New symbols must be added to a **new** version section that chains the -previous one. For example, if the current latest section is -`LIBNVME_ACCESSORS_3_0`, add a new section for the next release: - -``` -LIBNVME_ACCESSORS_3_1 { - global: - libnvme_ctrl_get_new_field; - libnvme_ctrl_set_new_field; -} LIBNVME_ACCESSORS_3_0; -``` - -Then commit `accessors.ld` together with the regenerated source files. - -#### CI enforcement - -A GitHub Actions workflow (`.github/workflows/check-accessors.yml`) runs on -every push and pull request. It regenerates the accessor files and fails if -the result differs from what is committed, ensuring the source tree never -drifts silently. +See [`libnvme/README.md`](libnvme/README.md#accessor-generation) for full +details, including how to maintain the `.ld` version-script files. ## Dependency diff --git a/libnvme/README.md b/libnvme/README.md index d4afffa4dc..df77397abb 100644 --- a/libnvme/README.md +++ b/libnvme/README.md @@ -186,3 +186,85 @@ meson setup .build -Db_sanitize=address && LD_PRELOAD=/lib64/libasan.so.6 ninja ``` It's also possible to enable the undefined behavior sanitizer with `-Db_sanitize=undefined`. To enable both, use `-Db_sanitize=address,undefined`. + +## Accessor generation + +Some public structs in libnvme use auto-generated setter/getter accessor +functions to provide ABI stability. Callers never access struct members +directly; they use the generated accessors instead. The generated files are +committed to the source tree and are **not** regenerated during a normal build. + +Two sets of accessors are maintained — one for common NVMe structs and one for +NVMe-oF-specific structs. The split exists so that non-fabrics (e.g. embedded +or PCIe-only) builds can exclude all fabrics code entirely. + +| Meson target | Input header | Generated files | +|---|---|---| +| `update-common-accessors` | `src/nvme/private.h` | `src/nvme/accessors.{h,c}`, `src/accessors.ld` | +| `update-fabrics-accessors` | `src/nvme/private-fabrics.h` | `src/nvme/nvmf-accessors.{h,c}`, `src/nvmf-accessors.ld` | + +### When to regenerate + +Regeneration is needed whenever a `/*!generate-accessors*/` struct in +`private.h` or `private-fabrics.h` has a member added, removed, or renamed. + +### How to regenerate + +To regenerate both sets at once: + +```bash +meson compile -C .build update-accessors +``` + +Or regenerate only one set: + +```bash +meson compile -C .build update-common-accessors +meson compile -C .build update-fabrics-accessors +``` + +The script atomically updates the `.h` and `.c` files when their content +changes. Commit the updated files afterward: + +```bash +git add libnvme/src/nvme/accessors.h libnvme/src/nvme/accessors.c +git add libnvme/src/nvme/nvmf-accessors.h libnvme/src/nvme/nvmf-accessors.c +git commit -m "libnvme: regenerate accessors following changes" +``` + +### Maintaining the .ld version-script files + +The `.ld` files (`src/accessors.ld` and `src/nvmf-accessors.ld`) are GNU +linker version scripts that control which accessor symbols are exported from +the shared library and under which ABI version label they were introduced +(e.g. `LIBNVME_ACCESSORS_3`, `LIBNVMF_ACCESSORS_3`). + +These files are **not** updated automatically, because each new symbol must be +placed in the correct version section by the maintainer. Adding a symbol to an +already-published version section would break binary compatibility for +existing users of the library. + +When the generator detects that the symbol list has drifted, it prints a +report like the following: + +``` +WARNING: accessors.ld needs manual attention. + + Symbols to ADD (new version section, e.g. LIBNVME_ACCESSORS_X_Y): + libnvme_ctrl_get_new_field + libnvme_ctrl_set_new_field +``` + +New symbols must be added to a **new** version section that chains the +previous one. For example, if the current latest section is +`LIBNVME_ACCESSORS_3`, add: + +``` +LIBNVME_ACCESSORS_4 { + global: + libnvme_ctrl_get_new_field; + libnvme_ctrl_set_new_field; +} LIBNVME_ACCESSORS_3; +``` + +Then commit the updated `.ld` file together with the regenerated source files. diff --git a/libnvme/examples/discover-loop.c b/libnvme/examples/discover-loop.c index 1083e427bb..37702a22d2 100644 --- a/libnvme/examples/discover-loop.c +++ b/libnvme/examples/discover-loop.c @@ -51,12 +51,13 @@ 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; int ret; struct libnvme_fabrics_config cfg; + struct nvmf_discovery_args *args; nvmf_default_config(&cfg); @@ -86,7 +87,13 @@ int main() return 1; } - ret = nvmf_get_discovery_log(c, &log, 4); + ret = nvmf_discovery_args_create(&args); + if (!ret) { + nvmf_discovery_args_set_max_retries(args, 4); + ret = nvmf_get_discovery_log(c, args, &log); + nvmf_discovery_args_free(args); + } + libnvme_disconnect_ctrl(c); libnvme_free_ctrl(c); diff --git a/libnvme/libnvme/nvme.i b/libnvme/libnvme/nvme.i index d71de88e81..11c3507481 100644 --- a/libnvme/libnvme/nvme.i +++ b/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; diff --git a/libnvme/src/libnvmf.ld b/libnvme/src/libnvmf.ld index 6ee8e719d2..45b91f9224 100644 --- a/libnvme/src/libnvmf.ld +++ b/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; diff --git a/libnvme/src/meson.build b/libnvme/src/meson.build index 4fa1214167..d06873d718 100644 --- a/libnvme/src/meson.build +++ b/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 \n#include ') + '#include \n#include \n#include ') else libconf.set('FABRICS_INCLUDE', '') endif diff --git a/libnvme/src/nvme/fabrics.c b/libnvme/src/nvme/fabrics.c index bd2b87c7f0..659bc3615f 100644 --- a/libnvme/src/nvme/fabrics.c +++ b/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)); @@ -1319,7 +1323,6 @@ 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, @@ -1351,7 +1354,6 @@ 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); @@ -1370,7 +1372,6 @@ 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, @@ -1421,37 +1422,55 @@ 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->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, + .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 +2004,12 @@ 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 +2684,12 @@ 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", diff --git a/libnvme/src/nvme/fabrics.h b/libnvme/src/nvme/fabrics.h index 1d934b2412..572ef07bcc 100644 --- a/libnvme/src/nvme/fabrics.h +++ b/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. diff --git a/libnvme/src/nvme/nvmf-accessors.c b/libnvme/src/nvme/nvmf-accessors.c new file mode 100644 index 0000000000..a021d551c5 --- /dev/null +++ b/libnvme/src/nvme/nvmf-accessors.c @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +/** + * This file is part of libnvme. + * + * Copyright (c) 2025, Dell Technologies Inc. or its subsidiaries. + * Authors: Martin Belanger + * + * ____ _ _ ____ _ + * / ___| ___ _ __ ___ _ __ __ _| |_ ___ __| | / ___|___ __| | ___ + * | | _ / _ \ '_ \ / _ \ '__/ _` | __/ _ \/ _` | | | / _ \ / _` |/ _ \ + * | |_| | __/ | | | __/ | | (_| | || __/ (_| | | |__| (_) | (_| | __/ + * \____|\___|_| |_|\___|_| \__,_|\__\___|\__,_| \____\___/ \__,_|\___| + * + * Auto-generated struct member accessors (setter/getter) + * + * To update run: meson compile -C [BUILD-DIR] update-accessors + * Or: make update-accessors + */ +#include +#include +#include "nvmf-accessors.h" + +#include "private-fabrics.h" +#include "compiler_attributes.h" + +/**************************************************************************** + * Accessors for: struct nvmf_discovery_args + ****************************************************************************/ + +__public void nvmf_discovery_args_set_max_retries( + struct nvmf_discovery_args *p, + int max_retries) +{ + p->max_retries = max_retries; +} + +__public int nvmf_discovery_args_get_max_retries( + const struct nvmf_discovery_args *p) +{ + return p->max_retries; +} + +__public void nvmf_discovery_args_set_lsp( + struct nvmf_discovery_args *p, + __u8 lsp) +{ + p->lsp = lsp; +} + +__public __u8 nvmf_discovery_args_get_lsp(const struct nvmf_discovery_args *p) +{ + return p->lsp; +} + diff --git a/libnvme/src/nvme/nvmf-accessors.h b/libnvme/src/nvme/nvmf-accessors.h new file mode 100644 index 0000000000..25a0558802 --- /dev/null +++ b/libnvme/src/nvme/nvmf-accessors.h @@ -0,0 +1,68 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +/** + * This file is part of libnvme. + * + * Copyright (c) 2025, Dell Technologies Inc. or its subsidiaries. + * Authors: Martin Belanger + * + * ____ _ _ ____ _ + * / ___| ___ _ __ ___ _ __ __ _| |_ ___ __| | / ___|___ __| | ___ + * | | _ / _ \ '_ \ / _ \ '__/ _` | __/ _ \/ _` | | | / _ \ / _` |/ _ \ + * | |_| | __/ | | | __/ | | (_| | || __/ (_| | | |__| (_) | (_| | __/ + * \____|\___|_| |_|\___|_| \__,_|\__\___|\__,_| \____\___/ \__,_|\___| + * + * Auto-generated struct member accessors (setter/getter) + * + * To update run: meson compile -C [BUILD-DIR] update-accessors + * Or: make update-accessors + */ +#ifndef _NVMF_ACCESSORS_H_ +#define _NVMF_ACCESSORS_H_ + +#include +#include +#include +#include +#include /* __u32, __u64, etc. */ + +/* Forward declarations. These are internal (opaque) structs. */ +struct nvmf_discovery_args; + +/**************************************************************************** + * Accessors for: struct nvmf_discovery_args + ****************************************************************************/ + +/** + * nvmf_discovery_args_set_max_retries() - Set max_retries. + * @p: The &struct nvmf_discovery_args instance to update. + * @max_retries: Value to assign to the max_retries field. + */ +void nvmf_discovery_args_set_max_retries( + struct nvmf_discovery_args *p, + int max_retries); + +/** + * nvmf_discovery_args_get_max_retries() - Get max_retries. + * @p: The &struct nvmf_discovery_args instance to query. + * + * Return: The value of the max_retries field. + */ +int nvmf_discovery_args_get_max_retries(const struct nvmf_discovery_args *p); + +/** + * nvmf_discovery_args_set_lsp() - Set lsp. + * @p: The &struct nvmf_discovery_args instance to update. + * @lsp: Value to assign to the lsp field. + */ +void nvmf_discovery_args_set_lsp(struct nvmf_discovery_args *p, __u8 lsp); + +/** + * nvmf_discovery_args_get_lsp() - Get lsp. + * @p: The &struct nvmf_discovery_args instance to query. + * + * Return: The value of the lsp field. + */ +__u8 nvmf_discovery_args_get_lsp(const struct nvmf_discovery_args *p); + +#endif /* _NVMF_ACCESSORS_H_ */ diff --git a/libnvme/src/nvme/private-fabrics.h b/libnvme/src/nvme/private-fabrics.h new file mode 100644 index 0000000000..936cca45f9 --- /dev/null +++ b/libnvme/src/nvme/private-fabrics.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * This file is part of libnvme. + * Copyright (c) 2026, Dell Technologies Inc. or its subsidiaries. + * + * Authors: Martin Belanger + */ +#pragma once + +#include + +#include + +/** + * NVMe-oF private struct definitions. + * + * Structs in this file are NVMe-oF-specific (fabrics layer). They are kept + * separate from private.h so that PCIe-only builds can exclude this entire + * file and its generated accessors (nvmf-accessors.{h,c}) along with the + * rest of the fabrics layer. + */ + +struct nvmf_discovery_args { /*!generate-accessors*/ + int max_retries; + __u8 lsp; +}; diff --git a/libnvme/src/nvmf-accessors.ld b/libnvme/src/nvmf-accessors.ld new file mode 100644 index 0000000000..b7aaf57c17 --- /dev/null +++ b/libnvme/src/nvmf-accessors.ld @@ -0,0 +1,17 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later + +/** + * This file is part of libnvme. + * + * Copyright (c) 2025, Dell Technologies Inc. or its subsidiaries. + * Authors: Martin Belanger + * + */ + +LIBNVMF_ACCESSORS_3 { + global: + nvmf_discovery_args_get_lsp; + nvmf_discovery_args_set_lsp; + nvmf_discovery_args_get_max_retries; + nvmf_discovery_args_set_max_retries; +}; diff --git a/libnvme/test/ioctl/discovery.c b/libnvme/test/ioctl/discovery.c index a51574af97..a408c41046 100644 --- a/libnvme/test/ioctl/discovery.c +++ b/libnvme/test/ioctl/discovery.c @@ -37,6 +37,23 @@ static void arbitrary_ascii_string(size_t max_len, char *str, char *log_str) } } +/* Convenience wrapper: create args, fetch log, free args */ +static int fetch_discovery_log(libnvme_ctrl_t c, + struct nvmf_discovery_log **logp, + int max_retries) +{ + struct nvmf_discovery_args *args; + int err; + + err = nvmf_discovery_args_create(&args); + if (err) + return err; + nvmf_discovery_args_set_max_retries(args, max_retries); + err = nvmf_get_discovery_log(c, args, logp); + nvmf_discovery_args_free(args); + return err; +} + static void arbitrary_entry(struct nvmf_disc_log_entry *entry, struct nvmf_disc_log_entry *log_entry) { @@ -74,7 +91,7 @@ static void test_no_entries(libnvme_ctrl_t c) struct nvmf_discovery_log *log = NULL; set_mock_admin_cmds(mock_admin_cmds, ARRAY_SIZE(mock_admin_cmds)); - check(nvmf_get_discovery_log(c, &log, 1) == 0, "discovery failed"); + check(fetch_discovery_log(c, &log, 1) == 0, "discovery failed"); end_mock_cmds(); cmp(log, &header, HEADER_LEN, "incorrect header"); free(log); @@ -118,7 +135,7 @@ static void test_four_entries(libnvme_ctrl_t c) arbitrary_entries(num_entries, entries, log_entries); set_mock_admin_cmds(mock_admin_cmds, ARRAY_SIZE(mock_admin_cmds)); - check(nvmf_get_discovery_log(c, &log, 1) == 0, "discovery failed"); + check(fetch_discovery_log(c, &log, 1) == 0, "discovery failed"); end_mock_cmds(); cmp(log, &header, HEADER_LEN, "incorrect header"); cmp(log->entries, entries, 0x16 /* sizeof(entries)*/, "incorrect entries"); @@ -177,7 +194,7 @@ static void test_five_entries(libnvme_ctrl_t c) arbitrary_entries(num_entries, entries, log_entries); set_mock_admin_cmds(mock_admin_cmds, ARRAY_SIZE(mock_admin_cmds)); - check(nvmf_get_discovery_log(c, &log, 1) == 0, "discovery failed"); + check(fetch_discovery_log(c, &log, 1) == 0, "discovery failed"); end_mock_cmds(); cmp(log, &header, sizeof(header), "incorrect header"); cmp(log->entries, entries, sizeof(entries), "incorrect entries"); @@ -245,7 +262,7 @@ static void test_genctr_change(libnvme_ctrl_t c) arbitrary(entries1, sizeof(entries1)); arbitrary_entries(num_entries2, entries2, log_entries2); set_mock_admin_cmds(mock_admin_cmds, ARRAY_SIZE(mock_admin_cmds)); - check(nvmf_get_discovery_log(c, &log, 2) == 0, "discovery failed"); + check(fetch_discovery_log(c, &log, 2) == 0, "discovery failed"); end_mock_cmds(); cmp(log, &header2, sizeof(header2), "incorrect header"); cmp(log->entries, entries2, sizeof(entries2), "incorrect entries"); @@ -308,7 +325,8 @@ static void test_max_retries(libnvme_ctrl_t c) arbitrary(&entry, sizeof(entry)); set_mock_admin_cmds(mock_admin_cmds, ARRAY_SIZE(mock_admin_cmds)); - check(nvmf_get_discovery_log(c, &log, 2) == -EAGAIN, "discovery succeeded"); + check(fetch_discovery_log(c, &log, 2) == -EAGAIN, + "discovery succeeded"); end_mock_cmds(); check(!log, "unexpected log page returned"); } @@ -328,7 +346,8 @@ static void test_header_error(libnvme_ctrl_t c) struct nvmf_discovery_log *log = NULL; set_mock_admin_cmds(mock_admin_cmds, ARRAY_SIZE(mock_admin_cmds)); - check(nvmf_get_discovery_log(c, &log, 1) == -EAGAIN, "discovery succeeded"); + check(fetch_discovery_log(c, &log, 1) == -EAGAIN, + "discovery succeeded"); end_mock_cmds(); check(!log, "unexpected log page returned"); } @@ -358,7 +377,7 @@ static void test_entries_error(libnvme_ctrl_t c) struct nvmf_discovery_log *log = NULL; set_mock_admin_cmds(mock_admin_cmds, ARRAY_SIZE(mock_admin_cmds)); - check(nvmf_get_discovery_log(c, &log, 1) == -EIO, "discovery succeeded"); + check(fetch_discovery_log(c, &log, 1) == -EIO, "discovery succeeded"); end_mock_cmds(); check(!log, "unexpected log page returned"); } @@ -396,7 +415,8 @@ static void test_genctr_error(libnvme_ctrl_t c) arbitrary(&entry, sizeof(entry)); set_mock_admin_cmds(mock_admin_cmds, ARRAY_SIZE(mock_admin_cmds)); - check(nvmf_get_discovery_log(c, &log, 1) == NVME_SC_INTERNAL, "discovery succeeded"); + check(fetch_discovery_log(c, &log, 1) == NVME_SC_INTERNAL, + "discovery succeeded"); end_mock_cmds(); check(!log, "unexpected log page returned"); } diff --git a/libnvme/tools/check-public-symbols.py b/libnvme/tools/check-public-symbols.py index f9a0054503..ff387ee58a 100644 --- a/libnvme/tools/check-public-symbols.py +++ b/libnvme/tools/check-public-symbols.py @@ -38,6 +38,7 @@ ROOT / 'src' / 'libnvme.ld', ROOT / 'src' / 'libnvmf.ld', ROOT / 'src' / 'accessors.ld', + ROOT / 'src' / 'nvmf-accessors.ld', ] # --------------------------------------------------------------------------- diff --git a/libnvme/tools/generator/meson.build b/libnvme/tools/generator/meson.build index 6b20e9068f..113d2f318a 100644 --- a/libnvme/tools/generator/meson.build +++ b/libnvme/tools/generator/meson.build @@ -27,14 +27,31 @@ libnvme_src_nvme = libnvme_src / 'nvme' _py3 = find_program('python3', required: true) -run_target( - 'update-accessors', +_tgt_common = run_target( + 'update-common-accessors', command: [ 'update-accessors.sh', _py3, files('generate-accessors.py'), - libnvme_src_nvme, - libnvme_src, + libnvme_src_nvme / 'accessors.h', + libnvme_src_nvme / 'accessors.c', + libnvme_src / 'accessors.ld', libnvme_src_nvme / 'private.h', ], ) + +_tgt_fabrics = run_target( + 'update-fabrics-accessors', + command: [ + 'update-accessors.sh', + _py3, + files('generate-accessors.py'), + libnvme_src_nvme / 'nvmf-accessors.h', + libnvme_src_nvme / 'nvmf-accessors.c', + libnvme_src / 'nvmf-accessors.ld', + libnvme_src_nvme / 'private-fabrics.h', + ], +) + +# This alias allows generating all accessors in one shot. +alias_target('update-accessors', _tgt_common, _tgt_fabrics) diff --git a/libnvme/tools/generator/update-accessors.sh b/libnvme/tools/generator/update-accessors.sh index a21a90268b..25d72a0215 100755 --- a/libnvme/tools/generator/update-accessors.sh +++ b/libnvme/tools/generator/update-accessors.sh @@ -10,77 +10,68 @@ # This script is invoked via: meson compile -C update-accessors # It is NOT run during a normal build. # -# accessors.h and accessors.c are updated automatically when the generator -# produces different output. +# The .h and .c files are updated automatically when the generator produces +# different output. # -# accessors.ld is NOT updated automatically because its version section labels -# (e.g. LIBNVME_ACCESSORS_3) must be assigned by the maintainer. Instead, -# this script reports which symbols have been added or removed so the maintainer -# knows exactly what to change in accessors.ld. +# The .ld file is NOT updated automatically because its version section +# label (e.g. LIBNVME_ACCESSORS_3, LIBNVMF_ACCESSORS_3) must be assigned by +# the maintainer. Instead, this script reports which symbols have been added +# or removed so the maintainer knows exactly what to change. # # Arguments (supplied by the Meson run_target): # $1 path to the python3 interpreter # $2 path to generate-accessors.py -# $3 source directory for accessors.c and accessors.h (src/nvme/) -# $4 source directory for accessors.ld (src/) -# $5 ... one or more input headers (wildcards are accepted) +# $3 full path of the output .h file +# $4 full path of the output .c file +# $5 full path of the output .ld file +# $6 ... one or more input headers scanned for +# /*!generate-accessors*/ structs set -euo pipefail PYTHON="${1:?missing python3 interpreter}" GENERATOR="${2:?missing generator script}" -NVME_SRCDIR="${3:?missing nvme source directory}" -LD_SRCDIR="${4:?missing ld source directory}" -shift 4 -INPUT_HEADERS=("$@") -[ ${#INPUT_HEADERS[@]} -gt 0 ] || { echo "error: no input headers specified" >&2; exit 1; } +H_OUT="${3:?missing .h output path}" +C_OUT="${4:?missing .c output path}" +LD_OUT="${5:?missing .ld output path}" +shift 5 -TMPDIR=$(mktemp -d) -trap 'rm -rf "$TMPDIR"' EXIT +if [ $# -eq 0 ]; then + echo "error: no input headers provided" >&2 + exit 1 +fi -echo "Regenerating accessor files..." +TMPDIR_WORK=$(mktemp -d) +trap 'rm -rf "$TMPDIR_WORK"' EXIT -"$PYTHON" "$GENERATOR" \ - --h-out "$TMPDIR/accessors.h" \ - --c-out "$TMPDIR/accessors.c" \ - --ld-out "$TMPDIR/accessors.ld" \ - "${INPUT_HEADERS[@]}" +LABEL=$(basename "$H_OUT") # e.g. "accessors.h" or "nvmf-accessors.h" +BASE="${LABEL%.h}" # e.g. "accessors" or "nvmf-accessors" + +TMP_H="$TMPDIR_WORK/${BASE}.h" +TMP_C="$TMPDIR_WORK/${BASE}.c" +TMP_LD="$TMPDIR_WORK/${BASE}.ld" # --------------------------------------------------------------------------- -# Update accessors.h and accessors.c atomically when content changes. +# Helper: update a source file atomically when content changes. # --------------------------------------------------------------------------- -changed=0 -for f in accessors.h accessors.c; do - dest="$NVME_SRCDIR/$f" - if [ -f "$dest" ] && cmp -s "$TMPDIR/$f" "$dest"; then - printf " unchanged: %s\n" "$f" +update_if_changed() { + local src="$1" # generated file in TMPDIR_WORK + local dest="$2" # target path in the source tree + + if [ -f "$dest" ] && cmp -s "$src" "$dest"; then + printf " unchanged: %s\n" "$(basename "$dest")" else - # Write to a sibling temp file then rename for atomicity - tmp_dest=$(mktemp "$NVME_SRCDIR/.${f}.XXXXXX") - cp "$TMPDIR/$f" "$tmp_dest" + local tmp_dest + tmp_dest=$(mktemp "$(dirname "$dest")/.$(basename "$dest").XXXXXX") + cp "$src" "$tmp_dest" mv -f "$tmp_dest" "$dest" - printf " updated: %s\n" "$f" - changed=$((changed + 1)) + printf " updated: %s\n" "$(basename "$dest")" + CHANGED=$((CHANGED + 1)) fi -done - -echo "" -if [ "$changed" -gt 0 ]; then - printf "%d file(s) updated in %s\n" "$changed" "$NVME_SRCDIR" - echo "Don't forget to commit the updated files." -else - echo "All accessor source files are up to date." -fi +} # --------------------------------------------------------------------------- -# Compare symbol lists to detect accessors.ld drift. -# -# accessors.ld is manually maintained because its version section labels -# (e.g. LIBNVME_ACCESSORS_3) must be assigned by a human. We therefore -# only report what has changed; we never overwrite the file. -# -# Symbol lines in an ld version script look like: -# symbol_name ; +# Helper: compare symbol lists and report ld drift. # --------------------------------------------------------------------------- extract_syms() { grep -E '^\s+[a-zA-Z_][a-zA-Z0-9_]*;' "$1" \ @@ -88,24 +79,61 @@ extract_syms() { | sort } -extract_syms "$TMPDIR/accessors.ld" > "$TMPDIR/syms_new.txt" -extract_syms "$LD_SRCDIR/accessors.ld" > "$TMPDIR/syms_old.txt" +check_ld_drift() { + local new_ld="$1" + local old_ld="$2" + local ld_name + ld_name=$(basename "$old_ld") -added=$(comm -23 "$TMPDIR/syms_new.txt" "$TMPDIR/syms_old.txt") -removed=$(comm -13 "$TMPDIR/syms_new.txt" "$TMPDIR/syms_old.txt") + extract_syms "$new_ld" > "$TMPDIR_WORK/syms_new.txt" + extract_syms "$old_ld" > "$TMPDIR_WORK/syms_old.txt" -if [ -z "$added" ] && [ -z "$removed" ]; then - echo "accessors.ld: symbol list is up to date." -else - echo "WARNING: accessors.ld needs manual attention." - echo "" - if [ -n "$added" ]; then - echo " Symbols to ADD (place in a new version section, e.g. LIBNVME_ACCESSORS_X_Y):" - printf '%s\n' "$added" | sed 's/^/ /' - fi - if [ -n "$removed" ]; then + local added removed + added=$(comm -23 "$TMPDIR_WORK/syms_new.txt" "$TMPDIR_WORK/syms_old.txt") + removed=$(comm -13 "$TMPDIR_WORK/syms_new.txt" "$TMPDIR_WORK/syms_old.txt") + + if [ -z "$added" ] && [ -z "$removed" ]; then + echo "${ld_name}: symbol list is up to date." + else + echo "WARNING: $(realpath --relative-to=.. ${old_ld}) needs manual attention." echo "" - echo " Symbols to REMOVE from accessors.ld:" - printf '%s\n' "$removed" | sed 's/^/ /' + if [ -n "$added" ]; then + echo " Symbols to ADD (new version section, e.g. _ACCESSORS_X_Y):" + printf '%s\n' "$added" | sed 's/^/ /' + fi + if [ -n "$removed" ]; then + echo "" + echo " Symbols to REMOVE from ${ld_name}:" + printf '%s\n' "$removed" | sed 's/^/ /' + fi fi +} + +# --------------------------------------------------------------------------- +# Run generator +# --------------------------------------------------------------------------- +echo "++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++" +echo "--- ${BASE}: begin generation ---" +echo "" +"$PYTHON" "$GENERATOR" \ + --h-out "$TMP_H" \ + --c-out "$TMP_C" \ + --ld-out "$TMP_LD" \ + "$@" + +CHANGED=0 +update_if_changed "$TMP_H" "$H_OUT" +update_if_changed "$TMP_C" "$C_OUT" +echo "" +if [ "$CHANGED" -gt 0 ]; then + printf "%d file(s) updated in %s\n" "$CHANGED" "$(dirname "$H_OUT")" + echo "Don't forget to commit the updated files." +else + echo "All accessor source files are up to date." fi +echo "" +check_ld_drift "$TMP_LD" "$LD_OUT" +echo "" +echo "--- ${BASE}: generation complete ---" +echo "++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++" +echo ""