tree: add windows tree support by bcapener · Pull Request #3440 · linux-nvme/nvme-cli

bcapener · 2026-06-09T21:14:44Z

Add tree.c and lib.c support. Adds ctrl-map.c for mapping controller name, nvmeX, to controller path, since windows controller paths might not fit within the d_name field of the dirent struct. The ctrl-map also creates a subsystem name to replicate linux's nvme-subsysX naming.

igaw · 2026-06-16T17:41:59Z

The patch is too big to be properly reviewed. Can you at least split into two parts, the first one refactoring the existing code before adding new code?

bcapener · 2026-06-16T23:46:34Z

Good idea. Sorry, I could not think of a better way to split this up. The refactoring of the existing code can be found here #3450 , I'll use the PR for adding the new code.

igaw · 2026-06-17T07:40:01Z

I've merged #3450 so you can rebase this PR :)

Add tree.c and lib.c support. Adds ctrl-map.c for mapping controller name, nvmeX, to controller path, since windows controller paths might not fit within the d_name field of the dirent struct. The ctrl-map also creates a subsystem name to replicate linux's nvme-subsysX naming. Signed-off-by: Brandon Capener <bcapener@micron.com>

bcapener · 2026-06-17T16:08:52Z

Thanks Daniel, I have rebased this PR with upstream changes.

igaw · 2026-06-18T09:24:33Z

+#include <stdlib.h>
+#include <string.h>
+#include <wchar.h>
+#include "private-ctrl-map.h"


this should be grouped with the other local headers below

igaw · 2026-06-18T09:26:36Z

+	size_t count;
+	size_t capacity;
+	bool initialized;
+} ctrl_map;


Why not using the ccan hash table here? It should work for the use case here as far I can tell.

igaw · 2026-06-18T09:30:21Z

+ * Creates a temporary transport context and handle internally.
+ * Returns 0 on success with id_ctrl filled in, negative errno on failure.
+ */
+static int identify_ctrl_from_handle(HANDLE h, struct nvme_id_ctrl *id_ctrl)


I would suggest to use the libnvme typedefs here, because it looks a bit strange below when assiging it to hdl->fd = h.

igaw · 2026-06-18T09:33:13Z

+	struct libnvme_passthru_cmd cmd;
+	int ret;
+
+	ctx = libnvme_create_global_ctx(NULL, LIBNVME_DEFAULT_LOGLEVEL);


A don't think this should be here. Creating inside the library a global context is not correct.

igaw · 2026-06-18T09:41:26Z

+
+	*ctrls = NULL;
+
+	libnvme_ctrl_map_clear();


I see why you had to create global context hidden inside the ctrl map code. I suppose we should look how to extend the scan API in a sane way.

igaw · 2026-06-18T09:47:15Z

+	if (hostnqn)
+		hnqn = strdup(hostnqn);
+	else
+		hnqn = strdup("nqn.2014-08.org.nvmexpress:uuid:00000000-0000-0000-0000-000000000000");


Doesn't Microsoft have a default UUID? Should be we just use one instead providing an NULL one?

igaw · 2026-06-18T09:55:56Z


 	s->model = libnvme_get_attr(path, "model");
-	if (!s->model)
+	if (path && !s->model)


This whole hunk doesn't look right. What does it try to do?

All of these changes in tree.c (including the other comment below) were our attempt to avoid using #ifndef _WIN32, instead we check if the sysfs_dir is not NULL, so that code will only run on linux and not windows.

if (path && !s->model) s->model = strdup("undefined");

here we don't want s->model set to "undefined" on windows, since it will be set somewhere else, that is why we added the path check.

igaw · 2026-06-18T09:57:00Z

+		subsysnqn = libnvme_get_attr(path, "subsysnqn");
+		if (!subsysnqn)
+			return -ENODEV;
+	}


same here, what does it try to fix and if so, it should definitely be a separate patch with a proper commit message.

see other comment #3440 (comment)

Copilot

Pull request overview

This PR adds Windows support for libnvme’s “tree” enumeration and controller/namespace discovery by introducing a Windows controller mapping layer and wiring it into scanning/open paths. It extends the Windows build to link against SetupAPI/ConfigMgr32 and implements Windows-specific controller/namespace enumeration and attribute population.

Changes:

Add a Windows controller map (ctrl-map.c) to translate nvmeX naming into Windows device paths and provide subsystem naming analogous to Linux (nvme-subsysX).
Implement Windows controller/namespace scanning and open logic (tree-win.c, filters-win.c, lib-win.c) using the controller map.
Update Meson build files to compile/link Windows-specific sources and dependencies.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
meson.build	Adds Windows dependency detection for `setupapi` and `cfgmgr32`.
libnvme/src/meson.build	Builds `nvme/ctrl-map.c` on Windows and links `setupapi`/`cfgmgr32`.
libnvme/src/nvme/tree.c	Adjusts subsystem init/scan to tolerate missing sysfs dir (Windows).
libnvme/src/nvme/tree-win.c	Implements Windows controller reconfigure, host lookup, ctrl scan, and namespace open/init paths.
libnvme/src/nvme/lib-win.c	Implements Windows transport handle open/close and nvmeX<->PhysicalDrive mapping in `libnvme_open`.
libnvme/src/nvme/filters-win.c	Implements Windows controller and namespace enumeration using the controller map.
libnvme/src/nvme/private-ctrl-map.h	Adds Windows-only controller map API.
libnvme/src/nvme/ctrl-map.c	New Windows controller mapping implementation based on SetupAPI/CM APIs and Identify.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

 	libnvme_msg(ctx, LIBNVME_LOG_DEBUG, "scan subsystem %s\n", name);
-	ret = asprintf(&path, "%s/%s", libnvme_subsys_sysfs_dir(), name);
-	if (ret < 0)
-		return -ENOMEM;
+	if (sysfs_dir) {
+		ret = asprintf(&path, "%s/%s", sysfs_dir, name);
+		if (ret < 0)
+			return -ENOMEM;


+	/* Handle test devices */
+	if (!strncmp(name, "NVME_TEST_FD", 12)) {
+		hdl->type = LIBNVME_TRANSPORT_HANDLE_TYPE_DIRECT;
+		hdl->fd = LIBNVME_TEST_FD;
+
+		if (!strcmp(name, "NVME_TEST_FD64"))
+			hdl->ioctl_admin_state = IOCTL_STATE_IOCTL64;
+
+		*hdlp = hdl;
+		return 0;
+	}


+	/* Store the nvmeX or nvmeXnY-style name in hdl->name. */
+	hdl->name = strdup(name);
+	if (!hdl->name) {
+		free(hdl);
+		return -ENOMEM;
+	}


+	char *state = c->state;
+
+	c->state = strdup("");
+	free(state);
+	return c->state;


+	c->hdl = NULL;
+	c->name = xstrdup(name);
+	c->sysfs_dir = xstrdup(path);
+
+	if (!libnvme_ctrl_get_transport_handle(c))
+		return -ENODEV;


+	n->sysfs_dir = strdup(name); /* \\\\.\\PhysicalDriveX */
+
+	*ns = n;
+	return 0;


igaw · 2026-06-19T15:58:07Z

BTW, I'll try to look into the scan API issue next week.

bcapener marked this pull request as ready for review June 10, 2026 20:04

bcapener marked this pull request as draft June 17, 2026 15:32

bcapener force-pushed the bcapener/add-tree-support branch from 0b4d062 to 9f6484f Compare June 17, 2026 15:54

bcapener marked this pull request as ready for review June 17, 2026 16:06

igaw reviewed Jun 18, 2026

View reviewed changes

Comment thread libnvme/src/nvme/lib-win.c

igaw reviewed Jun 18, 2026

View reviewed changes

igaw requested a review from Copilot June 18, 2026 10:59

Copilot started reviewing on behalf of igaw June 18, 2026 10:59 View session

Copilot AI reviewed Jun 18, 2026

View reviewed changes

Conversation

bcapener commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igaw commented Jun 16, 2026

Uh oh!

bcapener commented Jun 16, 2026

Uh oh!

igaw commented Jun 17, 2026

Uh oh!

bcapener commented Jun 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

igaw commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bcapener commented Jun 9, 2026 •

edited

Loading