Skip to content

fix: reclaim unused encrypted data disk blocks - #1175

Merged
kvinwang merged 2 commits into
nextfrom
fix/data-disk-discard
Sep 4, 2026
Merged

fix: reclaim unused encrypted data disk blocks#1175
kvinwang merged 2 commits into
nextfrom
fix/data-disk-discard

Conversation

@kvinwang

@kvinwang kvinwang commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • propagate data-disk discard through ZFS/ext4, dm-crypt, virtio-blk, and QEMU
  • enable discard by default with an attested storage_discard: false opt-out for allocation-pattern-sensitive workloads
  • start a one-time asynchronous ZFS trim when upgrading a pool whose autotrim was disabled
  • expose the option in the VMM UI and all compose-hash SDK types, and document the security trade-off

Testing

  • cargo check -p dstack-types -p dstack-util -p dstack-vmm
  • cargo test -p dstack-types storage_discard_defaults_on_and_can_be_disabled
  • cargo test -p dstack-vmm qemu_command_builder_does_not_require_prepared_paths_to_exist
  • npm run build (dstack/vmm/ui)
  • PYTHONPATH=src python3 -m pytest -q tests/test_get_compose_hash.py
  • go test ./dstack -run '^$'
  • JS compose-hash tests pass as part of npm run test:ci; integration tests requiring /var/run/dstack.sock remain unavailable in this environment

Copilot AI lite review requested due to automatic review settings September 4, 2026 02:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new dstack.storage_discard= kernel cmdline option is parsed but effectively ignored due to unconditional override from app_compose, which is misleading and likely unintended.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Propagates data-disk discard/TRIM end-to-end (guest FS → dm-crypt → virtio-blk → QEMU) and exposes an attested storage_discard control across SDKs and the VMM UI, with documentation of the security trade-off.

Changes:

  • Adds storage_discard to app-compose types (Rust + SDK parity), VMM UI form/payload, and security docs.
  • Passes discard behavior into QEMU -drive and guest disk setup (ZFS autotrim + one-time trim on upgrade; ext4 mount -o discard; dm-crypt --allow-discards).
  • Updates changelog and adds/extends targeted tests (types parity + QEMU args).
File summaries
File Description
sdk/python/src/dstack_sdk/get_compose_hash.py Adds storage_discard field to Python compose-hash type surface.
sdk/js/src/get-compose-hash.ts Adds storage_discard to JS AppCompose interface with security note.
sdk/go/dstack/compose_hash.go Adds StorageDiscard to Go AppCompose for compose-hash parity.
dstack/vmm/ui/src/composables/useVmManager.ts Wires storage_discard into UI form state and compose payload; defaults to true.
dstack/vmm/ui/src/components/CreateVmDialog.ts Adds UI checkbox + tooltip for reclaiming unused storage blocks.
dstack/vmm/src/app/qemu.rs Adds `discard=unmap
dstack/dstack-util/src/system_setup.rs Propagates discard through ZFS/ext4 and dm-crypt; starts one-time ZFS trim on upgrade if needed.
dstack/dstack-types/src/lib.rs Adds attested storage_discard (default true) + tests and SDK-parity list update.
docs/security/cvm-boundaries.md Documents storage_discard semantics and trade-off.
CHANGELOG.md Records the end-to-end discard propagation and default/opt-out behavior.
Review details

Suppressed comments (1)

dstack/dstack-util/src/system_setup.rs:2807

  • Error messages should start with lowercase per CLAUDE.md; this newly introduced bail message starts with "Failed". Consider lowercasing it for consistency with the repo’s logging/error-message style.
        let status = mount.arg(dev).arg(mount_point).status()?;
        if !status.success() {
            bail!("Failed to mount ext4 filesystem: {status}");
        }
  • Files reviewed: 10/10 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread dstack/dstack-util/src/system_setup.rs
Comment thread dstack/dstack-util/src/system_setup.rs
@kvinwang

kvinwang commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed Copilot feedback and the Rust CI failure in a21d2ff: removed the ineffective cmdline option, inlined the ext4 mount branches for readability, fixed message casing, and initialized storage_discard in the guest-agent test fixture. The targeted dstack-util, guest-agent, and VMM Rust tests pass locally.

@kvinwang

kvinwang commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Real-CVM validation completed on a TDX-capable host using the mkosi-built development image from this branch (dstack-dev-0.6.0, revision a21d2ff463) and the SGX-backed local key provider.

Configuration:

  • Real Intel TDX guest (no_tee = false), 2 vCPU, 4 GiB RAM
  • 12 GiB sparse data disk
  • key_provider = local
  • storage_fs = zfs
  • storage_discard = true

Boot evidence:

Memory Encryption Features active: Intel TDX
Getting keys from local key provider
Key provider info: KeyProviderInfo { name: "local-sgx", ... }
Filesystem options: encryption=true, filesystem=Zfs

The provider independently logged:

processing TDX quote quote_len=5006
sealing key provisioned successfully

Guest discard-path evidence:

NAME                   SIZE TYPE  FSTYPE      MOUNTPOINTS DISC-GRAN DISC-MAX
vdb                     12G disk                               512B       2G
└─vdb1                  12G part  crypto_LUKS                  512B       2G
  └─dstack_data_disk    12G crypt zfs_member                   512B       2G

dstack_data_disk: ... crypt ... 1 allow_discards

NAME    PROPERTY  VALUE  SOURCE
dstack  autotrim  on     local

End-to-end sparse-host-file allocation test:

  1. After encrypted ZFS initialization, before the test write: 1,373,761,536 bytes allocated.
  2. Wrote and synced 2 GiB from /dev/urandom: 2,171,052,032 bytes allocated.
  3. Deleted the file, synced, ran zpool trim dstack, and waited for trim completion: 40,779,776 bytes allocated.

This verifies that discard reaches the guest block device, passes through dm-crypt, is enabled in ZFS, and deallocates the backing sparse host file in a real local-key-provider-backed TDX CVM.

@kvinwang

kvinwang commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Correction and stronger end-to-end validation: the previous test explicitly ran zpool trim, so it proved the discard path but did not prove automatic cleanup. I reran the real-CVM test without invoking zpool trim, fstrim, or any other trim command.

Test setup:

  • mkosi-built dstack-dev-0.6.0 from this branch
  • Real Intel TDX guest, backed by the SGX local key provider
  • 12 GiB sparse data disk
  • encrypted ZFS, storage_discard = true
  • guest script only wrote data, called sync, deleted the file, called sync, and waited

The guest confirmed:

Memory Encryption Features active: Intel TDX
Getting keys from local key provider
Key provider info: KeyProviderInfo { name: "local-sgx", ... }

dstack_data_disk: ... crypt ... 1 allow_discards

dstack  autotrim  on  local

Workload:

dd if=/dev/urandom of=/var/volatile/dstack/persistent/autotrim-e2e.bin bs=4M count=512
sync
sleep 60
rm /var/volatile/dstack/persistent/autotrim-e2e.bin
sync
# No zpool trim, fstrim, or other trim command.

Host sparse-file allocation observations:

after write       2,170,785,792 bytes
after delete 0s   2,170,785,792 bytes
after delete 170s 1,973,719,040 bytes
after delete 180s 1,636,069,376 bytes
after delete 290s 1,448,566,784 bytes
after delete 320s 1,091,260,416 bytes
after delete 350s   556,740,608 bytes

Therefore, ordinary guest file deletion plus sync caused ZFS autotrim to issue discards automatically, and the host backing file allocation fell by about 1.61 GiB within 350 seconds. No manual trim was used in this run.

@kvinwang
kvinwang merged commit 4fd93de into next Sep 4, 2026
18 checks passed
@kvinwang
kvinwang deleted the fix/data-disk-discard branch September 4, 2026 10:30
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.

2 participants