From 7945d33454e325b7facea6adc1a1b87a94ebc444 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Mon, 22 Jun 2026 11:24:22 -0400 Subject: [PATCH] nvme: rework the registry command-line interface Take the controller as the positional argument on retrieve, update and delete, matching every other nvme command and dropping the odd-one-out -d/--device option. Drop the hard -EPERM block on writing the 'owner' attribute: it protected nothing (root can edit /run/nvme/registry/ directly) yet inconsistently let 'delete' clear ownership anyway. update and delete now confirm before an operation that changes or removes ownership; scripts proceed without prompting. While here, give delete an optional --attr to remove a single attribute (clearer than "update with no value"), and require an explicit --attr on retrieve rather than silently defaulting to 'owner'. Signed-off-by: Martin Belanger --- Documentation/nvme-registry-delete.txt | 38 +++++-- Documentation/nvme-registry-retrieve.txt | 25 ++--- Documentation/nvme-registry-update.txt | 36 +++---- plugins/registry/registry-nvme.c | 128 +++++++++++++++++------ 4 files changed, 147 insertions(+), 80 deletions(-) diff --git a/Documentation/nvme-registry-delete.txt b/Documentation/nvme-registry-delete.txt index 6f1bd32430..5f775a764b 100644 --- a/Documentation/nvme-registry-delete.txt +++ b/Documentation/nvme-registry-delete.txt @@ -3,30 +3,42 @@ nvme-registry-delete(1) NAME ---- -nvme-registry-delete - Remove a controller's ownership registry entry +nvme-registry-delete - Remove a controller's registry entry or attribute SYNOPSIS -------- [verse] -'nvme' [] 'registry delete' - --device= | -d +'nvme' [] 'registry delete' + [--attr= | -a ] DESCRIPTION ----------- -Remove the ownership registry entry for an NVMe-oF controller. The -registry entry is the directory /run/nvme/registry// and all -attribute files it contains. +Remove the ownership registry entry for an NVMe-oF controller, or a single +attribute from it. The registry entry is the directory +/run/nvme/registry// and all attribute files it contains. + +The is the NVMe controller name (e.g. nvme3), given as a +positional argument; the /dev/ prefix is optional. + +Without --attr the whole entry is removed. With --attr only that one +attribute file is removed, leaving the rest of the entry in place. Under normal operation registry entries are removed automatically: a udev rule fires on the kernel REMOVE event and deletes the entry. This command is provided for manual cleanup or for orchestrators that want to explicitly release ownership before disconnecting. +Removing the whole entry, or the 'owner' attribute specifically, drops +ownership and can stop an orchestrator from protecting the controller, so +it prompts for confirmation when run interactively; non-interactive callers +(scripts) proceed without prompting. Removing any other attribute does not +affect ownership and proceeds silently. + OPTIONS ------- --d :: ---device=:: - NVMe device name (e.g. nvme3). The /dev/ prefix is optional. +-a :: +--attr=:: + Attribute to remove. If omitted, the whole entry is removed. include::global-options.txt[] @@ -35,7 +47,13 @@ EXAMPLES * Remove the registry entry for nvme3: + ------------ -# nvme registry delete --device nvme3 +# nvme registry delete nvme3 +------------ + +* Remove a single attribute, leaving the rest of the entry: ++ +------------ +# nvme registry delete nvme3 --attr note ------------ SEE ALSO diff --git a/Documentation/nvme-registry-retrieve.txt b/Documentation/nvme-registry-retrieve.txt index 388fe42f10..fe15a136fa 100644 --- a/Documentation/nvme-registry-retrieve.txt +++ b/Documentation/nvme-registry-retrieve.txt @@ -8,9 +8,8 @@ nvme-registry-retrieve - Read an attribute from a controller's registry entry SYNOPSIS -------- [verse] -'nvme' [] 'registry retrieve' - --device= | -d - [--attr= | -a ] +'nvme' [] 'registry retrieve' + --attr= | -a DESCRIPTION ----------- @@ -19,33 +18,23 @@ controller. The registry is stored under /run/nvme/registry/ as one directory per live controller, with one plain-text file per attribute (e.g. /run/nvme/registry/nvme3/owner). -If --attr is omitted the 'owner' attribute is retrieved. +The is the NVMe controller name (e.g. nvme3), given as a +positional argument; the /dev/ prefix is optional. OPTIONS ------- --d :: ---device=:: - NVMe device name (e.g. nvme3). The /dev/ prefix is optional. - -a :: --attr=:: - Attribute name to retrieve. Defaults to 'owner'. + Attribute name to retrieve (e.g. owner, note). Required. include::global-options.txt[] EXAMPLES -------- -* Retrieve the owner of nvme3 (default attribute): -+ ------------- -# nvme registry retrieve --device nvme3 -stas ------------- - -* Retrieve a specific attribute: +* Retrieve the owner of nvme3: + ------------ -# nvme registry retrieve --device nvme3 --attr owner +# nvme registry retrieve nvme3 --attr owner stas ------------ diff --git a/Documentation/nvme-registry-update.txt b/Documentation/nvme-registry-update.txt index 5b722bbf76..c02f383bf7 100644 --- a/Documentation/nvme-registry-update.txt +++ b/Documentation/nvme-registry-update.txt @@ -8,36 +8,36 @@ nvme-registry-update - Write an attribute to a controller's registry entry SYNOPSIS -------- [verse] -'nvme' [] 'registry update' - --device= | -d +'nvme' [] 'registry update' --attr= | -a --value= | -V DESCRIPTION ----------- -Write a non-owner attribute to the ownership registry entry for an -NVMe-oF controller. The registry is stored under /run/nvme/registry/ as -one directory per live controller, with one plain-text file per -attribute. The entry directory is created if it does not already exist. +Write an attribute to the ownership registry entry for an NVMe-oF +controller. The registry is stored under /run/nvme/registry/ as one +directory per live controller, with one plain-text file per attribute. +The entry directory is created if it does not already exist. -The 'owner' attribute is immutable: it is recorded when the connection is -created (e.g. by 'nvme connect --owner') and cannot be changed by this -command. Attempting to update 'owner' is rejected. Use this command for -auxiliary attributes such as a free-form 'note'. +The is the NVMe controller name (e.g. nvme3), given as a +positional argument; the /dev/ prefix is optional. + +The 'owner' attribute is normally recorded when the connection is created +(e.g. by 'nvme connect --owner'). Changing it by hand can stop an +orchestrator from protecting the controller, so updating 'owner' prompts +for confirmation when run interactively; non-interactive callers (scripts) +proceed without prompting. This is a guard against accidental mistakes, +not access control -- a user with root can edit the files under +/run/nvme/registry/ directly. Writes are atomic: a temporary file is written and synced, then renamed into place, preventing corruption under concurrent access. OPTIONS ------- --d :: ---device=:: - NVMe device name (e.g. nvme3). The /dev/ prefix is optional. - -a :: --attr=:: - Attribute name to write (e.g. note). The reserved 'owner' - attribute is immutable and cannot be written with this command. + Attribute name to write (e.g. note, owner). -V :: --value=:: @@ -50,13 +50,13 @@ EXAMPLES * Attach a free-form note to nvme3: + ------------ -# nvme registry update --device nvme3 --attr note --value "maintenance 2026-07" +# nvme registry update nvme3 --attr note --value "maintenance 2026-07" ------------ * Update a previously written note: + ------------ -# nvme registry update --device nvme3 --attr note --value "released" +# nvme registry update nvme3 --attr note --value "released" ------------ SEE ALSO diff --git a/plugins/registry/registry-nvme.c b/plugins/registry/registry-nvme.c index 8603e65e2e..c3736265aa 100644 --- a/plugins/registry/registry-nvme.c +++ b/plugins/registry/registry-nvme.c @@ -6,6 +6,7 @@ * Authors: Martin Belanger */ #include +#include #include #include #include @@ -26,6 +27,47 @@ static void strip_dev_prefix(char **device) *device += 5; } +/* + * The device (a controller name like "nvme3", optionally given as + * "/dev/nvme3") is passed as a positional argument, consistent with the rest + * of the nvme CLI. Returns the bare controller name, or NULL if no positional + * argument was supplied. + */ +static char *get_device(int argc, char **argv) +{ + char *device; + + if (optind >= argc) + return NULL; + + device = argv[optind]; + strip_dev_prefix(&device); + return device; +} + +/* + * Warn before an operation that changes or removes ownership and ask the + * user to confirm. Returns true if the operation should proceed. A + * non-interactive caller (no controlling terminal) always proceeds -- + * scripting the command is itself the statement of intent. + */ +static bool confirm_owner_change(void) +{ + char ans[8] = { 0 }; + + if (!isatty(STDIN_FILENO)) + return true; + + fprintf(stderr, + "Changing or removing the owner may prevent NVMe orchestrators from\n" + "protecting this controller against accidental removal. Continue? [y/N]: "); + + if (!fgets(ans, sizeof(ans), stdin)) + return false; + + return ans[0] == 'y' || ans[0] == 'Y'; +} + static void print_attr(const char *attr, const char *value, void *user_data) { printf(" %-12s %s\n", attr, value); @@ -58,37 +100,39 @@ static int registry_retrieve(int argc, char **argv, struct command *acmd, { __cleanup_nvme_global_ctx struct libnvme_global_ctx *ctx = NULL; const char *desc = "Read an attribute from a controller's registry entry."; - const char *device_help = "NVMe device name (e.g. nvme3)"; - const char *attr_help = "attribute name (default: owner)"; + const char *attr_help = "attribute name (e.g. owner, note)"; __cleanup_free char *value = NULL; + char *device; int ret; struct config { - char *device; char *attr; }; - struct config cfg = { .attr = "owner" }; + struct config cfg = { 0 }; NVME_ARGS(opts, - OPT_STRING("device", 'd', "DEV", &cfg.device, device_help), - OPT_STRING("attr", 'a', "ATTR", &cfg.attr, attr_help)); + OPT_STRING("attr", 'a', "ATTR", &cfg.attr, attr_help)); ret = argconfig_parse(argc, argv, desc, opts); if (ret) return ret; - if (!cfg.device) { - fprintf(stderr, "--device required\n"); + device = get_device(argc, argv); + if (!device) { + fprintf(stderr, "device required\n"); + return -EINVAL; + } + if (!cfg.attr) { + fprintf(stderr, "--attr required\n"); return -EINVAL; } - strip_dev_prefix(&cfg.device); ctx = libnvme_create_global_ctx(stdout, LIBNVME_DEFAULT_LOGLEVEL); - ret = libnvmf_registry_retrieve(ctx, cfg.device, cfg.attr, &value); + ret = libnvmf_registry_retrieve(ctx, device, cfg.attr, &value); if (ret == -ENOENT) { fprintf(stderr, "%s: not registered or '%s' not found\n", - cfg.device, cfg.attr); + device, cfg.attr); return ret; } if (ret) { @@ -103,14 +147,13 @@ static int registry_update(int argc, char **argv, struct command *acmd, struct plugin *plugin) { __cleanup_nvme_global_ctx struct libnvme_global_ctx *ctx = NULL; - const char *desc = "Write an attribute to a controller's registry entry. The 'owner' attribute is immutable and cannot be changed."; - const char *device_help = "NVMe device name (e.g. nvme3)"; - const char *attr_help = "attribute name (e.g. note); 'owner' is not allowed"; + const char *desc = "Write an attribute to a controller's registry entry."; + const char *attr_help = "attribute name (e.g. note, owner)"; const char *value_help = "new attribute value"; + char *device; int ret; struct config { - char *device; char *attr; char *value; }; @@ -118,28 +161,30 @@ static int registry_update(int argc, char **argv, struct command *acmd, struct config cfg = { 0 }; NVME_ARGS(opts, - OPT_STRING("device", 'd', "DEV", &cfg.device, device_help), - OPT_STRING("attr", 'a', "ATTR", &cfg.attr, attr_help), - OPT_STRING("value", 'V', "VAL", &cfg.value, value_help)); + OPT_STRING("attr", 'a', "ATTR", &cfg.attr, attr_help), + OPT_STRING("value", 'V', "VAL", &cfg.value, value_help)); ret = argconfig_parse(argc, argv, desc, opts); if (ret) return ret; - if (!cfg.device || !cfg.attr || !cfg.value) { - fprintf(stderr, "--device, --attr and --value are required\n"); + device = get_device(argc, argv); + if (!device) { + fprintf(stderr, "device required\n"); return -EINVAL; } - - if (!strcmp(cfg.attr, "owner")) { - fprintf(stderr, "the 'owner' attribute is immutable and cannot be changed via this command\n"); - return -EPERM; + if (!cfg.attr || !cfg.value) { + fprintf(stderr, "--attr and --value are required\n"); + return -EINVAL; } - strip_dev_prefix(&cfg.device); + if (!strcmp(cfg.attr, "owner") && !confirm_owner_change()) { + fprintf(stderr, "Aborted.\n"); + return 0; + } ctx = libnvme_create_global_ctx(stdout, LIBNVME_DEFAULT_LOGLEVEL); - ret = libnvmf_registry_update(ctx, cfg.device, cfg.attr, cfg.value); + ret = libnvmf_registry_update(ctx, device, cfg.attr, cfg.value); if (ret) fprintf(stderr, "update failed: %s\n", libnvme_strerror(-ret)); return ret; @@ -150,31 +195,46 @@ static int registry_delete(int argc, char **argv, struct command *acmd, { __cleanup_nvme_global_ctx struct libnvme_global_ctx *ctx = NULL; const char *desc = "Remove a controller's registry entry."; - const char *device_help = "NVMe device name (e.g. nvme3)"; + const char *attr_help = "attribute to remove (default: whole entry)"; + char *device; int ret; struct config { - char *device; + char *attr; }; struct config cfg = { 0 }; NVME_ARGS(opts, - OPT_STRING("device", 'd', "DEV", &cfg.device, device_help)); + OPT_STRING("attr", 'a', "ATTR", &cfg.attr, attr_help)); ret = argconfig_parse(argc, argv, desc, opts); if (ret) return ret; - if (!cfg.device) { - fprintf(stderr, "--device required\n"); + device = get_device(argc, argv); + if (!device) { + fprintf(stderr, "device required\n"); return -EINVAL; } - strip_dev_prefix(&cfg.device); + + /* + * Removing the whole entry, or the owner attribute specifically, drops + * ownership -- confirm first. Removing any other attribute does not + * affect ownership and proceeds silently. + */ + if ((!cfg.attr || !strcmp(cfg.attr, "owner")) && + !confirm_owner_change()) { + fprintf(stderr, "Aborted.\n"); + return 0; + } ctx = libnvme_create_global_ctx(stdout, LIBNVME_DEFAULT_LOGLEVEL); - ret = libnvmf_registry_delete(ctx, cfg.device); + if (cfg.attr) + ret = libnvmf_registry_update(ctx, device, cfg.attr, NULL); + else + ret = libnvmf_registry_delete(ctx, device); if (ret) - fprintf(stderr, "%s: %s\n", cfg.device, libnvme_strerror(-ret)); + fprintf(stderr, "%s: %s\n", device, libnvme_strerror(-ret)); return ret; }