6474 Commits
Author SHA1 Message Date
Haoqian He 5689c59055 block/mirror: inherit the source node discard flag in mirror_top
mirror_start_job() inserts a mirror_top filter above the source node
for both mirror and active commit jobs. Previously, the filter was
opened with BDRV_O_RDWR only, even when the source had BDRV_O_UNMAP
enabled.

The block layer checks BDRV_O_UNMAP before invoking a node's discard
callback. Without this flag on mirror_top, discard requests return
success without reaching bdrv_mirror_top_pdiscard(). Consequently,
starting a job can silently suppress guest UNMAP/TRIM requests that
would otherwise reach the discard-enabled source.

Initialize the filter's open flags with BDRV_O_RDWR and conditionally
add BDRV_O_UNMAP from bs->open_flags. Pass the resulting flags to
bdrv_new_open_driver() so that mirror_top follows the source node's
discard setting.

This allows discard requests to reach the existing filter callback,
which forwards them to the source and either tracks the affected
range in the dirty bitmap or synchronously propagates the discard
to the target, depending on the copy mode.

Signed-off-by: Haoqian He <haoqian.he@smartx.com>
Message-ID: <20261013060549.3008438-1-haoqian.he@smartx.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
2026-10-16 09:46:54 +03:00
Jean-Louis Dupond 49edb98a81 qcow2: add functionality to repair bitmaps
In some cases, it might happen that bitmaps become corrupt.
For example when adding/removing bitmaps on a live image.

Of course this should not happen, but in case this happens, the image is
corrupt and even cannot be opened anymore. You'll get something like the
following:
qemu-img: Could not open 'disk.qcow2': Bitmap '' doesn't satisfy the constraints

So the image becomes useless, and cannot be repaired. This while in fact
only (one) bitmap entry is corrupt, and the rest of the data is just
intact.

This commit adds a way to fix this corruption, by just replacing the
bitmap list in the qcow2 image with the valid bitmaps, and dropping the
bitmaps that are corrupt.

$ qemu-img check disk.qcow2
qemu-img: Check failed: Invalid argument
qemu-img: Lost persistent bitmaps during inactivation of node '#block147': Bitmap '' doesn't satisfy the constraints

$ qemu-img check -r all disk.qcow2
qcow2_free_clusters failed: Invalid argument
Leaked cluster 3 refcount=1 reference=0
Leaked cluster 26 refcount=1 reference=0
ERROR cluster 983214 refcount=0 reference=1
Rebuilding refcount structure
Repairing cluster 1 refcount=1 reference=0
Repairing cluster 2 refcount=1 reference=0
Repairing cluster 32768 refcount=1 reference=0
....
Repairing cluster 983214 refcount=1 reference=0
The following inconsistencies were found and repaired:

    2 leaked clusters
    3 corruptions

Double checking the fixed image now...
No errors were found on the image.
983056/1048576 = 93.75% allocated, 0.05% fragmented, 0.00% compressed clusters
Image end offset: 64437878784

And the image is valid again!
Worst case you lose all bitmaps, but at least the image and the data
itself is useable again.

Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
Message-ID: <20260826101005.2345883-2-jean-louis@dupond.be>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
2026-10-16 09:46:54 +03:00
Richard Henderson 5acdfee7f0 Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging
* target/i386/tcg: zero FXSAVE FOP and reserved slot bytes
* target/i386: fix immediate stack adjustment for RET and RETF
* target/i386/tcg: remove unused operand types
* pr-manager-helper: fix sense buffer overflow
* scsi, pr-helper: clean up multipathd PR state after migration

# -----BEGIN PGP SIGNATURE-----
#
# iQFIBAABCgAyFiEE8TM4V0tmI4mGbHaCv/vSX3jHroMFAmrQpX4UHHBib256aW5p
# QHJlZGhhdC5jb20ACgkQv/vSX3jHroMKHgf/cSvNqED1GVOMHMCnaERq1xwJ8Q7X
# fCvfUYylKiuy1UC/j4X3OX7QmzHQ9hNPLD/MI3t/0wK4VuwvZFjUK9RynEr8QvvY
# PHfRC0vwvz0RNZVQsz0Mun58QqFs8R1iIju5wyj92CdTcF0Bw+dopCBptDzxH3gR
# 2d9AdOlg5StxpFqtkyDNLCrmc7UiNq5sMCR40L2U5RalqoFfy2p2d1+qm+hUi7xa
# h3JnoCNxe92OvJ6RFECxlJAWkF/0fhkak2zmQ7puKzhLdcJmFEZsK+PGp8UgpcMO
# saAEELOfBY5WFBjug+3OMJEvnTcuA7Z06t7HH4GcfwTILIXa57RvsCpBJg==
# =BVJD
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 15 Oct 2026 12:05:50 PM CEST
# gpg:                using RSA key F13338574B662389866C7682BFFBD25F78C7AE83
# gpg:                issuer "pbonzini@redhat.com"
# gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" [unknown]
# gpg:                 aka "Paolo Bonzini <pbonzini@redhat.com>" [unknown]
# gpg: WARNING: The key's User ID is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4  E2F7 7E15 100C CD36 69B1
#      Subkey fingerprint: F133 3857 4B66 2389 866C  7682 BFFB D25F 78C7 AE83

* tag 'for-upstream' of https://gitlab.com/bonzini/qemu:
  scsi: document PR_HELPER_CLEANUP command
  scsi: use REGISTER AND IGNORE EXISTING KEY for robustness
  scsi: clean up persistent reservation state after migration
  file-posix: forward PR_HELPER_CLEANUP commands to pr-manager
  pr-manager-helper: add PR_HELPER_CLEANUP command support
  qemu-pr-helper: add PR_HELPER_CLEANUP for live migration
  hw/scsi-disk: move MIG_EVENT_FAILED into a new function
  scsi: increase sense buffer size in scsi_SG_IO()
  pr-manager-helper: fix sense buffer overflow
  target/i386/tcg: drop X86_TYPE_A
  target/i386/tcg: remove unused X86_TYPE_L
  target/i386: fix immediate stack adjustment for RET and RETF
  target/i386/tcg: zero FXSAVE FOP and reserved slot bytes

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
2026-10-15 13:13:40 +02:00
Stefan Hajnoczi c6cae61e46 file-posix: forward PR_HELPER_CLEANUP commands to pr-manager
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Link: https://lore.kernel.org/r/20261013181752.1299456-7-stefanha@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2026-10-15 10:55:17 +02:00
Stefan Hajnoczi f659157103 dmg: avoid out-of-bounds memory load in dmg_read_mish_block()
dmg_read_mish_block() loads from memory beyond the end of the buffer
when count is less than 4.

There is no known security impact here because dmg_read_mish_block()
returns almost immediately without any opportunity to gain control of
execution or cause other damage. A crash will not happen because it's a
32-bit load from a memory address that was just heap allocated. The
address will be properly aligned and will not span a page of memory.

Reported-by: router0mail <https://gitlab.com/router0mail>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20261013193710.1311621-1-stefanha@redhat.com
Fixes: 7aee37b93a ("block/dmg: process a buffer instead of reading ints")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4569
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2026-10-14 10:41:11 -04:00
Kevin Wolf cf4f672c11 block: Remove now redundant bdrv_wait_serialising_requests() calls
All callers of bdrv_wait_serialising_requests() (as opposed to
bdrv_wait_serialising_requests_locked()) are just duplicating the check
that tracked_request_begin() already performs now. After removing them,
bdrv_wait_serialising_requests() has no callers left and can go as well.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20261002132135.93345-3-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2026-10-07 18:00:36 +02:00
Kevin Wolf 0bd5a1b3d6 block: Fix corruption from broken RMW atomicity
When emulating 512 byte sector devices on top of 4k native host storage,
QEMU performs a read-modify-write cycle for misaligned write requests.
These requests are marked as serialising so that any other writes to the
same 4k block will have to wait and can't overwrite the block in the
middle, which would make the RMW write back stale data.

In principle, this mechanism works fine, but there is a race condition
in the implementation: bdrv_find_conflicting_request() assumes that
req->waiting_for == NULL means that the request is the active owner of
the data blocks it touches and everyone else has to wait for this
request. However, there is a window between tracked_request_begin() and
bdrv_wait_serialising_requests_locked() where any new request has
req->waiting_for == NULL even if it conflicts with an already running
serialising request. So at this point, we have seemingly two active
owners at the same time until the new request checks for conflicts.

For a third request coming in, this isn't a problem, it just might wait
for a little longer than necessary. However, if the original RMW request
calls bdrv_wait_serialising_requests_locked() again (which it does in
bdrv_aligned_pwritev() -> bdrv_co_write_req_prepare() before writing
back the RMW buffer), it will now wait on the other request that snuck
in, which is fatal and makes the RMW buffer stale. This scenario causes
data corruption.

Fix this by checking for conflicts already in tracked_request_begin()
while holding the bs->reqs_lock mutex, so that the race window is closed
and req->waiting_for == NULL really has the intended meaning.

For good measure, add an active_owner boolean that is set when a request
is marked serialising and has completed waiting for other requests, and
assert that it is not set when waiting for another request, so that in
the case of another bug, we'd crash instead of causing data corruption.

Cc: qemu-stable@nongnu.org
Buglink: https://redhat.atlassian.net/browse/RHEL-272836
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20261002132135.93345-2-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2026-10-07 18:00:36 +02:00
Denis V. Lunev af28b65fae qcow2: do not clear the dirty bit when reopening a read-only node
qcow2_reopen_prepare() clears the dirty bit whenever the node is
reopened read-only, with an unguarded header write. A read-only node
can still be dirty, inherited from an earlier writable session, and it
holds no BLK_PERM_WRITE to resolve that. A read-only to read-only
reopen of a dirty image therefore fails outright:

  $ qemu-io -r -f qcow2 dirty.qcow2 <<< $'reopen -r\nquit'
  qemu-io: failed while preparing to reopen image 'dirty.qcow2'

Where the file node below is writable the write is not refused early,
and bdrv_co_write_req_prepare() aborts on its BLK_PERM_WRITE assertion
instead.

Clear it only for a node that is writable now, the predicate
qcow2_do_open() already uses for the repair. bdrv_is_writable() also
excludes an inactive node, whose header must not be touched either.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Cc: qemu-stable@nongnu.org
Message-ID: <20260824133729.1141990-2-den@openvz.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2026-10-07 17:36:20 +02:00
Stefan Hajnoczi 196a50b090 file-posix: add statx(STATX_DIOALIGN) support
Linux v6.1 commit 825cf206ed51 ("statx: add direct I/O alignment
information") added an interface to fetch O_DIRECT alignment values for
block devices and file systems.

Prefer STATX_DIOALIGN to older interfaces and probing, but keep them as
fallbacks in case STATX_DIOALIGN cannot provide the information.

STATX_DIOALIGN support as of Linux 6.1:
- btrfs: no
- ext4: yes
- XFS: yes
- block devices: yes
- dm-crypt: yes

This change fixes #1290 ("IO alignment probing delivers incorrect
results on Linux when used with e.g. dm-crypt") on Linux host kernel
version 6.1 or later by avoiding the problematic probing code in
raw_probe_alignment(). It would be nice to fix the probing code but I
have not been able to reproduce it. Stable/LTS distros ship Linux 6.1+,
so the number of users without STATX_DIOALIGN is becoming small.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/1290
Cc: Eric Biggers <ebiggers@google.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20260922154351.226540-1-stefanha@redhat.com>
Reviewed-by: Ben Chaney <bchaney@akamai.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2026-10-07 17:36:20 +02:00
Andrey Drobyshev 971330f94f block: allocate aligned write buffer for 'truncate -m full'
In case we're truncating an image opened with O_DIRECT, we might get
-EINVAL on write with unaligned buffer.  In particular, when running
iotests/298 with '-nocache' we get:

qemu-io: Failed to resize underlying file: Could not write zeros for
preallocation: Invalid argument

Let's just allocate the buffer using qemu_blockalign0() instead.

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20260923162224.34946-1-andrey.drobyshev@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2026-10-07 17:36:19 +02:00
Denis V. Lunev 8daaf8d3b1 block: fix bdrv_next() skipping monitor-owned nodes
it->bs is the cursor into monitor_bdrv_states for the second phase of
bdrv_next(), so it has to be NULL when that phase starts. Commit
f6d38c9f6d made the first phase store the node it returns there, to
unreference the right one when the graph changes underneath. The cursor
is now left pointing at the last BlockBackend root, so the second phase
resumes from there instead of from the head of the list and never
returns a node added before it.

A skipped node drops out of the vm_stop and migration handover paths:
not flushed, not inactivated, not snapshotted. bdrv_inactivate_all()
still reports success, so the node keeps its image lock and the
migration target cannot open the image. Only detached nodes are hit in
practice, as these callers also recurse into children.

Reset the cursor when the second phase starts. old_bs is taken at the
top of the function, so f6d38c9f6d keeps working.

Cc: qemu-stable@nongnu.org
Fixes: f6d38c9f6d ("block-backend: fix edge case in bdrv_next() where BDS associated to BB changes")
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Fiona Ebner <f.ebner@proxmox.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260828222005.2888213-1-den@openvz.org>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2026-10-07 17:36:19 +02:00
Denis V. Lunev 429bf66a1f block/throttle-groups: fix crash when enabling I/O limits on a busy disk
throttle_group_register_tgm() stores tgm->throttle_state before it takes
tg->lock, so the I/O path can see a member whose group still has NULL
tokens[] and an empty member list. A request in an iothread then reaches
throttle_group_co_io_limits_intercept(), wins tg->lock ahead of the
registering thread, and next_throttle_token() passes the NULL token to
throttle_group_next_tgm(), which dereferences it:

  throttle_group_next_tgm (tgm=0x0) at block/throttle-groups.c:185
  next_throttle_token (tgm=..., direction=THROTTLE_READ)
  throttle_group_co_io_limits_intercept (tgm=..., bytes=8192, ...)
  blk_co_do_preadv_part (blk=..., offset=..., bytes=8192, ...)
  blk_aio_read_entry (opaque=...)
  coroutine_trampoline (i0=..., i1=...)

blk_io_limits_enable() does not drain the BlockBackend, unlike its disable
counterpart, so nothing keeps requests away while the group is built. A
guest probing a disk that libvirt has just attached, at the moment the QoS
settings for it are applied, is enough to hit this.

Publish tgm->throttle_state with a release store once the member is
linked into the group and its timers exist, and read it with an acquire
load on the two unlocked I/O paths. A request that sees the new pointer
then also sees a fully built group.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Cc: Alberto Garcia <berto@igalia.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Message-ID: <20260908145554.3215221-2-den@openvz.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2026-10-07 17:36:19 +02:00
Denis V. Lunev 0bb4f47124 parallels: report the stored dirty bitmaps in qemu-img info
Nothing tells which persistent bitmaps an image carries. qemu-img info
says nothing about them, and the only other way to see one is to export
the image over NBD and ask for a bitmap by name, which needs the name
beforehand.

Add ImageInfoSpecificParallels with the bitmaps and their granularity,
in the same way as qcow2 reports the contents of its bitmap directory:

    Format specific information:
        bitmaps:
            [0]:
                name: b2c9e1a4-5d3f-4e8b-9a7c-6f0d1e2b3a45
                granularity: 65536

For running VM process the list is built on the fly from in-memory
list. This is the best we can do. FormatExtension could be dead in
the image while VMs are running as all bitmaps are cleared on
non-clean VM stop.

A bitmap of an image which was not closed correctly is inconsistent
and can not be used. Report that as well, the way qcow2 reports its
in-use flag, so such a bitmap is not listed as a valid one.

The section is left empty for an image with no persistent dirty
bitmaps, and an empty section is not printed, so the human readable
output of "qemu-img info" and "info block" is unchanged.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Acked-by: Markus Armbruster <armbru@redhat.com>
2026-09-15 13:30:29 +02:00
Denis V. Lunev c7a9d87caf parallels: implement removing a stored dirty bitmap
block-dirty-bitmap-remove leaves a parallels image alone. The driver has
no bdrv_co_remove_persistent_dirty_bitmap(), and a missing handler means
nothing to do, so the command reports success while the bitmap stays in
the image until the node is inactivated. qcow2 updates its bitmap
directory on the spot.

Implement the handler. The Format Extension is written as a whole rather
than edited in place, so the bitmap is dropped by clearing its
persistence and storing what is left, and the header is updated to point
at the new extension. Releasing the bitmap itself is up to the caller.
The store runs under s->lock, as it allocates a cluster and a request
can be in flight, unlike the inactivation path where the node is
quiesced already.

Removing a bitmap which was never stored is not an error, as
bdrv_co_remove_persistent_dirty_bitmap() spells out, so a name which is
not there, or is not persistent, is answered with success and no write.

Write access is checked first. The extension is only rewritten while the
node is writable, so on a read-only or an inactive node the request could
not reach the image at all and reporting success would be a lie. The
generic BDRV_BITMAP_RO check in block_dirty_bitmap_remove() hides this
for a read-only node, but it does not cover an inactive one, and
dropping a bitmap which was never stored needs no write access at all.

Persistence is only dropped for the duration of the rewrite, so it goes
back on when the rewrite fails. The bitmap is still in the image, and a
bitmap which is no longer marked persistent would be dropped for good by
the next inactivation, turning a removal the caller was told had failed
into one which silently succeeded.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev 8f3c8f6a30 parallels: do not trust the bitmaps of an image which was not closed
The inuse magic in the header says that the image was not closed
correctly. The bitmaps stored in the Format Extension are then stale by
definition: they were written by the last inactivation, and every write
which happened after it is missing from them. Nothing said so, the
bitmaps were loaded and handed out as valid, and an incremental backup
taken from one of them would silently miss the data written after the
last clean close.

Mark them inconsistent, as qcow2 does for a bitmap whose in-use flag
survived a crash. Using such a bitmap fails with an error naming it, so
the loss is reported to whoever tries to rely on it instead of being
discovered later in a backup.

parallels_save_bitmap() already skips an inconsistent bitmap, so it is
not written back. The format has no per bitmap flag to record that the
contents are unusable, so keeping it would present it as valid again on
the next open. Say what happens, as the bitmap disappears from the image
and 'qemu-img bitmap --remove' would report it as missing afterwards.

The image data itself is unaffected: it is repaired at open as before,
and only the bitmaps are dropped.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev 6933f46332 parallels: reject a bitmap L1 entry outside the data area
Nothing checks where a bitmap L1 entry points. The entry is turned into
an offset and the cluster is read from there, so the image decides which
part of the file, if any, is deserialized as bitmap data.

A short read on the protocol node is zero filled rather than refused, so
an entry beyond the end of the file does not even fail: the bitmap
quietly loads as completely clean. An entry below data_off deserializes
the header and the BAT as bitmap data instead. Both cases used to be
harmless in the sense that the extension was only parsed for read-only
images, but the bitmap becomes writable and is stored back as
authoritative once the image can be opened read-write.

Bound the entry the way parallels_check_outside_image() bounds a BAT
entry: it has to address the data area of the image file. The offset
computation is bounded first, as the entry is a 64 bit value coming from
the image and the shift by BDRV_SECTOR_BITS would overflow.

The overflow case was reported as an I/O error before, so the test
expectation changes along with it.

Fixes: baefd97700 ("parallels: support bitmap extension for read-only mode")
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev 27f1fb7af6 parallels: Handle L1 entries equal to one
If all the bits in a dirty bitmap cluster are ones, the cluster shouldn't
be written. Instead the corresponding L1 entry should be set to 1.

Ask bdrv_dirty_bitmap_next_zero() for a clean bit in the range the entry
covers, and store the marker when there is none. Two things have to
match the region rather than the cluster which serializes it: the search
is bounded by the end of the region, as a count and not as an offset, and
a missing clean bit is the answer we are looking for rather than a reason
to give up. A dirty run is not a substitute for either. Its length says
nothing about the region it lies in, so comparing it to the cluster size
marks a chunk as all ones when only a cluster worth of it is dirty, and
treating "no clean bit at all" as a failure leaves the entry at zero,
which says the whole chunk is clean.

Both directions are silent: a full disk overwrite comes back as a
completely clean bitmap, and a single dirty cluster comes back as
everything the entry covers being dirty. An incremental backup driven by
the first one copies nothing.

The marker goes through cpu_to_le64() like the cluster offsets below it,
as the loading side reads the table with ldq_le_p(). The serialization
moved behind the check, as there is nothing to write when the marker is
stored.

Based on the original work from Alexander Ivanov.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev 19f40ae0f5 parallels: Let image extensions work in RW mode
Saving extensions is supported now, so we can let them work in
read-write mode as well.

The bounds check rejecting a Format Extension outside the image file
used to sit in the read-only branch of the condition. Perform it
unconditionally, as the extension is now read in both modes.

A loaded bitmap was marked read-only unconditionally, guarded by an
assert that the image is not writable. Mark it so for a read-only node
only, as qcow2_load_dirty_bitmaps() does, or the next inactivation
fails with "No write access". The condition is read-only alone rather
than the full write access qcow2 asks about, as an inactive node can
become writable later and parallels has no path which would clear the
flag again. Mark the bitmap persistent as well, or it disappears from
the image the first time it is written out.

An unreadable extension must not keep the image shut. Its clusters are
deliberately left free for reuse, so a guest write may have landed on
them, and every qemu up to this one ignores the extension in read-write
mode and truncates the file to the end of the payload on close, leaving
ext_off pointing past the end of the file. Both refuse an image whose
payload is perfectly fine, in either mode, with no way back through
qemu-img check or convert. So do not ask the in use flag, ask whether
what the header points at is an extension at all: an offset outside the
file, a wrong magic or a wrong checksum all say that it is not, whoever
left it behind, so drop it with a warning and forget where it ended,
which lets the leak check reclaim the space.

Behind the checksum the extension says what its writer meant it to say,
so a feature this driver does not implement, or a bitmap it can not
parse, keeps the image shut as before. Only an image which was not
closed correctly gives up its extension there, as its bitmaps predate
every write which followed the last inactivation.

Reading a bitmap allocates a cluster sized buffer with the aborting
qemu_blockalign(), and parallels_open() lets a cluster reach almost
2 GiB. That is reachable through every open now, so allocate it the way
the storing side does and align it to bs->file.

Based on the original work from Alexander Ivanov.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev 1532731ca6 parallels: Add dirty bitmaps saving
Dirty bitmaps can be loaded now, but there is no way to save them. Add
code for dirty bitmap storage.

A bitmap which can not be stored is refused when it is created, by
parallels_co_can_store_new_dirty_bitmap(), as at store time there is
nowhere left to report it to: it would be dropped while the command
which asked to persist it still succeeded. The format identifies a
bitmap by a UUID, so a name which does not parse as one is refused, and
so is one whose L1 table does not fit the Format Extension cluster next
to the feature headers of the bitmaps already stored and the end of
features marker. The hook and the store path share the size arithmetic.

Losing a bitmap on an I/O error is still possible, and it used to be
silent. Report it through errp and answer -EINVAL from
parallels_inactivate(), the way qcow2_inactivate() does, giving up
before the in use flag is cleared: the inactivation failed, the node
stays writable, and the next open must not be told that the image was
closed correctly.

The extension is rebuilt as a whole and written to freshly allocated
clusters on every store, and its clusters are deliberately absent from
used_bmap, which is what lets the space of the copy read at open time
be reused. parallels_check_unused_clusters() derives the end of the
payload from used_bmap alone, though, so it would report the extension
as a leak and 'qemu-img check -r leaks' would truncate it away while
the header still points at it. Remember where the extension ends in
s->ext_end and use it as a lower bound for the end of the payload.

Based on the original work from Alexander Ivanov.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev c4e2b27423 parallels: Remove unnecessary data_end field
Since we have used bitmap, field data_end in BDRVParallelsState is
redundant and can be removed.

Add parallels_data_end() helper and remove data_end handling.

The two are not equivalent, which is why this comes before the Format
Extension is stored. data_end is the highest extent seen in the BAT,
while the helper derives the end of the payload from the image file. A
cluster which belongs to the image without being referenced by the BAT,
as the Format Extension and its bitmap data clusters are, stays
invisible to the field: parallels_allocate_host_clusters() appends at
data_end, so it would hand out an offset which is already occupied and
parallels_mark_used() would refuse it with -EBUSY, failing the guest
write.

seek_to_sector() validates a BAT entry against data_end, so it becomes
a user of the new helper, which suits that check better as well: a
cluster has to live inside the image file, while the field could grow
to whatever extent a corrupted BAT entry claimed.

The BAT scan in parallels_open() no longer tracks the maximum extent,
but it keeps rejecting entries below data_start or beyond the end of the
file, so need_check is still set when an entry is out of bounds. Nothing
is accumulated any more, so the scan stops at the first such entry.
high_off in parallels_check_outside_image() only fed the
image_end_offset which the helper now provides, so it goes away with
the field.

The bdrv_pwrite_zeroes() of the branch which reuses a hole goes as well.
It was guarded by data_end, and it is redundant: the space was already
fallocated when the image grew over it. The 'bytes' variable goes with
it, as the used bitmap grows by the preallocated size rather than the
requested one.

Based on the original work from Alexander Ivanov.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev 22b1d76fc2 parallels: Drop unused clusters at the end of the image
On an image closing there can be unused clusters in the end of the
image. Since we have used bitmap, they can be found without a leak
check, so parallels_check_unused_clusters() answers both questions:
inactivation asks it to truncate them away, and the leak check asks it
how much there is.

A repairing leak check truncates the file, so the used bitmap is
recreated afterwards, as it would no longer comply to it.

The helper answers with the size of the leak, which is a byte count and
does not belong in an int. parallels_inactivate() keeps it in an int64_t
and reports only a failure, as a leak of 2 GiB or more would otherwise
look like one and fail the inactivation of an image which was truncated
just fine.

A BAT entry pointing at a cluster which does not fit the file makes the
used bitmap reach further than the file does, and that difference is
not a leak of negative size. parallels_check_outside_image() reports it
as corruption on its own, so answer with no leak at all rather than
-EINVAL, which would fail the whole check before the duplicate check
and the statistics ever run.

Repairing such an entry clears it from the BAT, and the used bitmap has
to follow the way parallels_check_data_off() already makes it follow a
repaired data_off. Otherwise it keeps a bit for a cluster the image no
longer has, and 'qemu-img check -r all' leaves the space behind it
untouched instead of truncating it away.

Rebuilding the used bitmap after the truncation fails with -EBUSY or
-E2BIG on a BAT which points a cluster twice or out of the image.
parallels_open() calls those correctable and repairs them, so they must
not abort the check that is meant to do the repairing.

Based on the original work from Alexander Ivanov.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Alexander Ivanov f646df4868 parallels: Create used bitmap even if checks needed
All the checks were fixed to work with used bitmap. Create used bitmap in
parallels_open() even if need_check is true.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev 739a8df763 parallels: do not let the check die on what it is meant to report
parallels_check_duplicate() sizes its local bitmap by the end of the
payload and asserts that every BAT entry fits into it. An entry
pointing at a cluster which runs past the end of the image file does
not fit, so a plain 'qemu-img check' on such an image dies:

    qemu-img: block/parallels.c:843: parallels_check_duplicate:
    Assertion `ret != -E2BIG' failed.

A repairing check survives by accident, as
parallels_check_outside_image() clears the entry before the duplicate
check gets to see it. There is nothing for the duplicate check to do
with such a cluster anyway, as the corruption has already been
reported, so skip it.

The answer parallels_mark_used() gives has to stay out of ret, which is
what the function returns once the loop is over. -EBUSY for the
duplicate this function exists to find is left in ret whenever the last
allocated BAT entry is the duplicated one and the check is not
repairing. 'qemu-img check' then ends with

    ERROR duplicate offset in BAT entry 1
    qemu-img: Check failed: Device or resource busy

and prints no summary at all, so the corruption it just found is
reported as a failure to look. Keep the answer in a variable of its
own, as the only errors worth returning from here are the I/O ones,
and those leave the loop where they happen.

The tests write two clusters and damage the second entry, one by
cutting the cluster in half so that it no longer fits the file, one by
pointing it at the first cluster.

Fixes: a398275e88 ("parallels: create mark_used() helper which sets bit in used bitmap")
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev 70615b7966 parallels: Move host clusters allocation to a separate function
For parallels images extensions we need to allocate host clusters
without any connection to BAT. Move host clusters allocation code to
parallels_allocate_host_clusters().

This function can be called not only from coroutines so all the
*_co_* functions were replaced by corresponding wrappers.

Add parallels_mark_unused(), the helper releasing an area in the used
bitmap, as the new function needs it to undo an allocation.

The size of the request and the size of the area preallocated for it
live in two variables here, where the code being moved kept them in
one. The used bitmap has to grow by the latter, as it is what tells
the allocator how far the image reaches: counting only the requested
clusters hides the preallocated tail, so the next allocation starts
over at the end it knows about and preallocates the very same space
again.

data_end has to grow past an allocation which lands in the space
preallocated by an earlier one as well, not only past one which appends
to the image. The field marks the end of the payload for the truncation
on inactivation, so an allocation which leaves it behind is cut off the
image the moment the node is closed, and the data written into it is
lost.

Based on the original work from Alexander Ivanov.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev c2a35d4b05 parallels: Limit search in parallels_mark_used to the last marked cluster
There is no necessity to search to the end of the bitmap. Limit the
search area as cluster_index + count.

Add cluster_end variable to avoid its calculation in a few places.

Based on the original work from Alexander Ivanov.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Alexander Ivanov 8cce36c0f2 parallels: Make mark_used() a global function
We will need this function and a function for marking unused clusters (will
be added in the next patch) in parallels-ext.c too. Let it be a global
function parallels_mark_used().

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev 22999fd6a0 parallels: split inactivation out and add the activation counterpart
We are going to add parallels image extensions storage and need a
separate function for the inactivation code, which parallels_close()
carries inline today. Move it into parallels_inactivate() and register
it as .bdrv_inactivate, so the image is written out when the node is
inactivated and not only when it is closed.

The condition guarding the old call moves inside, as the handler is now
reached for any node: bdrv_inactivate_recurse() calls it and
blockdev-set-active can ask for it at any time. There is nothing to
write out for a node we can not write to, and trying turns a request
which has nothing to do into "Failed to inactivate node:
Operation not permitted".

Clearing the in use flag now needs someone to set it again.
parallels_open() is the only place doing that, and it does not run when
a node is made active again, which would leave an image whose header
says it was closed correctly while it is open for writing: a crash then
looks like a clean shutdown and the repair on open is skipped. Add
parallels_co_invalidate_cache(), the counterpart of the above, which
arms the flag again and reports through errp when it can not.

The flag may only be cleared once everything which can still fail has
succeeded, as a failed inactivation leaves the node writable and the
image has to keep saying so. The order is therefore the reverse of the
code being moved: truncate first, write the header last and answer with
the result of that write instead of ignoring it.

The migration blocker stays, though its comment asks for it to go once
an activate method exists. An activated node needs more than the in use
flag to be correct, as the used cluster bitmap describes the image as
it was before the handover.

Based on the original work from Alexander Ivanov.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Alexander Ivanov 7fbf48446d parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap()
After used bitmap freeng s->used_bmap points to the freed memory. If we try
to free used bitmap one more time it leads to double free error.

Set s->used_bmap to NULL to exclude double free error.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:11 +02:00
Denis V. Lunev 6b1019e937 parallels: fix GSList leak on the format extension success path
parallels_parse_format_extension() returns directly on the end of
features marker, leaking the list which tracks the loaded bitmaps. The
bitmaps themselves belong to the block driver state, only the list
nodes are lost.

Fixes: baefd97700 ("parallels: support bitmap extension for read-only mode")
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:08 +02:00
Denis V. Lunev c6d10873db parallels: allocate the Format Extension cluster gracefully
s->cluster_size comes from the image and qemu_blockalign() aborts when
the allocation cannot be satisfied, so use the try variant and report
the failure instead.

The two are not interchangeable: qemu_try_blockalign() asserts that the
alignment is non zero, while qemu_blockalign() reaches
qemu_try_memalign(), which quietly raises a zero alignment to
sizeof(void *). bs->bl is only filled in once .bdrv_open() returns, so
the alignment has to come from bs->file, which is the node the cluster
is read through anyway and what parallels_open() uses for the header.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:08 +02:00
Denis V. Lunev 963f2ba200 parallels: reject a Format Extension outside the image file
The Format Extension offset and the cluster size both come from the
image header and neither is checked against the image file. Reject the
image rather than reading a cluster which is not there.

Fixes: baefd97700 ("parallels: support bitmap extension for read-only mode")
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:08 +02:00
Denis V. Lunev 4684bf187f parallels: bound the bitmap L1 table against the bitmap size
parallels_load_bitmap_data() derives the number of bytes to
deserialize from "bm_size - offset" without checking that the offset
is still inside the bitmap, and an offset past the end makes that
subtraction underflow.

The overflow which used to produce such an offset is fixed by
"dirty-bitmap: fix integer overflow in serialization coverage", but
both the cluster size and the L1 contents come from the image, so
refuse the table explicitly. The check cannot reject a valid table:
l1_size is DIV_ROUND_UP(bm_size, limit), so the largest offset the
loop reaches is (l1_size - 1) * limit, always below bm_size.

Fixes: baefd97700 ("parallels: support bitmap extension for read-only mode")
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:08 +02:00
Denis V. Lunev 29e74d939c parallels: validate dirty bitmap granularity
bf.granularity comes from the image and is passed to
bdrv_create_dirty_bitmap(), which asserts on it. An otherwise valid
image thus aborts qemu-img for a granularity of zero or one which is
not a power of two. The shift by BDRV_SECTOR_BITS is done on a
uint32_t as well, so 1 << 23 sectors and above wrap to zero and hit
the same assertion.

Compute the granularity in 64 bits and reject what
bdrv_create_dirty_bitmap() cannot accept.

Fixes: baefd97700 ("parallels: support bitmap extension for read-only mode")
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:08 +02:00
Denis V. Lunev 27d2c7a561 parallels: fix out-of-bounds read in format extension parsing
parallels_parse_format_extension() advances the cursor over the feature
payload but subtracts only the feature header size from the remaining
byte count. A feature whose payload reaches the end of the extension
cluster leaves the cursor at the end of the allocation while the count
still allows one more header, which the next iteration then reads out
of bounds, 0 bytes after the 512 byte cluster. The stale count also
breaks the data_size bound of any further feature, so
parallels_load_bitmap() can read past the cluster as well.

Account the aligned payload in both the cursor and the count. Aligning
data_size before the bound check changes nothing, as the count is
always a multiple of 8, but it has to be computed in 64 bits: on a
uint32_t a data_size of 0xfffffff9 or above wraps to zero.

Reported-by: Martin Holeček
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4156
Fixes: baefd97700 ("parallels: support bitmap extension for read-only mode")
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
2026-09-10 23:48:08 +02:00
Marc-André Lureau d40eed21e5 block/throttle-groups: fix throttle properties typename
The getter and setter use visit_type_int64, not visit_type_int.

Fixes: 432d889e55 ("block: convert ThrottleGroup to object with QOM")
Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2026-09-08 19:06:36 +04:00
Peter Maydell 7eddb112ab Merge tag 'pull-nbd-2026-09-02' of https://gitlab.com/vsementsov/qemu into staging
block: nbd patches

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCgAdFiEEi5wmzbL9FHyIDoahVh8kwfGfefsFAmqX4VgACgkQVh8kwfGf
# efv1KA//XpVJjBnou5RRgPvpGeTJz5G0ouI78sWsYFbqFRz1P/lvbvkl40a9wAJy
# 5IAeNCAj1FtTKJ+GMTVWKfLKn4fVLWdhcPX+33gxpVLbqx5uHweqxKN38p+hAvcr
# PtyVWX6kMRXGAyL7s3wb5qcvGqH/hf9+RG8hK+2KbrHnDJw7MRaicq17U7FL4Fzg
# EKxQMwGsjPEBGZYWSbYj6TfZI1sglfFifBEv5Y+kC2pWgVmmmXNRqXZcRPGNVBNS
# xMYus/nYPZAD3IdomCoQIERoccY5/mavy4UBQ2RdTNow4PiCMnjo+kRJRTrstBAE
# LokC9nUVs0hQkAd7Cq+M6aqXREJDfg871S4hYZmdpAsi6lIJPx6JvnjZ9NbO1TQY
# xvAThopZTA6qxgVc88K26/zwnJf3+tKgQ4JAKB7Pt25hPc4BJiYA/Uo8vIR7pu9H
# o1R+DAMWETv/x8J17+pbH8OgjqSe/KzptgwEi94lplz5lOTJOGvHz/5njPam6AsC
# 40DGtGG+ZoE7GTIKg2C3iYce+K0FtcM0ANuWj99bVO8vNdpKuv0TsCHLPSFViYev
# v37s20UVCX7AO4Q2G868BWUPwLFXSI+ENiT6Xbiwa3hBLGZK5fjqWwJxJVwSOA3q
# 3LpRNgCoDDO0kjkwqOh9000OjQhaBHOftax3mzW++Fb+TizW1n4=
# =IST8
# -----END PGP SIGNATURE-----
# gpg: Signature made Wed Sep  2 09:42:00 2026 BST
# gpg:                using RSA key 8B9C26CDB2FD147C880E86A1561F24C1F19F79FB
# gpg: Good signature from "Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>" [unknown]
# gpg:                 aka "Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 8B9C 26CD B2FD 147C 880E  86A1 561F 24C1 F19F 79FB

* tag 'pull-nbd-2026-09-02' of https://gitlab.com/vsementsov/qemu:
  iotests/nbd-commands: cover the command flags and sparse replies
  iotests/nbd-commands: cover NBD_CMD_BLOCK_STATUS with a payload
  iotests/nbd-commands: exercise the simple and structured reply modes
  nbd/server: accept NBD_CMD_CACHE above the maximum payload size
  iotests: add coverage for NBD transmission commands
  block/nbd: clear reply.cookie under receive_mutex
  block/nbd: never index requests[] with an unchecked cookie
  block/nbd: clear reply.cookie when the reply is rejected

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2026-09-03 09:43:07 +01:00
Peter Maydell a925240509 Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging
Pull request

Fixes from Daniil Tatianin and Niklas Cassel. No new features.

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCgAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAmqXHCUACgkQnKSrs4Gr
# c8iLKAf+PkEQHOWxATO63o96IcWC5v27Jknnxo8vlTZIZBECioTU/HZfE8g9hZv6
# vZo9ywyCVv4l0V5x/1/MZmlPgYCLJYNyOjD9dFkpKo2lgIRvAIdS7WkJKYc3fEtV
# XtoTqVR+uOFC8QAgIV8QSFyvhb6DFSDUZyXGRghcfn4QPSWZsV6WMuiO5nCsW+eV
# rhhG30l5G2ycbz0OP55H3LMr6jDqfVUrFa1H2GXqU6gj04M3E+oGAQo7AsK1a7VB
# 6/IenqJGuOyySpaCqFKJvEz7A6gi7B0K1PAOm8G7XkIzdgHoys+ULZWFx4veet3b
# QMH1uizI9ovattodHpJ3GTV7rGAdtw==
# =Ayma
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue Sep  1 19:40:37 2026 BST
# gpg:                using RSA key 8695A8BFD3F97CDAAC35775A9CA4ABB381AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" [full]
# gpg:                 aka "Stefan Hajnoczi <stefanha@gmail.com>" [full]
# Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35  775A 9CA4 ABB3 81AB 73C8

* tag 'block-pull-request' of https://gitlab.com/stefanha/qemu:
  file-posix: fix zone write granularity assignment for zoned block devices
  tests/../test_hotplug_blk: drop unused nic
  file-posix: fix cache.direct=on check for zoned block devices

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2026-09-02 13:40:43 +01:00
Denis V. Lunev cd3be3bd96 block/nbd: clear reply.cookie under receive_mutex
s->reply is documented as protected by s->receive_mutex, but the cookie
is cleared without it once the owning request has consumed its chunk.
A waiter in nbd_receive_replies() inspects the very same field under
the mutex, and does so with two separate loads:

    if (s->reply.cookie != 0) {
        ind2 = COOKIE_TO_INDEX(s->reply.cookie);
        assert(!s->requests[ind2].receiving);

Nothing keeps those two loads consistent. If the owner clears the
cookie in between, the second one reads 0, COOKIE_TO_INDEX() turns it
into an index of -1, and s->requests[] is accessed out of bounds:

  Assertion `!s->requests[ind2].receiving' failed.

  (gdb) p cookie
  $1 = 8
  (gdb) p s->reply.cookie
  $2 = 0
  (gdb) p &((NBDClientRequest *)s->requests)[-1].receiving
  $3 = (_Bool *) 0x5555558416c0
  (gdb) p &s->in_flight
  $4 = (unsigned int *) 0x5555558416c0
  (gdb) p s->in_flight
  $5 = 8

The cookie we wait for is 8, yet reply.cookie reads 0 one line after it
was found non-zero, so the index is -1. requests[-1].receiving lands on
in_flight, which is non-zero while requests are outstanding, and that is
what the assertion trips over.

Hitting this requires two coroutines of one NBD node to run in
different threads, as there is no yield point between the two loads
for the owner to squeeze into. A multiqueue configuration provides
exactly that, with the virtqueues of one disk spread over several
iothreads. Note that a compiler is free to merge the two loads into
one, in which case the race is invisible, so builds with reduced
optimization are much more likely to trip over it.

Accessing s->reply without the mutex is fine for the coroutine that
owns the reply: a non-zero cookie makes the field private to it.
Releasing that ownership is not, as it races with the waiters which
are explicitly allowed to look at the cookie. Clear it under the
mutex, in the same critical section as the wakeup, and make
nbd_recv_coroutines_wake() caller-locked, as CoMutex is not
recursive. It has a single caller.

The added acquisition cannot block behind the header read in
nbd_receive_replies(), because that path is only reachable with
reply.cookie == 0 while we still own a non-zero cookie. Merging the
clear with the wakeup also keeps a newcomer from starting a header
read in between, which would stall this already completed request for
the duration of that read.

There is no cookie to own when we get here after an error, and then a
newcomer can indeed be inside that read. It does not hold us for long
either, as the channel has been shut down before the error was
reported, so the read it sits in returns right away.

Fixes: 4ddb5d2fde ("block/nbd: drop connection_co")
Cc: Eric Blake <eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20260820115228.587427-4-den@openvz.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
2026-09-02 11:39:46 +03:00
Denis V. Lunev 75b0a993da block/nbd: never index requests[] with an unchecked cookie
Cookies are converted into indices of s->requests[] in several places
and the result is used right away, without any check:

    int i = COOKIE_TO_INDEX(cookie);
    ...
    return nbd_co_receive_offset_data_payload(s, s->requests[i].offset,

COOKIE_TO_INDEX() subtracts one, so a zero cookie becomes an index of
-1 and the access lands in front of the array. This is undefined and,
depending on the type of the index and on what the compiler has put
there, it can as well pass silently: at the site above i is signed, so
even a plain i < MAX_NBD_REQUESTS check would happily let -1 through.

Route every conversion through a helper which hands out the slot only
for a cookie in range and owned by a request in flight, so that the
check can not be forgotten again. The helper reports through an Error,
as the one cookie which is not ours to trust, the one taken from the
wire in nbd_receive_replies(), is a protocol error rather than an
internal inconsistency and has to stay a channel error. Every other
cookie is one we have issued and still own, so a bad value there is a
bug in this file, which is what &error_abort spells out.

The cookie of the reply in flight goes through the helper as well. It
is not an unchecked value either: it has passed the check on the wire
path, or it has been cleared by the previous commit.

Cc: Eric Blake <eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20260820115228.587427-3-den@openvz.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
2026-09-02 11:39:46 +03:00
Denis V. Lunev 9998d225bb block/nbd: clear reply.cookie when the reply is rejected
nbd_receive_replies() reads a reply header into s->reply and, when the
header turns out to be unusable, reports a channel error and returns
without touching it. The cookie stays there until the request which
owns the reply clears it, and until then the waiters are explicitly
allowed to look at a cookie which is not theirs:

    if (s->reply.cookie != 0) {
        ind2 = COOKIE_TO_INDEX(s->reply.cookie);
        assert(!s->requests[ind2].receiving);

Two of the error paths leave a value chosen by the server behind: one
returns before the cookie is validated at all, the other returns
because that validation has failed. A waiter which picks such a cookie
up turns it into an index which is not in requests[] and accesses the
array out of bounds, at an offset the server controls.

The reply is of no use to anybody at this point, so clear the cookie
before the mutex is released and keep the invariant that a non-zero
s->reply.cookie is always an index of a live request.

Observing the stale cookie takes a second thread, which a multiqueue
configuration provides. Within one AioContext there is no yield point
between the failed read and the clearing done by the owner in
nbd_co_receive_one_chunk(), so nothing else of this node runs in
between. The parked waiters cannot see it either, as they are woken
only after the cookie has been cleared. What can get in is a request
entering nbd_receive_replies() afresh, one just sent or one back for
its next reply chunk, because that path takes the mutex without
looking at the state.

Cc: Eric Blake <eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20260820115228.587427-2-den@openvz.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
2026-09-02 11:39:46 +03:00
Niklas Cassel 8e02b06882 file-posix: fix zone write granularity assignment for zoned block devices
In the ZBC and ZAC specifications, the requirement is that the write
granularity matches the physical block size.

However, in the ZNS specification, the requirement is simply that the
write granularity matches the logical block size.

See:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a805a4fa4fa376bbc145762bb8b09caa2fa8af48

In Linux, there is a zone_write_granularity sysfs property which abstracts
this away, so let's make use of it.

Currently, it is theoretically possible that QEMU presents an inflated
write granularity for NVMe devices where the physical block size is larger
than the logical block size.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Fixes: a3c41f06d5 ("file-posix: add tracking of the zone write pointers")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2026-09-01 14:38:03 -04:00
Niklas Cassel 7c12177aa3 file-posix: fix cache.direct=on check for zoned block devices
For zoned block devices with SWR zones, the order of the writes must be
maintained to avoid write not at write pointer errors.
Thus, cache.direct=on must be used to bypass the page cache, since the
page cache uses write-back caching, where the physical order of the writes
is not guaranteed.

There is already a check for cache.direct=on in raw_open_common(),
however, this check is done before raw_refresh_zoned_limits() has been
called (which initializes bs->bl.zoned), so it is currently dead code.

Fix this by moving the check to raw_refresh_zoned_limits(), such that the
check is done after bs->bl.zoned has been initialized.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
Fixes: 774c726ceb ("block: add zoned BlockDriver check to block layer")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Message-ID: <20260617133658.2022750-1-cassel@kernel.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2026-09-01 14:26:10 -04:00
Marc-André Lureau 34c5de5c6d build-sys: make HMP source files conditional on have_hmp
Exclude all *-hmp-cmds.c files and HMP-only source files from the
build when HMP is disabled.

Also conditionalise the hmp-commands.hx header generation and the
test-hmp qtest.

Acked-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20260828-qemu-no-hmp-v5-45-9227de146347@redhat.com>
2026-08-29 12:07:45 +04:00
Marc-André Lureau f3900cfb0c monitor: tighten monitor_printf*()
Rename monitor_printf->monitor_hmp_printf, monitor_vprintf->
monitor_hmp_vprintf, and monitor_printc->monitor_hmp_printc, changing
the first parameter from Monitor * to MonitorHMP * to enforce type
safety. The implementation is also simplified: monitor_hmp_vprintf now
directly calls g_strdup_vprintf + monitor_puts, removing the virtual
dispatch via moncls->vprintf.

The dev_print() callbacks are temporarily using the MONITOR_HMP(mon)
cast, they are fixed in the following commits.

Early return in qemu_vprintf() if "hmp" is NULL, relying on
monitor_hmp_vprintf() handling NULL case is a bit uncommon.

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20260828-qemu-no-hmp-v5-37-9227de146347@redhat.com>
2026-08-29 12:07:45 +04:00
Marc-André Lureau 06c1600b55 monitor: make hmp_handle_error() take MonitorHMP
Mostly mechanical, with a few adjustments around it.

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20260828-qemu-no-hmp-v5-32-9227de146347@redhat.com>
2026-08-29 12:07:44 +04:00
Marc-André Lureau 139ffc1039 monitor: change HMPCommand cmd to take MonitorHMP
HMP commands are specific to the HMP monitor, so make the cmd function
pointer in HMPCommand take MonitorHMP * instead of Monitor *. This
strengthens type safety and makes the HMP-specific nature of these
handlers explicit.

Handler functions are renamed to take MonitorHMP *hmp, with a local
Monitor *mon = MONITOR(hmp) added where the body needs the base type.
Following patches will drop it, since most functions are HMP specific.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20260828-qemu-no-hmp-v5-31-9227de146347@redhat.com>
2026-08-29 12:07:44 +04:00
Marc-André Lureau 1f8d06ef42 block: add x-qemu-io QMP command
Add an x-qemu-io QMP command that runs qemu-io commands on block
devices. The command accepts a device name (block backend name,
node-name, or qdev ID) and a qemu-io command string.

Refactor hmp_qemu_io() to be a thin wrapper around the new QMP
command, following the standard HMP-over-QMP pattern used by other
block commands.

This change is also required for the qtest qemu-io command in the
following patch.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20260828-qemu-no-hmp-v5-22-9227de146347@redhat.com>
2026-08-29 12:07:44 +04:00
Marc-André Lureau 3c0c7bfeba qemu-io: propagate errors through Error API instead of printf
Convert the qemu-io command infrastructure to use QEMU's Error API
(Error **errp) for error propagation instead of printing directly to
stdout/stderr via printf/fprintf.

The cfunc_t typedef, all ~35 command functions, the dispatcher chain
(command/init_check_command/qemuio_command), and helper functions
(parse_pattern, create_iovec, qemu_io_alloc_from_file) all gain an
Error **errp parameter. Async completion callbacks (aio_read_done,
aio_write_done, aio_discard_done) use error_report() since they have
no Error path back to the caller.

Update tests:
- error_report_err() prepends qemu-io: prefix
- copy-before-write: check the HMP return value, rather than stdio
- other tests: update to check JSON {"return": "Error: error text"}

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20260828-qemu-no-hmp-v5-21-9227de146347@redhat.com>
2026-08-29 12:07:44 +04:00
Marc-André Lureau a22c9e5056 hmp: fix snapshot_blkdev argument type
The `snapshot-file` argument for `snapshot_blkdev` is mandatory. Correct
the `args_type` definition to reflect this.

Fixes: 6106e2492f ("qapi: Convert blockdev_snapshot_sync")
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20260828-qemu-no-hmp-v5-4-9227de146347@redhat.com>
2026-08-29 12:07:43 +04:00
Denis V. Lunev dc04053687 dirty-bitmap: fix integer overflow in serialization coverage
The chunk size is an int and is shifted left by 3 before the result is
widened, so a chunk size of 1 << 28 or above overflows.

parallels passes s->cluster_size, which parallels_open() lets reach
2 GiB. With a bitmap needing two L1 entries the bogus limit makes the
"bm_size - offset" in parallels_load_bitmap_data() underflow; both
wrong values slip past the assertions in serialization_chunk() and the
resulting index lands outside the hbitmap, so a 128 KiB image memsets
unrelated memory through hbitmap_deserialize_ones().

Widen the shift. qcow2, the only other caller, never exceeds a 2 MiB
cluster.

Fixes: 35f428ba39 ("qcow2-bitmap: make bytes_covered_by_bitmap_cluster() public")
Cc: Eric Blake <eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Message-ID: <20260811173857.396571-4-den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
2026-08-17 18:32:44 +03:00