QCOW metadata published a fully deallocated cluster before the
caller performed the host punch-hole. Under multi-queue load, a
concurrent metadata flush could make the cluster allocatable, and
another queue could reuse it before the delayed punch ran. If reused as
an L2 table, the stale punch erased live metadata and made guest data
unreachable.
This was observed in production as confirmed guest data loss, with
allocated guest clusters becoming refcounted but unreachable from the
QCOW mapping.
Keep punch-pending clusters out of both free lists. Publish a cluster
only after the host punch succeeds, so another metadata flush is
required before reuse.
Add deterministic coverage for the cross-queue schedule and for host
operation failures.
Signed-off-by: doge <me@crackerben.com>
apply_dealloc_action() discarded the result of the host punch-hole
and write-zeroes operations, so a guest DISCARD or WRITE ZEROES
request completed successfully even when the host operation failed.
Return the error to the per-queue engine and complete the request
with an error instead, in both the synchronous and io_uring QCOW
engines. A failure does not abort the remaining actions of the
request: they are still applied, and the first error is reported.
Signed-off-by: doge <me@crackerben.com>
The raw, qcow, and vhdx sync engines each carried an EventFd and a
VecDeque of completions and repeated the same push and signal idiom at
every completion site. Bundle the two into a SyncCompletionQueue with
complete, notifier, and next_completed, and route the three engines
through it. The repeated eventfd signal now lives in one place.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Reject buf.len() that isn't sector_count * logical_sector_size in
io::read/write (VhdxIoError::InvalidBufferLength), and reject buf.len()
not a multiple of the sector size in Vhdx's Read/Write impl, before it's
converted to a sector count/index.
Signed-off-by: Alexander Lvov <alexander.lvov.git@gmail.com>
cloud-hypervisor always advertises a 512-byte logical block size over
virtio-blk, so a 4096-byte-sector VHDx image could never be addressed
correctly. Reject it at metadata parse time, matching QEMU's VHDx
support.
Signed-off-by: Alexander Lvov <alexander.lvov.git@gmail.com>
io::read/write sliced buf using free_sectors * SECTOR_SIZE, a hardcoded
512-byte constant, instead of the file's real logical sector size
([MS-VHDX] allows 512 or 4,096). On a 4096-byte-sector image, only the
first 512 bytes of each sector were transferred while the full 4096 was
reported as done.
Use sector.free_bytes instead.
Signed-off-by: Alexander Lvov <alexander.lvov.git@gmail.com>
Reimplement run_unaligned_operation over AlignedFile read_vectored_at
and write_vectored_at instead of scattering and gathering through the
per operation write_bytes_at and read_bytes_at closures. The aio and
uring engines already reach this helper for the O_DIRECT misaligned
case, so both now share the same vectored bounce path as the sync
engine and the scatter gather logic lives only in AlignedFile.
The operation iovecs point at the same memory that write_bytes_at and
read_bytes_at reach, and the aligned fast path already hands those
iovecs to the kernel, so the direct vectored call is equivalent. For
guest memory read targets the destination pages are marked dirty by
mark_read_dirty in execute_async before the operation is submitted,
independent of how the bounce copies bytes, so the raw iovec path
preserves dirty tracking. Drop the now unneeded mut bindings at the
call sites.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Cover the empty iovec noop, the fast path where iovecs go straight to
preadv or pwritev, and misaligned O_DIRECT scatter and gather across
multiple iovecs including a partial block read-modify-write.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add read_vectored_at and write_vectored_at to AlignedFile. They take
the aligned fast path with a single preadv or pwritev when the offset
and every iovec base and length satisfy the O_DIRECT alignment, and
otherwise bounce through an AlignedBuffer, scattering on read and
gathering with a read-modify-write on write.
Convert the raw sync engine to these methods and drop its raw preadv
and pwritev block. The methods are unsafe because their soundness
depends on the caller passing iovecs that describe valid memory for
iov_len bytes.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
vm-memory 0.18 has renamed GuestMemory to GuestMemoryBackend, and made
GuestMemory refer to something less specific. For simplicity, we keep
using GuestMemoryBackend (formerly GuestMemory) everywhere for now. We
can adjust bounds to be less specific later if we find ourselves needing
the newly enabled flexibility.
Signed-off-by: Alyssa Ross <hi@alyssa.is>
Signed-off-by: Rob Bradford <rbradford@meta.com>
The qcow2 backend caches L2 table and refcount updates in memory and
only writes them back on a guest-initiated flush, clean shutdown or
drop. A paused VM therefore leaves the on-disk image without the
mappings for any cluster allocated since the last guest flush: the
data clusters are present in the file, but nothing references them.
Anything that reads the image while the VM is paused sees those
writes as missing. Copying the disk alongside vm.snapshot (the
documented snapshot workflow) captures a stale image, and live
migration releases the disk locks after pausing so the destination
reopens the file with the same stale metadata. In both cases writes
the guest has completed, and may later read back, silently disappear.
Add a MetadataSync capability trait with a no-op default, fold it
into FullDiskFile, implement it for the qcow2 backend as a metadata
cache flush, and call it from the virtio-block pause path after
in-flight requests have drained. Pause is the quiesce point both
flows rely on, and it is a cold path, so the extra flush does not
affect runtime I/O.
Reproduced by writing to a qcow2 disk from the guest with O_DIRECT
and no explicit flush, pausing the VM and copying the image: qemu-img
map on the copy shows no mapped clusters and reads return zeros. With
this change the copy contains every completed write. A unit test
covers the same sequence at the format level: a completed write is
invisible to a fresh reader until sync_metadata, and visible after.
Signed-off-by: CMGS <ilskdw@gmail.com>
Exercise the EOPNOTSUPP fallback path using memfd, which shares
the same backing as tmpfs and triggers the fallback without
requiring privileges or a specific filesystem.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The WriteZeroesAt fallback means write_zeroes no longer fails on
filesystems that lack fallocate support. The skip is now dead
code.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Filesystems such as tmpfs do not support fallocate with
FALLOC_FL_ZERO_RANGE or FALLOC_FL_PUNCH_HOLE and return EOPNOTSUPP.
When a raw disk image lives on such a filesystem, virtio write zeroes
and discard requests fail with IOERR.
Use the WriteZeroesAt trait from vmm_sys_util through AlignedFile,
which already bundles fallocate with a positional write fallback.
For punch_hole, catch EOPNOTSUPP and fall back to the same trait.
The io_uring engine previously submitted fallocate directly through
the ring, where the async EOPNOTSUPP completion had no retry path.
Route it through the same sync helpers that the other engines
already use.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
After churning a single L2 region with an fsync per write, close and
reopen the image. parse_qcow rebuilds the free list from the on disk
refcounts, so a reopen that recovers many clusters means the runtime
allocator stranded them. Without the fix the reopen recovered every
relocated cluster. Now it finds none.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
After churning a single L2 region with an fsync per write, scan the file
for refcount==0 clusters and assert every one is on the runtime free
list. The reuse bug left relocated refcount block clusters free on disk
but off the list, so this fails without the fix, reporting the exact
count of stranded clusters.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add a regression test that churns one L2 region with an fsync per write,
relocating the L2 table and its refcount block every iteration, and
asserts committed host blocks track live data instead of doubling. Data
written across the churn is read back to confirm integrity.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Metadata tables use relocate on write. The first modification after a
flush writes the table to a new cluster and frees the old one. The L2
path returns the old cluster to the free list, but the refcount block
path did not. QcowState::set_cluster_refcount set the dropped refcount
block cluster to refcount 0 yet never pushed it onto unref_clusters, so
it stayed committed on the host and was invisible to the allocator.
Every refcount block relocation stranded one cluster, and
get_new_cluster extended the file instead of reusing it.
Under a sync heavy workload each fsync that dirties a table relocates
it, so the physical footprint grew at roughly twice the live data
without bound. qemu-img check stayed clean because the refcounts are
correct, and a reopen, which rebuilds the free list from refcounts,
recovered the space.
Push the dropped refcount block cluster onto unref_clusters so the
running allocator reuses it, matching the L2 table path.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The request can be spread over multiple descriptors but the virtio-block
specification (and this code) expects that is a whole number of sectors
(512 bytes).
Signed-off-by: Rob Bradford <rbradford@meta.com>
When writing the header after a resize the compression type field was
always set to 0, which selects zlib, even when the image was originally
created with zstd. The resized image would then no longer be usable.
Write the actual configured compression type instead.
Fixes: #8558
Assisted-by: Claude:Opus-4.8
Signed-off-by: Rob Bradford <rbradford@meta.com>
Use a pair of structs for the basic QCOW V2 header and V3 additional
fields and serialize them to/from disk via the zerocopy crate.
This removes the need to manage the position to read from/write to.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Rob Bradford <rbradford@meta.com>
Remove the internal and worker submodule layers from the VHDX format
directory. The bat, header, io, and metadata parsers move up as
direct children, internal/mod.rs becomes parser.rs, and the sync
backend moves up as engine_sync.rs. The declaration only
worker/mod.rs is dropped.
The public types are surfaced at the vhdx module level, so callers
use block::formats::vhdx instead of reaching into the internal
module.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Remove the internal and worker submodule layers from the VHD format
directory. The footer and fixed parsers move up as footer.rs and
fixed.rs, and the backends move up as engine_sync.rs and
engine_uring.rs. Both internal/mod.rs and worker/mod.rs held only
module declarations and are dropped.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Remove the worker submodule layer from the raw format directory. The
backend files move up as engine_sync.rs, engine_uring.rs, and
engine_aio.rs, the shared test helpers move up as tests.rs, and the
two alignment helper functions from worker/mod.rs merge into the raw
module.
The vhd backends that reused the raw io_uring and sync engines are
updated to the new block::formats::raw::engine_* paths.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Remove the internal and worker submodule layers from the QCOW2
format directory. The former internal files become direct children
of qcow, with internal/mod.rs turning into parser.rs. The worker
backends move up as engine_sync.rs and engine_uring.rs, and
worker/mod.rs, which held only module declarations, is dropped.
The public parser types are now surfaced at the qcow module level,
so external callers use block::formats::qcow instead of reaching
into the internal module.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
VhdxSync::submit_data_operation() passed every read/write straight to
the underlying Vhdx without checking the request against the virtual
disk's logical size. A request that started inside the image but
extended past its end (or an offset past the end entirely) was passed
through unchecked, silently reading/writing out of the intended
bounds.
Call AsyncIoOperation::validate_bounds() from submit_data_operation()
before dispatching the operation, the same way the VHD sync worker
does. The check rejects any request whose offset + length exceeds the
logical size.
Signed-off-by: Alexander Lvov <alexander.lvov.git@gmail.com>
Reuse global validate_bounds() operation helper instead of
having a local implementation in vhd/worker/common.rs
Signed-off-by: Alexander Lvov <alexander.lvov.git@gmail.com>
Extract the dynamic VHDX qemu-img helper into a shared vhdx::
test_util module to reuse inside the upcoming VhdxSync bounds-check.
Signed-off-by: Alexander Lvov <alexander.lvov.git@gmail.com>
```
warning: using `chunks_exact` with a constant chunk size
--> block/src/formats/qcow/internal/header.rs:253:39
|
253 | for entry in data.chunks_exact(FEATURE_NAME_ENTRY_SIZE) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `as_chunks` instead
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#chunks_exact_to_as_chunks
= note: `-D clippy::chunks-exact-to-as-chunks` implied by `-D clippy::all`
```
Signed-off-by: Rob Bradford <rbradford@meta.com>
The region table overlap fix in RegionInfo::new computes each entry's
end offset as `file_offset + length`. Both values are taken verbatim
from the image, so a crafted or corrupt VHDX can set a file offset near
u64::MAX and make that addition wrap. A wrapped end offset compares as a
small value, which can slip a genuinely overlapping region past the
half-open interval check that #8483 added.
Use checked_add for the end offset and return a new RegionEntryOverflow
error when it wraps, so a malformed entry is rejected instead of being
folded into a valid-looking range. The computed end is now reused for
the region_entries map so the bound is only calculated once. Add a
regression test for a wrapping entry.
Signed-off-by: Sayed Kaif <metsw24@gmail.com>
Nothing reads or writes the AlignedFile through a cursor anymore, so
remove the Read, Write, and Seek impls together with the in memory
position field. SeekHole no longer tracks a position. The cursor unit
tests move to read_at and write_at, dropping the one that duplicated
existing coverage.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Request::execute seeked the disk to the request sector and then read or
wrote sequentially through the cursor. Walk a running offset with
read_exact_at and write_all_at instead, and flush with fsync, so the
bound becomes FileExt and FileSync rather than Seek, Read, and Write.
The seek step is gone, so drop the now unused ExecuteError::Seek.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
detect_image_type read the first block through the AlignedFile Read
cursor. Read it with read_exact_at at offset 0 instead, so image type
detection no longer depends on the cursor.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Read the trailing footer sector with query_device_size and
read_exact_at instead of seeking to the end of the AlignedFile and
reading through its cursor.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the qcow image compressor test helper to positional access
instead of the cursor. Test only change.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the qcow sync worker test helpers to positional access instead
of the cursor. Test only change.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the qcow header test helpers to positional access instead of
the AlignedFile cursor. Test only change.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the QcowRawFile unit test verifications to positional access
instead of the cursor. Test only change.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the qcow header feature bit writes to positional access and
drop the now unused Seek imports. The result is unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the qcow header reader and writer to positional access instead
of the AlignedFile Read, Write, and Seek cursor. The writes still go
through the AlignedFile O_DIRECT bounce, so alignment is unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
zfs and virtiofs do not eagerly account a mode-0 fallocate() in
st_blocks, so detect those filesystems with fstatfs() and skip the
physical-size assertions there. A skip now names a proven platform
limitation instead of being inferred from the value under test, and
every assertion still runs unconditionally on ext4/xfs.
Fixes#8296
Signed-off-by: Adel-Ayoub <adelayoub.maaziz@gmail.com>
Add a write side counterpart to read_unaligned_propagates_closure_error.
The test drives write_unaligned with a gather closure that returns an
error and checks that write_unaligned surfaces it unchanged.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Cover the qcow2 magic and the non qcow magic cases of the AlignedFile
detect_image_type, which now reads the magic positionally.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Opening a raw backing file issued a seek to the end for its size and
then rewound the cursor. RawBacking reads through read_exact_at, so
the cursor reset was dead. Query the size through query_device_size,
matching the crate convention and returning the right size for a
block device backing file as well, and drop the rewind. The now
unused SeekFrom import is removed.
The result is unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
detect_image_type saved the cursor, rewound, read the magic, then
restored the cursor. Read the four magic bytes with read_exact_at at
offset 0 and decode with from_be_bytes, so the save, rewind, and
restore go away. BeUint moves to the test module, its only remaining
user in this file.
The result is unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The refcount rebuild check issued a seek to the refcount table and to
the first refblock before each cursor read. Read the fixed size fields
with read_exact_at at their offsets and decode with from_be_bytes. The
seeks and the matching error paths go away.
The result is unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The compressed cluster write and read and the L1 resize size query
went through a seek on the AlignedFile cursor before the access. Pass
the target offset to write_at and read_exact_at, and read the file
length from physical_size.
The result is unchanged. The compressed paths keep routing through the
AlignedFile O_DIRECT bounce.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Cover read_unaligned and write_unaligned directly: a scatter read at an
unaligned offset, a short read at EOF, a read-modify-write gather that
preserves head and tail padding, and error propagation from the
scatter closure.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
