Add Btrfs fallback for other Linux distros - #956
Conversation
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>
|
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 |
|
I think we also need to be more careful about where we use the |
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:
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>
|
@jonathan-conder I've implemented the requested change in I ran this on an Ubuntu Resolute LXD VM, reproducing a Reproduction
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 driverRecovery The ZFS pool can't be deleted through LXD while the module is missing ( On adding a Spread test Let me know if you think we should add a Spread test covering this case. Two caveats:
|
| inst.Devices = map[string]map[string]string{} | ||
| } | ||
| if err := mergeDevices(inst.Devices, snapshot.Sdks, name); err != nil { | ||
| usesZFS, err := poolUsesZFS(conn) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
With this change cbe0b61 poolUsesZFS reads from a cached value.
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.
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.
we already do this, it's not a change at all |
cbe0b61 to
2fcccdc
Compare
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>
2fcccdc to
949bc5e
Compare
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 |
| |ws_markup| uses ZFS for storage on Linux, | ||
| with automatic Btrfs fallback on Windows Subsystem for Linux (WSL). | ||
| |ws_markup| uses ZFS for storage where it detects that ZFS is available, | ||
| with automatic Btrfs fallback otherwise, |
There was a problem hiding this comment.
| with automatic Btrfs fallback otherwise, | |
| with automatic btrfs fallback otherwise, |
It looks like upstream alternates between BTRFS and btrfs, what do you think @akcano?
| // 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" | ||
| } |
There was a problem hiding this comment.
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.
| // 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 |
There was a problem hiding this comment.
do we need a mutex here? I think anything that looks at the storage driver should run after the value has been initialised.
| // 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 | ||
| } |
There was a problem hiding this comment.
I also don't see a need for lazy initialisation
| // 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. |
There was a problem hiding this comment.
what is the point of this comment?
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/zfsAND/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.Self-review quick check
isWSL()Docs
Procedure:
Content:
tutorial/andhow-to/sections).docs/.coverage.yamlupdated, coverage tags added (.. artefact).Or: