Add Btrfs fallback for other Linux distros by locnnil · Pull Request #956 · canonical/workshop

locnnil · 2026-07-21T22:29:50Z

Description

Enables Btrfs fallback for other Linux distributions.

ZFS is the preferred storage driver, but it isn't available everywhere: WSL, SUSE, Arch, Fedora, and other distributions may ship without it. Keying the fallback off the distribution name is too coarse; it assumes every non-Ubuntu host lacks ZFS and every Ubuntu-derived one has it, neither of which holds for Debian and its derivatives.

Instead, detect whether the host provides a usable ZFS kernel module, already loaded (present at /dev/zfs AND /sys/module/zfs ) or loadable for the running kernel per modules.dep(5) (/lib/modules/$(uname -r)/modules.dep) or modules.builtin (/lib/modules/$(uname -r)/modules.builtin) and keep ZFS only then, falling back to btrfs otherwise. Only the kernel module is checked: LXD ships as a snap that bundles the zpool/zfs userspace tools, so a loaded or loadable module is what actually determines usability from the host side, tested on Arch Linux and openSUSE tumbleweed.

Closes: workshop does not work on non-Ubuntu systems #742

Self-review quick check

Make decisions that cost a lot to reverse explicit in the PR description.
Avoid nested conditions.
Delete dead code and redundant comments. TODO: check if it is okay to delete isWSL()
Normalise symmetries by sticking to doing identical things identically.

// one way to handle errors
if err := f(); err != nil {
   ...
}

// one way to handle multiple returns
val, err := f()
if err != nil {
   ...
}
...

Check that coupled code elements, files, and directories are adjacent. For example, test data is stored as close as possible to a test.
Put variable declaration and initialisation together.
Divide large expressions into digestable and self-explanatory ones. Use multiple variables if required.
Put a blank line between two logically different chunks of code.
Follow the style guide for new error messages.

Docs

Procedure:

I have checked and added or updated relevant documentation.
I have checked and added or updated relevant release notes.
I have included the technical author in the review.

Content:

Headings and titles accurately describe the content.
New and updated pages include correct metadata.
Documentation tests are added or updated where applicable (for tutorial/ and how-to/ sections).
Documentation follows the style guide.
If needed, docs/.coverage.yaml updated, coverage tags added (.. artefact).

Or:

I confirm the PR has no implications for documentation.

ZFS is the preferred storage driver, but it isn't available everywhere: WSL, SUSE, Arch, Fedora and other distributions may ship without it. Keying the fallback off the distribution name is too coarse -- it assumes every non-Ubuntu host lacks ZFS and every Ubuntu-derived one has it, neither of which holds for Debian and its derivatives. Instead, detect whether the host provides a usable ZFS kernel module (already loaded, or loadable for the running kernel per modules.dep / modules.builtin) and keep ZFS only then, falling back to btrfs otherwise. Only the kernel module is checked: LXD ships as a snap that bundles the zpool/zfs userspace tools, so a loaded or loadable module is what actually determines usability from the host side. Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

The storage docs described the Btrfs fallback as WSL-specific. Reword the three affected passages to reflect that Workshop uses ZFS wherever it detects ZFS is available and otherwise falls back to Btrfs, keeping WSL as an example rather than the sole case. Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

jonathan-conder · 2026-07-21T22:35:16Z

It looks to me like the way LXD does it is just trying to load the zfs module; maybe we should follow their lead on that

jonathan-conder · 2026-07-21T22:37:40Z

I think we also need to be more careful about where we use the storagePoolDriver variable. Currently we can assume it's accurate, since whether the host is WSL or not is unlikely to change. The availability of the ZFS module can easily change, though; we need a separate mechanism to indicate "what's the current driver of the workshop pool."

locnnil · 2026-07-22T00:23:10Z

I think we also need to be more careful about where we use the storagePoolDriver variable. Currently we can assume it's accurate, since whether the host is WSL or not is unlikely to change. The availability of the ZFS module can easily change, though; we need a separate mechanism to indicate "what's the current driver of the workshop pool."

So, considering the case where ZFS was available when the pool was created but later stops being present, most likely because the user installed a different kernel that doesn't ship the ZFS module. What should we do with the now-unusable ZFS pool?

I can think of those options:

Put the daemon into a degraded state and add an actionable error message like "restore the ZFS-capable kernel, or remove the pool to recreate it as Btrfs"
Warn the user with an actionable message like "Old ZFS pool detected on system without support for ZFS on path <path-to-zfs-pool>" but create a new Btrfs pool under a different name.

I believe that the first option is better, right?

jonathan-conder · 2026-07-22T01:37:09Z

I believe that the first option is better, right?

I agree, but we should not suggest removing the pool, it will probably break the workshops using it (if it's even possible to remove it). Instead maybe it's best to reinstall the Workshop snap.

We just need to test what actually happens in that scenario, maybe LXD will fail to start and we probably handle that already.

ZFS is the preferred storage driver, but it isn't available everywhere (Debian, SUSE, Fedora and WSL, among others, may ship without it). Use ZFS only when LXD reports it as a supported storage driver; which LXD determines by trying to load the driver's kernel module, and fall back to Btrfs otherwise. Keying the choice off actual availability rather than the host distribution makes the fallback apply to any host lacking ZFS, while still using ZFS wherever it works. Derive the pool's driver from LXD and the pool itself rather than from a fixed value: pick it from LXD's supported drivers when creating the pool, and read it back from the pool wherever behaviour depends on it. A pool's driver is fixed at creation and ZFS availability can change, so a single cached value can't be trusted. When the workshop pool exists but its storage driver is no longer usable (for example a ZFS pool after booting a kernel without the ZFS module), put the daemon into degraded mode with an actionable message instead of failing obscurely. Closes: canonical#742 Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

locnnil · 2026-08-14T01:50:14Z

@jonathan-conder I've implemented the requested change in 54bf6dd, which stops assuming a fixed state for ZFS availability. I then tested the degraded-state behavior for the scenario where ZFS is present at first but later vanishes.

I ran this on an Ubuntu Resolute LXD VM, reproducing a workshop pool created on ZFS whose kernel module later becomes unavailable after a kernel change.

Reproduction

Started with ZFS available, so the workshop pool was created on the zfs driver.
Built and installed a .deb for a newer kernel (7.2.0-rc7) that doesn't ship the ZFS module.
Removed the previous kernel and its ZFS module package (linux-main-modules-zfs-7.0.0-29-generic).
Rebooted into the new kernel and ran workshop list.

Result

Instead of surfacing a raw LXD error, the daemon goes into degraded mode with an actionable message:

$ workshop list
error: system is not healthy: cannot use the "workshop" storage pool, its storage driver may be unavailable (for example a ZFS pool after booting a kernel without the ZFS module): Error loading "zfs" module: Failed running: modprobe -b zfs
: exit status 1 (modprobe: FATAL: Module zfs not found in directory /lib/modules/7.2.0-rc7)
Boot into a kernel that provides the pool's storage driver, or reinstall Workshop to recreate the pool on an available driver

Recovery

The ZFS pool can't be deleted through LXD while the module is missing (lxc storage delete loads the driver, which fails), so the recovery is to reinstall Workshop: this recreates the workshop pool on an available driver (Btrfs) for a host that no longer provides ZFS.

On adding a Spread test

Let me know if you think we should add a Spread test covering this case. Two caveats:

For my manual test I had to build and install an upstream kernel, since simply uninstalling linux-main-modules-zfs-7.0.0-xx-generic isn't enough to reproduce a realistic scenario. For an automated test we could instead force the broken state, making it reproducible by just forcefully removing a package.
The bigger drawback is that this would require Spread to run on LXD VMs instead of containers. That would be a significant change to the current testing infrastructure.

jonathan-conder · 2026-08-16T22:17:22Z

 		inst.Devices = map[string]map[string]string{}
 	}
-	if err := mergeDevices(inst.Devices, snapshot.Sdks, name); err != nil {
+	usesZFS, err := poolUsesZFS(conn)


this adds an additional API call to these operations, do we have any info on the impact it has on latency? we could alternatively cache the storage pool driver at launch

With this change cbe0b61 poolUsesZFS reads from a cached value.

jonathan-conder · 2026-08-16T23:15:41Z

The ZFS pool can't be deleted through LXD while the module is missing (lxc storage delete loads the driver, which fails), so the recovery is to reinstall Workshop: this recreates the workshop pool on an available driver (Btrfs) for a host that no longer provides ZFS.

I don't see how reinstalling Workshop will help. If LXD is unable to delete the pool then it will still be there after the reinstall.

For my manual test I had to build and install an upstream kernel, since simply uninstalling linux-main-modules-zfs-7.0.0-xx-generic isn't enough to reproduce a realistic scenario. For an automated test we could instead force the broken state, making it reproducible by just forcefully removing a package.

It's not reasonable to do that in a test. It's a rare enough case that I think some manual recovery steps are OK.

The bigger drawback is that this would require Spread to run on LXD VMs instead of containers. That would be a significant change to the current testing infrastructure.

we already do this, it's not a change at all

Refine how the LXD backend determines the workshop pool's storage driver: - Query the workshop pool directly with GetStoragePool and branch on the 404 status instead of listing every pool. This matches checkStoragePool and checkStorageSpace and avoids loading unrelated pools' drivers. - Record the pool's driver once it is known (picked when the pool is created, or read back from an existing pool) and reuse it for snapshot and copy operations, instead of querying LXD on every call. The driver is fixed at creation, so the cached value stays accurate for the daemon run. - Drop the redundant "is the preferred driver supported" check: it only re-tested the choice preferredDriver already made from the supported list, and pool creation surfaces an unusable driver anyway. - Inline the "zfs"/"btrfs" literals; the named constants added nothing over LXD's fixed driver identifiers. Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

locnnil · 2026-08-29T01:13:18Z

I don't see how reinstalling Workshop will help. If LXD is unable to delete the pool then it will still be there after the reinstall.

Yeah, I think in this case you would need to reinstall LXD and Workshop, but this seems too drastic. Anyway, I've tested, and when the ZFS-capable kernel is restored, the workshop will be working again.

jonathan-conder · 2026-09-04T00:09:16Z

+// driverSupported reports whether LXD lists the named storage driver as
+// supported. LXD builds this list by attempting to load each driver's kernel
+// module (e.g. modprobe zfs), so it reflects what the host can actually back.
+func driverSupported(supported []api.ServerStorageDriverInfo, name string) bool {
+	return slices.ContainsFunc(supported, func(d api.ServerStorageDriverInfo) bool {
+		return d.Name == name
+	})
+}
+
+// preferredDriver picks the driver for a new workshop pool: ZFS when LXD
+// reports it as supported, otherwise Btrfs.
+func preferredDriver(supported []api.ServerStorageDriverInfo) string {
+	if driverSupported(supported, "zfs") {
+		return "zfs"
+	}
+	return "btrfs"
+}


instead of factoring out a function with a single callsite, I would turn preferredDrivers into a list and loop over it, in a single function. If none of the preferred drivers are supported we should return an error.

jonathan-conder · 2026-09-04T00:10:27Z

+// re-querying LXD (which reloads the driver's module). The pool's driver is
+// fixed at creation, so the cached value stays accurate for the daemon's run.
+var poolDriverCache struct {
+	sync.RWMutex


do we need a mutex here? I think anything that looks at the storage driver should run after the value has been initialised.

jonathan-conder · 2026-09-04T00:11:25Z

+// poolUsesZFS reports whether the workshop storage pool is backed by ZFS. It
+// reuses the driver recorded by ensureBackendReady, falling back to querying
+// LXD if it hasn't been recorded yet.
+func poolUsesZFS(conn lxd.InstanceServer) (bool, error) {
+	driver := getPoolDriver()
+	if driver == "" {
+		pool, _, err := conn.GetStoragePool(storagePool)
+		if err != nil {
+			return false, err
+		}
+		driver = pool.Driver
+		setPoolDriver(driver)
 	}
-	if slices.ContainsFunc(drivers, hasDriver) {
+	return driver == "zfs", nil
+}


I also don't see a need for lazy initialisation

jonathan-conder · 2026-09-04T00:11:53Z

+// checkStoragePool verifies the workshop storage pool is usable. When the pool
+// exists but cannot be loaded, most likely because its storage driver's kernel
+// module is gone (e.g. a ZFS pool after booting a kernel without the ZFS
+// module), it returns an actionable error that puts the daemon into degraded
+// mode. A missing pool is not an error here: ensureBackendReady creates it.


what is the point of this comment?

- doc: lowercase btrfs spelling - cleanup: refact preferredDrivers function - feat: Record workshop pool driver without a mutex The pool driver is recorded once during the initial system check, before the daemon serves requests, so pool operations can read it directly. Drop the RWMutex and lazy initialisation, which are no longer needed. - chore: improve comment Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

Co-authored-by: Jonathan Conder <jono.conder@gmail.com>

Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

jonathan-conder

Ok I think we're nearly there, just a couple of minor comments below, and I still think we should check for storagePoolDriver != existingPool.Driver in ensureBackendReady()

Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

Copilot

Copilot review overview

🟡 Changes recommended

Critical storage-driver race and pool-validation issues remain, and fallback integration coverage is missing.

Review effort: Lite
Findings: 1 · 2

Open (3)

Prevent driver races and changes during pool recovery · New
Update documentation and diagrams for Btrfs support · New
Capitalize filesystem name as Btrfs · New

What changed in this PR

Adds dynamic ZFS-preferred storage selection with Btrfs fallback based on LXD capabilities, plus tests and documentation updates.

Changes:

Selects and tracks the storage pool driver.
Adds driver-selection test coverage and test exports.
Updates tutorial, reference, and architecture documentation.

File	Summary
`internal/workshop/lxd/lxd_backend.go`	Implements storage-driver selection and pool initialization.
`internal/workshop/lxd/lxd_backend_test.go`	Tests preferred-driver selection and fallback behavior.
`internal/workshop/lxd/export_test.go`	Exposes driver-selection logic for tests.
`docs/tutorial/part-1-get-started.rst`	Documents storage fallback prerequisites.
`docs/reference/workshops.rst`	Updates storage-driver documentation.
`docs/explanation/architecture/components.rst`	Updates the storage architecture description.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Recreate a missing pool on the already-selected driver and only set storagePoolDriver on the first check, avoiding a data race and a silent ZFS/Btrfs switch while the daemon is serving requests. Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

Copilot

🟡 Changes recommended

Approval is blocked by an unresolved critical data race during LXD recovery, along with documentation inconsistencies about Btrfs capitalization and ZFS-only behavior.

1 open issue

Synchronize storage driver recovery with backend goroutines · New

1 previously missed issue

In code that hasn't changed since last review

Capitalize Btrfs consistently in user-facing documentation

docs/explanation/architecture/components.rst:262

This prose uses lowercase btrfs, whereas the tutorial and the new code comment use Btrfs. Keep the filesystem name consistently capitalized in user-facing documentation; reserve lowercase for the LXD driver identifier.

3 resolved since last review

_{Review effort: Lite}

+		if storagePoolDriver == "" {
+			storagePoolDriver = driver


locnnil added 2 commits July 21, 2026 17:34

jonathan-conder reviewed Aug 16, 2026

View reviewed changes

locnnil added 2 commits August 19, 2026 22:59

Merge branch 'main' into add-btrfs-fallback

9ce2502

Merge branch 'main' into add-btrfs-fallback

d75fc17

locnnil force-pushed the add-btrfs-fallback branch from cbe0b61 to 2fcccdc Compare August 28, 2026 22:42

locnnil force-pushed the add-btrfs-fallback branch from 2fcccdc to 949bc5e Compare August 28, 2026 22:48

locnnil requested a review from jonathan-conder August 29, 2026 01:13

jonathan-conder requested changes Sep 4, 2026

View reviewed changes

jonathan-conder requested changes Sep 27, 2026

View reviewed changes

locnnil and others added 3 commits September 28, 2026 10:11

Merge branch 'main' into add-btrfs-fallback

bacaa53

Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

Apply batched suggestions from code review

4655600

Co-authored-by: Jonathan Conder <jono.conder@gmail.com>

address comments

5958238

Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

locnnil force-pushed the add-btrfs-fallback branch from e2fffd4 to 5958238 Compare October 5, 2026 14:48

locnnil requested a review from jonathan-conder October 5, 2026 14:56

jonathan-conder reviewed Oct 5, 2026

View reviewed changes

Comment thread docs/reference/workshops.rst Resolved

Comment thread internal/workshop/lxd/lxd_backend.go Resolved

address comments

f314d84

Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

locnnil requested review from jonathan-conder and a lite review from Copilot October 6, 2026 13:42

Copilot started reviewing on behalf of locnnil October 6, 2026 13:43

View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Comment thread internal/workshop/lxd/lxd_backend.go Resolved

Comment thread docs/explanation/architecture/components.rst Resolved

Comment thread docs/reference/workshops.rst Resolved

Copilot AI lite review requested due to automatic review settings October 29, 2026 20:03

Copilot started reviewing on behalf of locnnil October 29, 2026 20:04

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Comment thread internal/workshop/lxd/lxd_backend.go

Comment on lines +358 to +359

if storagePoolDriver == "" {

storagePoolDriver = driver

Conversation

locnnil commented Jul 21, 2026

Description

Self-review quick check

Docs

Uh oh!

jonathan-conder commented Jul 21, 2026

Uh oh!

jonathan-conder commented Jul 21, 2026

Uh oh!

locnnil commented Jul 22, 2026

Uh oh!

jonathan-conder commented Jul 22, 2026

Uh oh!

locnnil commented Aug 14, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonathan-conder Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

locnnil Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

jonathan-conder commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

locnnil commented Aug 29, 2026

Uh oh!

Uh oh!

jonathan-conder Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

jonathan-conder Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

jonathan-conder Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

jonathan-conder Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonathan-conder left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

jonathan-conder commented Aug 16, 2026 •

edited

Loading