Skip to content

build: make bootfs depend on the ZFS library for any ZFS build, not only fs=zfs - #1494

Closed
gburd wants to merge 1 commit into
cloudius-systems:masterfrom
gburd:pr/bootfs-zfs-dep
Closed

gburd wants to merge 1 commit into
cloudius-systems:masterfrom
gburd:pr/bootfs-zfs-dep

Conversation

@gburd

@gburd gburd commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A ZFS-enabled build whose root filesystem is not zfs has no rule ordering the ZFS
library and tools before MKBOOTFS runs, so the bootfs manifest can reference files
that have not been built yet. bootfs_dep listed them only under fs=zfs:

ifeq ($(fs),ext)
bootfs_dep += $(out)/modules/libext/libext.so
else
ifeq ($(fs),zfs)
bootfs_dep += $(tools:%=$(out)/%) $(out)/libsolaris.so
endif
endif

The case that matters in practice is conf_zfs=openzfs fs=ramfs with a ZFS pool
created at runtime, which is a normal configuration. For fs=ramfs,
scripts/build (line 224) sets manifest=$OUT/usr.manifest, i.e. the bootfs
manifest IS usr.manifest and bootfs.manifest.skel is not used at all. The ZFS
module contributes /usr/lib/fs/libsolaris.so to that manifest, so bootfs needs
libsolaris.so and the userspace ZFS tools built first, yet nothing said so.

Serially that fails outright on the missing file; under -j it is a race that
surfaces as an intermittent MKBOOTFS break on libsolaris.so or one of the
userspace ZFS libraries. It is why building such an image has required building the
ZFS shared objects by hand first.

The change

Key the dependency on the ZFS build itself (conf_zfs_openzfs) rather than on the
root filesystem choice.

Verified behaviour

Evaluating the conditional for each configuration, before and after:

configuration before after
conf_zfs=openzfs fs=ramfs libsolaris.so and tools absent present
conf_zfs=openzfs fs=zfs present present, unchanged
conf_zfs=openzfs fs=ext libext.so libext.so, unchanged
no ZFS, fs=ramfs nothing added nothing added, unchanged

Only the broken configuration changes; fs=zfs, fs=ext and non-ZFS builds keep
their existing dependencies.

One file.

Note on a separate, independent bug

While validating this we found a second and distinct defect at the same site, which
this PR does not address: passing open_zfs explicitly in the image list
shadows the zfs placeholder module. resolve.require() registers open_zfs's
provides=['zfs'] under the name zfs, so modules/zfs/module.py is never
imported and the /usr/lib/fs/libsolaris.so manifest line it owns never reaches
usr.manifest at all. Measured:

image list modules loaded libsolaris.so in usr.manifest
zfs,zfs-tools open_zfs, zfs, zfs-tools present
open_zfs,zfs-tools open_zfs, zfs-tools absent
open_zfs,zfs,zfs-tools open_zfs, zfs-tools absent

On fs=ramfs that is fatal rather than a race: the guest dies at "Failed to
preload ZFS library. Powering off." The same shadowing applies to bsd_zfs. That
deserves its own fix in the module-resolution layer and is not in scope here.

…nly fs=zfs

bootfs.manifest.skel names /usr/lib/fs/libsolaris.so unconditionally, so the ZFS
library is part of bootfs for every root filesystem. But bootfs_dep only listed
it (and the tools that accompany it) as a prerequisite when fs=zfs, so a
ZFS-enabled image with a different root filesystem had the file in its manifest
without a rule ordering its build first.

Serially that fails outright on the missing file; under -j it is a race that
surfaces as an intermittent MKBOOTFS break on libsolaris.so or one of the
userspace ZFS libraries. A conf_zfs=openzfs image with fs=ramfs and a ZFS pool
created at runtime is a normal configuration and hits this every time, which is
why such builds have needed the ZFS shared objects built by hand first.

Key the dependency on the ZFS build (conf_zfs_openzfs) instead of on the root
filesystem choice. fs=zfs and fs=ext keep their existing dependencies, and a
build with no ZFS at all still adds nothing.
@gburd

gburd commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Closing this in favour of #1514, which fixes the same bug correctly.

I tested this patch rather than reasoning about it, and it does not fix the failure it cites. The new branch is guarded by ifdef conf_zfs_openzfs, but conf_zfs defaults to bsd (Makefile:51, conf_zfs ?= bsd), and conf_zfs=bsd also builds libsolaris.so - the rule at Makefile:2566 is not conditional on the ZFS provider. So in the default configuration the guard cannot fire, and the reproducer in this PR's own description still fails with the patch applied.

The underlying bug is real and slightly broader than described here: bootfs.manifest.skel names /usr/lib/fs/libsolaris.so unconditionally, while bootfs_dep only supplied it for fs=zfs. So any invocation with a different fs, or with fs unset, runs mkbootfs.py before the library exists - a hard failure serially, and a race under -j. It is masked in normal use because scripts/build always passes fs=zfs.

#1514 makes the prerequisite unconditional, which covers every fs value and both conf_zfs providers, and adds the two mount tools for fs=ramfs where the generated manifest lists them. It also carries the before/after evidence on an unmodified tree.

Sorry for the noise of two PRs on one line of Makefile. Keeping the one that is correct.

@gburd gburd closed this Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant