cli/libnvme: add --idempotent and --devid-file to nvme connect#3573
Open
martin-belanger wants to merge 3 commits into
Open
cli/libnvme: add --idempotent and --devid-file to nvme connect#3573martin-belanger wants to merge 3 commits into
martin-belanger wants to merge 3 commits into
Conversation
added 3 commits
July 10, 2026 15:39
Give a caller that manages a fabrics connection as a supervised process (e.g. a systemd transient unit) a way to recover the kernel-assigned device name later, e.g. to disconnect. Writing it from inside libnvmf_connect() means every path out with a live controller -- freshly connected, found already connected in the topology already scanned, or found already connected only after a kernel-race rescan -- reuses the same controller-matching logic libnvme already relies on, rather than a caller re-deriving it. Signed-off-by: Martin Belanger <martin.belanger@dell.com>
nvme-discoverd manages each fabrics connection as a systemd transient unit whose ExecStart is "nvme connect". Without --idempotent, a connect that races with an existing connection exits 1 and fails the unit even though the target is reachable. --devid-file names the file libnvmf_connect() writes the kernel-assigned device name to, so the unit's ExecStop can find it again to disconnect. Signed-off-by: Martin Belanger <martin.belanger@dell.com>
RUNDIR was join_paths(prefix, get_option('rundir')), so any build
without an explicit -Drundir=/run override (this project's own
scripts/build.sh included) silently got <prefix>/run instead of the
real /run tmpfs -- e.g. /usr/local/run on a stock local build. /run is
not an install path: its location is fixed system-wide by the kernel
and the FHS, the same reason systemd hardcodes it rather than deriving
it from --prefix. Test isolation is a separate, already-solved runtime
concern (libnvme_set_test_base_dir()/LIBNVME_TEST_BASE_DIR), so the
build option bought nothing but a second way to get this wrong.
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
martin-belanger
force-pushed
the
nvme-connect-idempotent-devid
branch
from
6c87db5 to
2d9774a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prep patch for nvme-discoverd (not yet upstreamed). discoverd manages each fabrics connection as a systemd transient unit whose
ExecStart=isnvme connect:--idempotentexits 0 when the target is already connected, instead of 1 — otherwise a harmless race with an existing connection fails the unit.--devid-file FILEnames a filelibnvmf_connect()writes the kernel-assigned device name (e.g.nvme0) to on success, so the unit'sExecStop=can find it again to disconnect.Three commits:
devid_fileis sanitized and confined to a reservedRUNDIR "/nvme/devid"subdirectory for security.--idempotentand--devid-filetonvme connect— the CLI options themselves.RUNDIRfrom--prefix— found while testing:RUNDIRwasjoin_paths(prefix, get_option('rundir')), so any build silently got<prefix>/runinstead of the real/runtmpfs./runisn't an install path, so it's now hardcoded, the same way systemd does it.