From: Nilay Shroff <nilay@linux.ibm.com>
To: Daniel Wagner <dwagner@suse.de>
Cc: "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
Keith Busch <kbusch@kernel.org>, Christoph Hellwig <hch@lst.de>,
Sagi Grimberg <sagi@grimberg.me>, Gregory Joyce <gjoyce@ibm.com>,
Srimannarayana Murthy Maram <msmurthy@imap.linux.ibm.com>,
"axboe@fb.com" <axboe@fb.com>
Subject: Re: nvme-cli : attaching a namespace to an undiscovered nvme controller on multi-controller nvme disk
Date: Tue, 7 May 2024 17:14:40 +0530 [thread overview]
Message-ID: <dfb2488a-6275-4822-94ec-07a575cbf09c@linux.ibm.com> (raw)
In-Reply-To: <yhvmfctxhfzrcsq4lx6nr3f5n67kegrixtthh3pals7vn5qr26@k5as5whcdrum>
On 5/6/24 20:35, Daniel Wagner wrote:
> On Mon, May 06, 2024 at 07:45:20PM GMT, Nilay Shroff wrote:
>> There could be multiple ways to address this issue. However my proposal would be to address
>> it in nvme-cli. As nvme-cli builds the nvme topology it shall be easy
>> for nvme-cli to detect
>
> The topology scan in libnvme is only using the information available in
> sysfs. libnvme doesn't issue any commands anymore and I'd like to keep
> it this in this way. So if the kernel doesn't exposes this information
> to userspace via sysfs, I don't think it's simple to 'fix' this in
> nvme-cli/libnvme.
>
I think the information which we need to contain this issue is already
available through sysfs. If we scan nvme topology then we could find
the controller id assigned to each controller under each nvme subsystem.
We can then leverage this information to figure out whether each controller
id specified in the attach-ns command is valid or not. So essentially we
match controller id specified in attach-ns command against the controller id
learnt through scanning the topology. If we find that any discrepancy then we
can show the WARNING to the user. I have worked out a patch using this method.
I have attached the patch below for suggestion/feedback.
diff --git a/nvme.c b/nvme.c
index c1d4352a..533cc390 100644
--- a/nvme.c
+++ b/nvme.c
@@ -2784,6 +2784,23 @@ static int delete_ns(int argc, char **argv, struct command *cmd, struct plugin *
return err;
}
+static bool nvme_match_subsys_device_filter(nvme_subsystem_t s, nvme_ctrl_t c,
+ nvme_ns_t ns, void *f_arg)
+{
+ nvme_ctrl_t _c;
+ const char *devname = (const char *)f_arg;
+
+ if (s) {
+ nvme_subsystem_for_each_ctrl(s, _c) {
+ if (!strcmp(devname, nvme_ctrl_get_name(_c)))
+ return true;
+ }
+ return false;
+ }
+
+ return true;
+}
+
static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, struct command *cmd)
{
_cleanup_free_ struct nvme_ctrl_list *cntlist = NULL;
@@ -2839,12 +2856,68 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
nvme_init_ctrl_list(cntlist, num, ctrlist);
- if (attach)
+ if (attach) {
+ const char *cntlid;
+ int __cntlid;
+ char *p;
+ nvme_host_t h;
+ nvme_subsystem_t s;
+ nvme_ctrl_t c;
+ nvme_root_t r = NULL;
+ int matched = 0;
+ nvme_scan_filter_t filter = nvme_match_subsys_device_filter;
+
+ r = nvme_create_root(stderr, log_level);
+ if (!r) {
+ nvme_show_error("Failed to create topology root: %s",
+ nvme_strerror(errno));
+ return -errno;
+ }
+
+ err = nvme_scan_topology(r, filter, (void *)dev->name);
+ if (err < 0) {
+ if (errno != ENOENT)
+ nvme_show_error("Failed to scan topology: %s",
+ nvme_strerror(errno));
+ nvme_free_tree(r);
+ return err;
+ }
+ nvme_for_each_host(r, h) {
+ nvme_for_each_subsystem(h, s) {
+ nvme_subsystem_for_each_ctrl(s, c) {
+ cntlid = nvme_ctrl_get_cntlid(c);
+ errno = 0;
+ __cntlid = strtoul(cntlid, &p, 0);
+ if (errno || *p != 0)
+ continue;
+ for (i = 0; i < num; i++) {
+ if (__cntlid == list[i])
+ matched++;
+ }
+ }
+ }
+ }
+
+ nvme_free_tree(r);
+
+ if (matched != num) {
+ fprintf(stderr,
+ "You are about to attach namespace 0x%x to an undiscovered nvme controller.\n",
+ cfg.namespace_id);
+ fprintf(stderr,
+ "WARNING: Attaching nampespace to undiscovered nvme controller may have undesired side effect!\n"
+ "You may not be able to perform any IO to such namespace.\n"
+ "You have 10 seconds to press Ctrl-C to cancel this operation.\n\n");
+ sleep(10);
+ fprintf(stderr, "Sending attach-ns operation ...\n");
+ }
+
err = nvme_cli_ns_attach_ctrls(dev, cfg.namespace_id,
cntlist);
- else
+ } else {
err = nvme_cli_ns_detach_ctrls(dev, cfg.namespace_id,
cntlist);
+ }
if (!err)
printf("%s: Success, nsid:%d\n", cmd->name, cfg.namespace_id);
Thanks,
--Nilay
next prev parent reply other threads:[~2024-05-07 11:45 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-06 14:15 nvme-cli : attaching a namespace to an undiscovered nvme controller on multi-controller nvme disk Nilay Shroff
2024-05-06 15:05 ` Daniel Wagner
2024-05-07 11:44 ` Nilay Shroff [this message]
2024-05-16 6:25 ` Nilay Shroff
2024-05-27 11:20 ` Nilay Shroff
2024-05-27 13:36 ` Daniel Wagner
2024-05-27 14:02 ` Nilay Shroff
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dfb2488a-6275-4822-94ec-07a575cbf09c@linux.ibm.com \
--to=nilay@linux.ibm.com \
--cc=axboe@fb.com \
--cc=dwagner@suse.de \
--cc=gjoyce@ibm.com \
--cc=hch@lst.de \
--cc=kbusch@kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=msmurthy@imap.linux.ibm.com \
--cc=sagi@grimberg.me \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).