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>
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>
* 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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>