Add dm-thin storage backend as alternative to ZFS

[antiguru]

Adds Linux device-mapper thin provisioning as a third storage backend alongside ZFS and APFS.
The motivation is dropping the out-of-tree kernel module + dedicated-pool requirement that ZFS imposes, while keeping block-level copy-on-write semantics that fit Firecracker's raw-drive model.
Selected at init time via ember init --storage <zfs|dm-thin> and persisted on GlobalConfig.

The design lives in docs/DM-THIN-SPEC.md.
Random u64 thin ids are stored on the existing VmMetadata/ImageEntry/SnapshotEntry records; no separate allocator state file.
Concurrent invocations are race-free by construction since the kernel rejects duplicate ids atomically.

Surface area

• ember init --storage dm-thin --storage-path <dir|dev> --size <SIZE> brings up a thin pool from sparse files (or a raw block device).
• All existing vm, image, and snapshot commands route through the dm-thin path when that backend is active.
• New: ember deinit [--purge] tears down the pool. New: ember storage grow --size <SIZE> grows the data device.
• Storage is now Arc<dyn StorageBackend> so multiple Linux backends can share one binary; Vm and Network stay compile-time selected.

Phases (one commit per phase)

• Trait-object dispatch refactor (no behavior change).
• StorageKind enum + multi-backend init fields on GlobalConfig/InitConfig.
• dm_thin CLI wrappers (pool, thin, loop_device, tools).
• StorageBackend trait reshape: VolumeHandle return type for creators; methods that consume an existing volume take &VmMetadata/&ImageEntry.
• DmThinStorage impl.
• thin_id + snapshots fields on metadata records (skip-serializing for backends that don't use them).
• --storage CLI flag + init dispatch.
• ember deinit + ember storage grow.
• Integration smoke tests for init/deinit/grow (gated #[ignore]).

15 new unit tests in the dm-thin module pass under cargo test.
The integration tests need root + the dm-thin kernel module, run them with sudo cargo test --test dm_thin -- --ignored --test-threads=1.

🤖 Generated with Claude Code

[aljoscha]

looks like there's formatting issues

[aljoscha]

Did a read-through of the design and the dm-thin backend code. The architectural shape looks good — Arc<dyn StorageBackend> is a clean enabler, the trait reshape (VolumeHandle, passing &VmMetadata/&ImageEntry for ops that need backend state, optional SnapshotEntry return) is the right surface, and the random-id-with-kernel-rejection design avoids inventing yet another allocator. Forks being independent (cleanup_fork and storage_dependents no-ops) is a nice property that brings Linux into parity with the planned btrfs path.

A few things worth addressing before un-drafting:

1. disk_device_path doesn't activate — vm start likely broken after reboot

crates/ember-linux/src/dm_thin_storage.rs:1960-1962:

fn disk_device_path(&self, vm: &VmMetadata) -> PathBuf {
    thin::vm_device_path(&vm.name)  // just builds "/dev/mapper/ember-vm-<name>"
}

crates/ember-linux/src/vm.rs:54 (LinuxVm::start) now uses this path to build the Firecracker drive config. dm-thin tables live only in kernel memory, so after a reboot the pool is gone and no ember-vm-* device exists. Nothing in the start path calls ensure_pool_active or ensure_thin_active.

The spec explicitly calls this out ("Per-VM and per-image volumes are activated lazily by methods that need them (e.g. disk_device_path, mount, start)") but the implementation doesn't honor it. resize does call ensure_thin_active (line 1922); disk_device_path and mount don't. Suggest having disk_device_path return Result<PathBuf> (or split off a prepare_for_use method) and route both LinuxVm::start and mount through it.

2. restore_snapshot window between delete(vm_id) and allocate_snap(snap_id)

dm_thin_storage.rs:1872-1876:

thin::deactivate(&dm_name)?;
thin::delete(pool::POOL_NAME, vm_id)?;          // (a) old vm_id gone
let new_id = thin::allocate_snap(pool::POOL_NAME, snap_id)?;  // (b) might fail
let disk_path = thin::activate(&dm_name, pool::POOL_NAME, new_id, size_sectors)?;

If (b) fails (transient dmsetup error, OOM, etc.), the VM is left with vm.thin_id pointing at a freed id and no live volume — and the persisted state on vm.json only gets updated after the function returns successfully. Consider: allocate the new snap from snap_id first, then delete the old vm_id, then activate. That keeps a valid id live throughout the window.

3. parse_iso8601 reinvents civil-date arithmetic

dm_thin_storage.rs:2275-2297 rolls a Howard Hinnant-style epoch conversion in 22 lines, just so list_snapshots can populate created_at: u64. Either use a date crate or have SnapshotInfo::created_at stay a String — nothing else in the codebase consumes it as seconds-since-epoch.

4. is_already_exists does substring match on stderr

crates/ember-linux/src/dm_thin.rs:30:

matches!(err, Error::Command { stderr, .. } if stderr.contains("File exists"))

This is the entire concurrency story for create_thin/create_snap collision retry. A locale-dependent or kernel-version-changed error string silently turns retries into hard failures. Not a blocker (dmsetup strings have been stable for years), but worth a comment pinning the kernel version this was observed on, or a regression test that runs dmsetup message against a known-duplicate id and asserts the predicate fires.

5. parse_size is reinvented; ByteSize already exists

dm_thin_storage.rs:2238-2255 re-parses size specs (50G, 200M) — but ember_core::config::size::ByteSize already does this and is used on the storage grow --size arg (src/cli/storage.rs:13). The init path could parse dm_thin_size / dm_thin_metadata_size as ByteSize in InitConfig directly rather than carrying them as Option<String> and re-parsing on the backend side.

---

6. Do we have all the data to reactivate the pool after a restart?

Mostly yes, but the encoding is fragile.

ensure_pool_active needs: metadata path, data path, block size (must match creation-time value — kernel rejects mismatches), data sectors (re-read each time). GlobalConfig persists storage_path and dm_thin_block_size. Three concerns:

(a) Block size is stored as the user's flag, not the resolved value. src/cli/init.rs writes dm_thin_block_size: args.block_size (which is None when the user omits --block-size). Both init and new then resolve via unwrap_or(pool::DEFAULT_BLOCK_SIZE_SECTORS). Same value today. But block size is permanent at pool creation — if DEFAULT_BLOCK_SIZE_SECTORS ever changes, every existing pool becomes unimportable on the next ember upgrade. Fix: at init time, resolve the value first and persist Some(actual_block_size) unconditionally.

(b) File-vs-device mode isn't persisted, it's derived at runtime. metadata_file() / data_file() (lines 1566-1582) decide between <dir>/metadata.img and storage_path.with_file_name("dm-thin-metadata.img") based on a live is_dir() check on storage_path. Mostly harmless, but reactivation depends on the filesystem still answering the same way init did. An explicit dm_thin_mode: File | RawDevice field would make the contract clearer.

(c) Raw-device metadata path looks broken. For --storage-path /dev/sdb:

storage_path.with_file_name("dm-thin-metadata.img")
// → "/dev/dm-thin-metadata.img"

with_file_name replaces the last component, so metadata lands in /dev/ — which is tmpfs on most distros and will vanish on reboot. The spec says metadata in device-backed mode should live on the state directory's filesystem (the "embedded metadata mode" paragraph in DM-THIN-SPEC.md), but the code puts it next to the device. So device-backed mode probably can't survive a reboot at all today, while file-backed mode can (assuming #1 above is also fixed).

Things that would be worth persisting to make pool config fully self-describing: resolved block size (not the user flag), mode (file vs raw-device), optionally data sectors at init time so reactivation can detect "user shrank the backing file" and refuse rather than silently importing.

The thin ids themselves — the real persistence question for individual VMs — are stored on VmMetadata.thin_id / ImageEntry.thin_id / SnapshotEntry.thin_id, with the kernel pool metadata as source of truth (queryable via thin_dump). That part of the design is sound. The gap is purely on the pool table parameters.

[antiguru]

Addressed your feedback, thank you! Should be good for another look.

[aljoscha]

Re-read the latest commit. All eight items from the previous round are addressed cleanly — disk_device_path lazily activates, restore_snapshot allocates first then swaps under a guard, parse_iso8601/parse_size are gone, is_already_exists has a regression test pinned to a real dmsetup failure line, raw-device metadata moved off /dev/, and DmThinMode/dm_thin_block_size are resolved at init and persisted on GlobalConfig. Architectural reshape (Arc<dyn StorageBackend>, VolumeHandle, methods taking &VmMetadata/&ImageEntry, optional SnapshotEntry return) reads well and is not gratuitously dm-thin-specific. Forks being independent is a real ergonomic win over the ZFS path.

A few things I'd still like to see before merge:

1. destroy_vm_storage leaks snapshot thin ids — real bug

crates/ember-linux/src/dm_thin_storage.rs:483-495:

fn destroy_vm_storage(&self, vm: &VmMetadata) -> Result<()> {
    // …
    if let Some(id) = vm.thin_id {
        let _ = thin::delete(pool::POOL_NAME, id);
    }
    Ok(())
}

If the VM has snapshots in vm.snapshots, only vm.thin_id is freed — every SnapshotEntry.thin_id stays alive in the pool metadata after vm delete. The vm.json record is gone (so ember loses the user-level handle), but the kernel still holds them. They consume metadata + any unique blocks, and only thin_dump can find them again.

Spec analogue: ZFS uses zfs destroy -r which is recursive over snapshots; the dm-thin spec at "VM destroy" only mentions deleting the VM's own thin id, missing this. Fix: iterate vm.snapshots and thin::delete(pool, snap.thin_id) for each (best-effort, before deleting the VM's own id).

2. LinuxPlatform::inspect_* still hardcodes "ZFS zvol"

crates/ember-linux/src/platform.rs:47-71:

fn inspect_vm_extra(metadata: &VmMetadata) -> Vec<(&'static str, String)> {
    let mut extra = vec![
        ("ZFS zvol", metadata.disk_path.clone()),
        // …

For a dm-thin VM, metadata.disk_path is /dev/mapper/ember-vm-myvm and the row prints ZFS zvol /dev/mapper/ember-vm-myvm — wrong label. Same in inspect_image_extra and info_extra ("ZFS pool", "Dataset" shown unconditionally). The dm-thin spec at lines 622-627 explicitly calls out that this needs to branch on config.storage_backend and add a "Thin id" row. The display layer didn't get the multi-backend update; everything else did.

3. lib.rs::create_storage btrfs branch silently falls back to ZFS

crates/ember-linux/src/lib.rs:32-36:

match config.storage_backend {
    StorageKind::Zfs => Arc::new(LinuxStorage::new(config)),
    StorageKind::DmThin => Arc::new(DmThinStorage::new(config)),
    StorageKind::Btrfs => Arc::new(LinuxStorage::new(config)),
}

init_storage properly errors out for btrfs ("not yet implemented"), but create_storage silently hands back a ZFS backend. If somebody hand-edits config.json to "storage_backend": "btrfs", every operation will go through ZFS code with garbage inputs. Should be unreachable!() or an explicit error — same shape as init_storage.

4. Pool-status check from the spec isn't wired up

The dm_thin/pool.rs module fully implements PoolStatus parsing — out_of_data_space, Failed, etc. — and the spec at line 333 says ember should "refuse VM create/start and prints an actionable error" when the pool is full. But nothing in dm_thin_storage.rs actually calls pool::status(...). The user gets EIO from dd mid-clone instead of a clean refuse. Worth at least gating create_image_volume/clone_for_vm/resize on mode != OutOfDataSpace.

5. Stale staging device on retry after a failed image pull

dm_thin_storage.rs:282-341 create_image_volume doesn't check whether ember-img-<name>-staging already exists from a previous failed run. The next dmsetup create for that name will fail with EEXIST and the leftover staging device + thin id stay until somebody manually cleans up. Cheap fix: at the top of create_image_volume, if pool::exists(&staging_dm) { thin::deactivate(&staging_dm); }.

6. Test crate noise

tests/dm_thin.rs does mod common; but only uses common::ember(...) — that pulls the entire tests/common/{mod,linux}.rs surface into the dm_thin test crate, where 35 helpers ring up as function … is never used. cargo clippy --all-targets ends with a 36-warning block, all from this test. Easiest fix: #[allow(dead_code)] on the common module, or split the helpers used by dm_thin into a smaller submodule.

Smaller things

• --block-size <sectors> is unusual UX (the help says "Defaults to 128 (= 64 KiB)"). Consider taking a ByteSize like elsewhere — same pattern as --size/--metadata-size.
• ensure_pool_active runs thin_check on every first command after reboot. For large pools this can take seconds. Worth a one-line note in the spec or a --skip-thin-check escape hatch.
• pool::exists() is used to test arbitrary dm-mapper devices (e.g. pool::exists(&dm_name) for a thin volume in ensure_thin_active). The name is misleading — it's really "does this dm device exist". Consider renaming or moving it to module level.

[antiguru]

Pushed 56b82f0. All items from the latest review addressed:

• Add CI workflow for build, clippy, and format checks #1 destroy_vm_storage now frees every snapshot's thin id before the VM's own.
• Add macOS support specification and implementation roadmap #2 inspect_vm_extra/inspect_image_extra label "Thin device"+"Thin id" when thin_id is set; info_extra branches on storage_backend.
• macos: add support for macos, using builtin Apple Virtualization and APFS cow #3 create_storage panics on StorageKind::Btrfs instead of silently routing to ZFS.
• docs: add full-scope implementation guideline to CLAUDE.md #4 New assert_pool_healthy() gate on create_image_volume / clone_for_vm / clone_vm_storage / snapshot / restore_snapshot / resize (skips grow and destroy paths). New Error::Pool variant.
• feat(sec-257): vsock support + pool commands #5 create_image_volume tears down a stale ember-img-<name>-staging left over from a prior failed run.
• feat(sec-254): vsock CID allocation and end-to-end tests #6 #[allow(dead_code)] on mod common; in tests/dm_thin.rs.

Smaller bits:

• Spec note on thin_check cost on first post-reboot command.
• Renamed pool::exists → dm_thin::dm_device_exists.
• --block-size now takes a ByteSize like --size/--metadata-size, validated to be a multiple of 64 KiB.