Skip to content

Conversation

@ekorenevsky
Copy link
Contributor

Add support of reading and writing compressed clusters.
Support zlib and zstd compressions.

@ekorenevsky ekorenevsky requested a review from a team as a code owner November 6, 2025 23:48
Copy link
Member

@rbradford rbradford left a comment

Choose a reason for hiding this comment

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

Great PR - really nice structure.

@ekorenevsky ekorenevsky force-pushed the qcow2-compression branch 5 times, most recently from b2f39d7 to a151f8a Compare November 8, 2025 00:23
@ekorenevsky
Copy link
Contributor Author

Had to squash all commits together due to impossibility to keep Cargo.lock, block/Cargo.toml, block/src/qcow/decoder.rs in sync if they were modified in different commits. The PR is not big, so I think this is not an issue.

Copy link
Member

@rbradford rbradford left a comment

Choose a reason for hiding this comment

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

It would be great if we could unit test with a real compressed data - or alternatively do a boot test in the integration test suite with a compressed qcow2 file (we could just compress one of the existing files.)

@ekorenevsky
Copy link
Contributor Author

ekorenevsky commented Nov 16, 2025

I succeeded to boot qcow2-compresed Ubuntu image. qcow2-backing also works well.

# create base snapshot, it will be immutable
qemu-img convert -c -O qcow2 ubuntu-raw.img ubuntu-compressed-frozen.qcow2
chmod -w ubuntu-compressed-frozen.qcow2

# create mutable layer, backed by immutable base snapshot
qemu-img create -b ubuntu-compressed-frozen.qcow2 -F qcow2 -f qcow2 ubuntu-compressed.qcow2


# boot
cloud-hypervisor/target/debug/cloud-hypervisor --kernel /path/to/linux/arch/x86/boot/bzImage --disk path=ubuntu-compressed.qcow2 --cmdline 'console=tty0 console=ttyS0 root=/dev/vda1 debug loglevel=8 rw net.ifnames=0' --memory size=2048M

Hope tests will work well when test qcow2 images are compressed. Will try a bit later...

Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

Generally LGTM, thanks for working on this! I left a few remarks

Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

Please don't use anyhow here - we want to get rid of it in most places in Cloud Hypervisor, not strengthen the dependency to it :) Apart from that, changes are looking good.

Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comments. LGTM now! Please fix CI

@ekorenevsky ekorenevsky force-pushed the qcow2-compression branch 2 times, most recently from 8e98d97 to bf3a856 Compare November 20, 2025 20:53
@ekorenevsky
Copy link
Contributor Author

ekorenevsky commented Nov 20, 2025

getting a weird error at CI

https://github.com/cloud-hypervisor/cloud-hypervisor/actions/runs/19551012517/job/55982246959?pr=7462

+ cargo test --lib --bins --target aarch64-unknown-linux-musl --release --workspace
    Updating crates.io index
error: failed to get `igvm` as a dependency of package `hypervisor v0.1.0 (/cloud-hypervisor/hypervisor)`

Caused by:
  failed to load source for dependency `igvm`

@likebreath
Copy link
Member

getting a weird error at CI

https://github.com/cloud-hypervisor/cloud-hypervisor/actions/runs/19551012517/job/55982246959?pr=7462

+ cargo test --lib --bins --target aarch64-unknown-linux-musl --release --workspace
    Updating crates.io index
error: failed to get `igvm` as a dependency of package `hypervisor v0.1.0 (/cloud-hypervisor/hypervisor)`

Caused by:
  failed to load source for dependency `igvm`

Looks like the recurring GitHub network flake we've seen today. I just restarted the worker.

Copy link
Member

@rbradford rbradford left a comment

Choose a reason for hiding this comment

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

Just those two changes and then we can 🚀 this.

@rbradford
Copy link
Member

Great ! lgtm- can you rebase this to drop the merge commit and then we can land it.

@ekorenevsky ekorenevsky force-pushed the qcow2-compression branch 3 times, most recently from 1601bb0 to 4787c0f Compare November 23, 2025 11:57
Add support of reading and writing compressed clusters.
Support zlib and zstd compressions.

L2 cache: store entire L2 entries, not only standard cluster addresses.

Read path. Offsets of compressed clusters cannot be determined,
therefore replace QcowFile.file_offset_read() with QcowFile.file_read().
This method reads the cluster, decompresses it if necessary and returns
the data to the caller.

Write path. QcowFile.file_offset_write(): since writing to compressed
clusters is not generally possible, allocate a new standard
(non-compressed) cluster if compressed L2 entry is encountered; then
decompress compressed cluster into new cluster; then return offset
inside new cluster to the caller. Processing of standard clusters is
not changed.

Signed-off-by: Eugene Korenevsky <[email protected]>
Add tests:
- zlib: test_virtio_block_qcow2_zlib()
- zstd: test_virtio_block_qcow2_zstd()
Both these tests use zlib- and zstd-compressed images as OS image.

Modify test_virtio_block_qcow2_backing_file() test: it is practical
to test qcow2 file-backing with compression, so use zlib-compressed
image as a backing file.

Signed-off-by: Eugene Korenevsky <[email protected]>
@ekorenevsky
Copy link
Contributor Author

ekorenevsky commented Nov 23, 2025

Great ! lgtm- can you rebase this to drop the merge commit and then we can land it.

Rebased exactly these reviewed commits on top of main. It is safe to merge.
Some jobs at CI were canceled for unknown reason.

Copy link
Member

@rbradford rbradford left a comment

Choose a reason for hiding this comment

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

🚀

@rbradford rbradford added this pull request to the merge queue Nov 24, 2025
Merged via the queue into cloud-hypervisor:main with commit e6d31a3 Nov 24, 2025
42 of 45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants