Skip to content

libnvme, python: pass global context to registry public APIs and fix python test issue#3460

Open
martin-belanger wants to merge 3 commits into
linux-nvme:masterfrom
martin-belanger:add-ctx-to-registry-api
Open

libnvme, python: pass global context to registry public APIs and fix python test issue#3460
martin-belanger wants to merge 3 commits into
linux-nvme:masterfrom
martin-belanger:add-ctx-to-registry-api

Conversation

@martin-belanger

@martin-belanger martin-belanger commented Jun 19, 2026

Copy link
Copy Markdown

@igaw - Per our discussion. I decided to bundle three commits into this PR.

  • Add global context to all registry public APIs.
  • Fix the python registry test. The problem was in the test fixture itself, not
    the python bindings (it relied on fork semantics, which broke under the
    spawn/forkserver default in Python 3.14).
  • Add a Fedora CI job that runs the tests without valgrind, to catch this class
    of Python-version regression early.

Martin Belanger added 2 commits June 19, 2026 13:47
Make struct libnvme_global_ctx the first argument of every public
libnvmf_registry_* function. Most do not use it yet, but threading it in
now lets a future version log via libnvme_msg() or reach per-context
state without breaking the ABI again. Each public function rejects a
NULL context with -EINVAL; the internal *_instance helpers only forward
it and do not re-validate. Also forward-declare the struct in registry.h
so the public header is self-contained.

While here, fix disconnect_all_match() in fabrics.c to take the caller's
context instead of creating one per controller. It also returned -ENOMEM
from a function declared bool, which coerces to true and would have
disconnected controllers on an allocation failure.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
The python-registry parallel-writes test only passed under the "fork"
multiprocessing start method. Python 3.14 makes "forkserver" the default
on Linux, where the test failed two ways:

  - Each child re-imports the test module, which unconditionally created
    a fresh NVME_REGISTRY_DIR. The children then wrote to a directory the
    parent never reads, so the final value came back as the parent's
    initial write.

  - The libnvme context was passed as a Process argument. A SwigPyObject
    cannot be pickled for spawn/forkserver, and a context is not meant to
    be shared across processes anyway.

Reuse an inherited /tmp registry directory instead of always creating a
new one, and have each writer process create its own GlobalCtx. Verified
under fork, forkserver and spawn.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
@martin-belanger martin-belanger changed the title libnvme: pass global context to registry public APIs libnvme, python: pass global context to registry public APIs and fix python test issue Jun 19, 2026
The build matrix runs the test suite only under valgrind, which skips the
fork-based tests (the python-registry parallel-writes test is
skipIf(_under_valgrind)).  Combined with the matrix's Fedora image
trailing the real release, nothing exercised the fork path on a current
interpreter -- so the Python 3.14 forkserver-default breakage went
unnoticed until it failed on a developer's machine.

Add a small job on the upstream fedora:latest image that builds and runs
meson test without valgrind.  Fedora ships new interpreter defaults ahead
of the other CI distros, so this catches that class of regression early.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant