Zpool: add NetBSD support #2051

uzuki314 · 2025-11-13T12:37:09Z

This PR adds NetBSD support to the zpool module.

CarterLi · 2025-11-13T13:09:46Z

Thank you for your contribution. I wonder why the zfs header is so different in NetBSD.

uzuki314 · 2025-11-13T13:50:16Z

Actually I'm not that familiar with NetBSD. I just tried multibooting Gentoo, FreeBSD, and NetBSD recently, and was surprised to find that NetBSD supports ZFS — so I managed to make them all boot from the same zpool.

As for your question, I tried to do some research: NetBSD’s ZFS implementation was originally ported from an older Solaris version (around the early OpenZFS days in 2009), and later rebased on FreeBSD’s code in 2018. This work was done before the OpenZFS and ZFS on Linux codebases were merged into OpenZFS 2.0 in 2020, so some parts of NetBSD’s implementation might still reflect the pre-merge structure. That might explain why some of the headers look a bit different. Because of that, it took me an afternoon to hunt down all the required types and definitions @@

reference:
https://en.wikipedia.org/wiki/NetBSD
https://wiki.netbsd.org/zfs/

CarterLi · 2025-11-13T14:51:07Z

Your code adds a lot of complexity, which I don't really like. Can we just don't use dlopen just like what I did for SunOS?

uzuki314 · 2025-11-13T16:02:54Z

I followed your style and simplified the code this time.

Architectures
ZFS is built by default on amd64 and aarch64 on netbsd-9. In 10 and current, it is also built by default on sparc64. Using it on other architectures requires a full release build with MKZFS=yes.

So while I set ENABLE_LIBZFS by default for NetBSD in CMakeLists, it might not be available on all architectures.
Do you think we should still keep it enabled by default for NetBSD, or make it opt-in instead?

CarterLi · 2025-11-13T16:06:35Z

src/detection/zpool/libzfs_simplified_nbsd.h

+
+// From https://github.com/NetBSD/src/blob/trunk/external/cddl/osnet/dist/uts/common/sys/nvpair.h
+
+extern nvlist_t *opensolaris_fnvlist_lookup_nvlist(nvlist_t *, const char *);


Does the following code work?

extern nvlist_t *fnvlist_lookup_nvlist(nvlist_t *, const char *) __attribute__(("opensolaris_fnvlist_lookup_nvlist"));

Not work, maybe just use macro.

Or make an inline wrapper?

Sorry, it was a typo. It should be __attribute__((alias("opensolaris_fnvlist_lookup_nvlist")));

CarterLi · 2025-11-13T16:07:07Z

src/detection/zpool/libzfs_simplified_nbsd.h

+
+extern nvlist_t *opensolaris_fnvlist_lookup_nvlist(nvlist_t *, const char *);
+extern int opensolaris_nvlist_lookup_uint64_array(nvlist_t *, const char *, uint64_t **, uint_t *);
+extern int opensolaris_nvlist_lookup_uint32_array(nvlist_t *, const char *, uint32_t **, uint_t *);


Can we merge this file into libzfs_simplified.h?

Yes. Aside from zpool_prop_t (which differs), the rest can be shared.

CarterLi · 2025-11-13T16:08:33Z

src/detection/zpool/zpool_linux.c

+        const char *str;
+        zpool_status_t status = zpool_get_status(zpool, (char**) &str);
+        if (status == ZPOOL_STATUS_IO_FAILURE_WAIT ||
+            status == ZPOOL_STATUS_IO_FAILURE_CONTINUE)


Just use #ifdef ZPOOL_STATUS_IO_FAILURE_MMP in __sun part.

CarterLi · 2025-11-13T16:38:38Z

I followed your style and simplified the code this time.

Architectures
ZFS is built by default on amd64 and aarch64 on netbsd-9. In 10 and current, it is also built by default on sparc64. Using it on other architectures requires a full release build with MKZFS=yes.

So while I set ENABLE_LIBZFS by default for NetBSD in CMakeLists, it might not be available on all architectures.
Do you think we should still keep it enabled by default for NetBSD, or make it opt-in instead?

Better to check the existance of libzfs.so in CMake.

uzuki314 · 2025-11-13T19:10:17Z

Merge libzfs_simplified_netbsd.h into libzfs_simplified.h, add inline wrappers to translate the opensolaris_* symbols, and reuse zpool_get_state_str.

Copilot

Pull Request Overview

This PR adds NetBSD support to the zpool module by integrating NetBSD's ZFS implementation alongside existing support for Linux, FreeBSD, and Solaris.

Key Changes:

Enabled libzfs support for NetBSD in the CMake configuration
Reorganized conditional compilation to support NetBSD's libzfs API
Added NetBSD-specific type definitions, enums, and wrapper functions for OpenSolaris compatibility layer

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
CMakeLists.txt	Added NetBSD to ENABLE_LIBZFS option and conditionally links zfs/nvpair libraries for NetBSD builds
src/detection/zpool/zpool_linux.c	Reorganized header includes and extended FF_DISABLE_DLOPEN/zpool_get_state_str to support NetBSD alongside Solaris
src/detection/zpool/libzfs_simplified.h	Added NetBSD-specific type definitions, enums, structs, and inline wrapper functions to interface with NetBSD's OpenSolaris compatibility layer

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

Copilot · 2025-11-14T01:21:34Z

src/detection/zpool/zpool_linux.c

+        #else
+        zpool_status_t status = zpool_get_status(zpool, (char**) &str);
+        if (
+        #endif
+            status == ZPOOL_STATUS_IO_FAILURE_WAIT ||
+            status == ZPOOL_STATUS_IO_FAILURE_CONTINUE)
            return "SUSPENDED";


[nitpick] The conditional check if (status == ZPOOL_STATUS_IO_FAILURE_MMP || for __sun is split awkwardly across lines 24 and 27-28. The if ( on line 27 for NetBSD creates an incomplete condition since the || operator is placed on line 24 (Solaris-only). This makes the code harder to read and maintain. Consider restructuring to make each platform's condition more self-contained.

Suggested change

#else

zpool_status_t status = zpool_get_status(zpool, (char**) &str);

if (

#endif

status == ZPOOL_STATUS_IO_FAILURE_WAIT ||

status == ZPOOL_STATUS_IO_FAILURE_CONTINUE)

return "SUSPENDED";

status == ZPOOL_STATUS_IO_FAILURE_WAIT ||

status == ZPOOL_STATUS_IO_FAILURE_CONTINUE)

return "SUSPENDED";

#else

zpool_status_t status = zpool_get_status(zpool, (char**) &str);

if (status == ZPOOL_STATUS_IO_FAILURE_WAIT ||

status == ZPOOL_STATUS_IO_FAILURE_CONTINUE)

return "SUSPENDED";

#endif

Copilot · 2025-11-14T01:21:35Z

src/detection/zpool/libzfs_simplified.h

-typedef bool boolean_t;
+#ifdef __NetBSD__
+
+#define	ZPOOL_CONFIG_VDEV_TREE "vdev_tree"


[nitpick] Inconsistent whitespace between macro name and value. Line 94 uses a tab character while line 95 uses a space. For consistency, both should use the same whitespace (preferably a single space).

Suggested change

#define ZPOOL_CONFIG_VDEV_TREE "vdev_tree"

#define ZPOOL_CONFIG_VDEV_TREE "vdev_tree"

CarterLi · 2025-11-14T02:06:28Z

src/detection/zpool/libzfs_simplified.h

    ZPOOL_PROP_ALLOCATED,
    ZPOOL_PROP_READONLY,
+#ifndef __NetBSD__
    ZPOOL_PROP_ASHIFT,


This is an ABI break change and won't work in future zfs versions.

* Pool properties are identified by these constants and must be added to the * end of this list to ensure that external consumers are not affected * by the change.

Bro...

Bro... 🥺
The concern about ABI stability is valid — but the thing is, libzfs is not a stable interface in the first place.
As noted here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=257222#c1

libzfs symbols are explicitly not guaranteed to be stable, the same zpool shows different fragmentation values on Gentoo and FreeBSD, and even within the same OS family (e.g., FreeBSD → TrueNAS), the ABI breaks frequently, including changes in internal struct layouts and symbol visibility.

So saying “this breaks ABI” is somewhat moot, because the whole module already relies on an ABI that upstream ZFS developers do not intend to keep stable.

Regarding NetBSD being niche — I agree it’s not a mainstream platform, but that's precisely why a simple, command-output-based fallback (zpool get -Hp ...) is a safer option than maintaining a separate set of libzfs headers for each OS.

If the goal is long-term maintainability, relying on zpool output (which is stable across Linux/FreeBSD/NetBSD/illumos(sunos)) is arguably more robust than relying on libzfs internals.

If full libzfs support is considered too fragile or high-maintenance, maybe it’s worth discussing whether the zpool module should:

use libzfs only where it’s reliable, and

fall back to parsing the CLI elsewhere, or

rely entirely on CLI output for consistency across platforms.

I’m not advocating removing the module — ZFS users exist on all these platforms — but the current situation already depends on unstable APIs, and a CLI-based approach avoids exactly the ABI issues being pointed out.

libzfs symbols are explicitly not guaranteed to be stable

I know ABI break change can be useful sometimes, but this breakage is completely useless and extremely dangerous. ZPOOL_PROP_FRAGMENTATION will be silently increased from 22 to 23, just because of they added a new entry at the MIDDLE of an enum. The change is ALWAYS wrong if they expose their enum outside the ABI boundry, NEVER makes sence and every C programmer should know it. They knew it actually (hence the comments). Your program won't crash, it will just query a wrong property and generate incorrect results.

CarterLi · 2025-11-14T02:28:58Z

I still don't like the change. I think the zfs code of NetBSD port is full of mess and I really don't want it to be embeded into fastfetch's repo. I would like just leave item->state empty.

CarterLi · 2025-11-14T02:31:09Z

NetBSD itself is a niche system and NetBSD don't use zfs by default either. I don't really care about zfs support on NetBSD, to be honest.

uzuki314 · 2025-11-14T03:28:30Z

How about implementing it by just parsing the output of zpool get -Hp version,size,free,fragmentation,health?

CarterLi · 2025-11-14T05:14:15Z

How about implementing it by just parsing the output of zpool get -Hp version,size,free,fragmentation,health?

Just use the Command module then.

Signed-off-by: hoshinomori <hoshinomori@owarisekai.moe>

uzuki314 force-pushed the dev branch from c278fec to d98c26d Compare November 13, 2025 15:44

CarterLi reviewed Nov 13, 2025

View reviewed changes

uzuki314 force-pushed the dev branch from d98c26d to 0168627 Compare November 13, 2025 18:47

CarterLi requested a review from Copilot November 14, 2025 01:17

Copilot started reviewing on behalf of CarterLi November 14, 2025 01:17 View session

Copilot finished reviewing on behalf of CarterLi November 14, 2025 01:21

Copilot AI reviewed Nov 14, 2025

View reviewed changes

CarterLi reviewed Nov 14, 2025

View reviewed changes

Zpool: add NetBSD support

e356454

Signed-off-by: hoshinomori <hoshinomori@owarisekai.moe>

uzuki314 force-pushed the dev branch from 0168627 to e356454 Compare November 15, 2025 21:26


		// From https://github.com/NetBSD/src/blob/trunk/external/cddl/osnet/dist/uts/common/sys/nvpair.h

		extern nvlist_t opensolaris_fnvlist_lookup_nvlist(nvlist_t , const char *);

	#define ZPOOL_CONFIG_VDEV_TREE "vdev_tree"
	#define ZPOOL_CONFIG_VDEV_TREE "vdev_tree"

Uh oh!

Zpool: add NetBSD support #2051

Are you sure you want to change the base?

Zpool: add NetBSD support #2051

Conversation

uzuki314 commented Nov 13, 2025

Uh oh!

CarterLi commented Nov 13, 2025

Uh oh!

uzuki314 commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CarterLi commented Nov 13, 2025

Uh oh!

uzuki314 commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CarterLi commented Nov 13, 2025

Uh oh!

uzuki314 commented Nov 13, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

CarterLi Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

CarterLi commented Nov 14, 2025

Uh oh!

CarterLi commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uzuki314 commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CarterLi commented Nov 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

uzuki314 commented Nov 13, 2025 •

edited

Loading

uzuki314 commented Nov 13, 2025 •

edited

Loading

CarterLi Nov 14, 2025 •

edited

Loading

CarterLi commented Nov 14, 2025 •

edited

Loading

uzuki314 commented Nov 14, 2025 •

edited

Loading