Michael Tokarev [Sat, 31 May 2025 17:15:45 +0000 (20:15 +0300)]
qemu-img: global option processing and error printing
In order to correctly print executable name in various
error messages, pass argv[0] to error_exit() function.
This way, error messages will refer to actual executable
name, which may be different from 'qemu-img'.
For subcommands, pass original command name from the
qemu-img argv[0], plus the subcommand name, as its own
argv[0] element, so error messages can be more useful.
Also don't require at least 3 options on the command
line: it makes no sense with options before subcommand.
Introduce tryhelp() function which just prints
try 'command-name --help' for more info
and exits. When tryhelp() is called from within a subcommand
handler, the message will look like:
try 'command-name subcommand --help' for more information
qemu-img uses getopt_long() with ':' as the first char in
optstring parameter, which means it doesn't print error
messages but return ':' or '?' instead, and qemu-img uses
unrecognized_option() or missing_argument() function to
print error messages. But it doesn't quite work:
so the aim is to let getopt_long() to print regular error
messages instead (removing ':' prefix from optstring) and
remove handling of '?' and ':' "options" entirely. With
concatenated argv[0] and the subcommand, it all finally
does the right thing in all cases. This will be done in
subsequent changes command by command, with main() done
last.
unrecognized_option() and missing_argument() functions
prototypes aren't changed by this patch, since they're
called from many places and will be removed a few patches
later. Only artifical "qemu-img" argv0 is provided in
there for now.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250531171609.197078-4-mjt@tls.msk.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Michael Tokarev [Sat, 31 May 2025 17:15:44 +0000 (20:15 +0300)]
qemu-img: create: convert img_size to signed, simplify handling
Initializing an unsigned as -1, or using temporary
sval for conversion is awkward. Since we don't allow
other "negative" values anyway, use signed value and
pass it to bdrv_img_create() (where it is properly
converted to unsigned), simplifying code.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250531171609.197078-3-mjt@tls.msk.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Michael Tokarev [Sat, 31 May 2025 17:15:43 +0000 (20:15 +0300)]
qemu-img: measure: convert img_size to signed, simplify handling
qemu_opt_set_number() expects signed int64_t.
Use int64_t instead of uint64_t for img_size, use -1 as "unset"
value instead of UINT64_MAX, and do not require temporary sval
for conversion from string.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250531171609.197078-2-mjt@tls.msk.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Thu, 5 Jun 2025 10:09:38 +0000 (12:09 +0200)]
iotests: add test for changing the 'drive' property via 'qom-set'
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250605100938.43133-1-f.ebner@proxmox.com>
[kwolf: Fixed up pylint warnings flagged by 297] Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Kevin Wolf [Wed, 25 Jun 2025 08:50:19 +0000 (10:50 +0200)]
file-posix: Fix aio=threads performance regression after enablign FUA
For aio=threads, we're currently not implementing REQ_FUA in any useful
way, but just do a separate raw_co_flush_to_disk() call. This changes
behaviour compared to the old state, which used bdrv_co_flush() with its
optimisations. As a quick fix, call bdrv_co_flush() again like before.
Eventually, we can use pwritev2() to make use of RWF_DSYNC if available,
but we'll still have to keep this code path as a fallback, so this fix
is required either way.
While the fix itself is a one-liner, some new graph locking annotations
are needed to convince TSA that the locking is correct.
Cc: qemu-stable@nongnu.org Fixes: 984a32f17e8d ("file-posix: Support FUA writes") Buglink: https://issues.redhat.com/browse/RHEL-96854 Reported-by: Tingting Mao <timao@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250625085019.27735-1-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
block/qapi: make @node-name in @BlockDeviceInfo non-optional
Since commit 15489c769b ("block: auto-generated node-names"), if the
node name of a block driver state is not explicitly specified, it
will be auto-generated.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250702123204.325470-3-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
block/qapi: include child references in block device info
In combination with using a throttle filter to enforce IO limits for
a guest device, knowing the 'file' child of a block device can be
useful. If the throttle filter is only intended for guest IO, block
jobs should not also be limited by the throttle filter, so the
block operations need to be done with the 'file' child of the top
throttle node as the target. In combination with mirroring, the name
of that child is not fixed.
Another scenario is when unplugging a guest device after mirroring
below a top throttle node, where the mirror target is added explicitly
via blockdev-add. After mirroring, the target becomes the new 'file'
child of the throttle node. For unplugging, both the top throttle node
and the mirror target need to be deleted, because only implicitly
added child nodes are deleted automatically, and the current 'file'
child of the throttle node was explicitly added (as the mirror
target).
In other scenarios, it could be useful to follow the backing chain.
Note that iotests 191 and 273 use _filter_img_info, so the 'children'
information is filtered out there.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250702123204.325470-2-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:25 +0000 (17:11 +0200)]
blockjob: mark block_job_remove_all_bdrv() as GRAPH_UNLOCKED
The function block_job_remove_all_bdrv() calls
bdrv_graph_wrlock_drained(), which must be called with the graph
unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-49-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:24 +0000 (17:11 +0200)]
block: mark bdrv_open_child_common() and its callers GRAPH_UNLOCKED
The function bdrv_open_child_common() calls
bdrv_graph_wrlock_drained(), which must be called with the graph
unlocked. Mark it and its two callers bdrv_open_file_child() and
bdrv_open_child() as GRAPH_UNLOCKED. This requires temporarily
unlocking in vmdk_parse_extents() and making the locked section
shorter in vmdk_open().
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-48-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:23 +0000 (17:11 +0200)]
block: mark bdrv_close() as GRAPH_UNLOCKED
The functions blk_log_writes_close(), blkverify_close(),
quorum_close(), vmdk_close() via vmdk_free_extents(), and other
bdrv_close() implementations call bdrv_graph_wrlock_drained(), which
must be called with the graph unlocked. They are reached via the
BlockDriver's bdrv_close() callback and the bdrv_close() wrapper,
which are also marked as GRAPH_UNLOCKED_PTR and GRAPH_UNLOCKED.
Furthermore, the function bdrv_close() also calls bdrv_drained_begin()
and bdrv_graph_wrlock_drained(), so there are additional reasons for
marking it GRAPH_UNLOCKED.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-47-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:22 +0000 (17:11 +0200)]
block: mark bdrv_close_all() as GRAPH_UNLOCKED
The function bdrv_close_all() calls bdrv_drain_all(), which must be
called with the graph unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-46-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:21 +0000 (17:11 +0200)]
block: mark bdrv_drop_intermediate() as GRAPH_UNLOCKED
The function bdrv_drop_intermediate() calls bdrv_drained_begin(),
which must be called with the graph unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-45-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:20 +0000 (17:11 +0200)]
block: mark bdrv_insert_node() as GRAPH_UNLOCKED
The function bdrv_insert_node() calls bdrv_drained_begin() which must
be called with the graph unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-44-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:19 +0000 (17:11 +0200)]
block: mark bdrv_replace_child_bs() as GRAPH_UNLOCKED
The function bdrv_replace_child_bs() calls bdrv_drained_begin() which
must be called with the graph unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-43-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Kevin Wolf [Mon, 14 Jul 2025 13:01:53 +0000 (15:01 +0200)]
block: Allow bdrv_new() with and without graph lock
bdrv_new() calls bdrv_drained_begin(), which can poll and therefore
can't be called while holding the graph lock. One option to make sure
that this call is allowed would be marking bdrv_new() GRAPH_UNLOCKED.
However, this is actually an unnecessary restriction because we know
that we only just created the BlockDriverState and it isn't even part of
the graph yet. We can use bdrv_do_drained_begin_quiesce() instead to
avoid the polling, which means that bdrv_new() can now safely be called
from callers that hold the graph lock as well as from callers that
don't.
Fiona Ebner [Fri, 30 May 2025 15:11:17 +0000 (17:11 +0200)]
block/commit: mark commit_abort() as GRAPH_UNLOCKED
The function commit_abort() calls bdrv_drained_begin(), which must be
called with the graph unlocked.
Also mark the JobDriver's abort() callback as GRAPH_UNLOCKED_PTR,
because that is the callback via which commit_abort() is reached.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-41-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:16 +0000 (17:11 +0200)]
block-backend: mark blk_io_limits_disable() as GRAPH_UNLOCKED
The function blk_io_limits_disable() calls bdrv_drained_begin(), which
must be called with the graph unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-40-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:15 +0000 (17:11 +0200)]
block: mark blk_drain() as GRAPH_UNLOCKED
The function blk_drain() calls bdrv_drained_begin(), which must be
called with the graph unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-39-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:14 +0000 (17:11 +0200)]
block: mark blk_remove_bs() as GRAPH_UNLOCKED
The function blk_remove_bs() calls bdrv_graph_wrlock_drained() and can
also call bdrv_drained_begin(), both of which which must be called with
the graph unlocked.
Marking blk_remove_bs() as GRAPH_UNLOCKED requires temporarily
unlocking in hmp_drive_del().
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-38-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:13 +0000 (17:11 +0200)]
block: mark bdrv_inactivate_all() as GRAPH_UNLOCKED
The function bdrv_inactivate_all() calls bdrv_drain_all_begin(), which
must be called with the graph unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-37-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:12 +0000 (17:11 +0200)]
block: mark bdrv_inactivate() as GRAPH_RDLOCK and move drain to callers
The function bdrv_inactivate() calls bdrv_drain_all_begin(), which
needs to be called with the graph unlocked, so either
bdrv_inactivate() should be marked as GRAPH_UNLOCKED or the drain
needs to be moved to the callers. The caller in
qmp_blockdev_set_active() requires that the locked section covers
bdrv_find_node() too, so the latter alternative is chosen.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-36-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:11 +0000 (17:11 +0200)]
block: mark bdrv_reopen_queue() and bdrv_reopen_multiple() as GRAPH_UNLOCKED
The function bdrv_reopen_queue() can call bdrv_drain_all_begin(),
which must be called with the graph unlocked.
The function bdrv_reopen_multiple() calls bdrv_reopen_prepare() which
must be called with the graph unlocked.
To mark bdrv_reopen_queue() as GRAPH_UNLOCKED, it is necessary to make
the locked section in reopen_backing_file() shorter.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-35-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:10 +0000 (17:11 +0200)]
block/stream: mark stream_prepare() as GRAPH_UNLOCKED
The function stream_prepare() calls bdrv_drain_all_begin(), which
must be called with the graph unlocked.
Also mark the JobDriver's prepare() callback as GRAPH_UNLOCKED_PTR,
because that is the callback via which stream_prepare() is reached.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-34-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:09 +0000 (17:11 +0200)]
block/snapshot: mark bdrv_all_delete_snapshot() as GRAPH_UNLOCKED
The function bdrv_all_delete_snapshot() calls bdrv_drain_all_begin(),
which must be called with the graph unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-33-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:08 +0000 (17:11 +0200)]
block-backend: mark blk_drain_all() as GRAPH_UNLOCKED
The function blk_drain_all() calls bdrv_drain_all_begin(), which must
be called with the graph unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-32-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:07 +0000 (17:11 +0200)]
block: drop wrapper for bdrv_set_backing_hd_drained()
Nearly all callers (outside of the tests) are already using the
_drained() variant of the function. It doesn't seem worth keeping.
Simply adapt the remaining callers of bdrv_set_backing_hd() and rename
bdrv_set_backing_hd_drained() to bdrv_set_backing_hd().
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-31-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:06 +0000 (17:11 +0200)]
blockdev: avoid locking and draining multiple times in external_snapshot_abort()
By using the appropriate variants bdrv_set_backing_hd_drained() and
bdrv_try_change_aio_context_locked(), there only needs to be a single
drained and write-locked section in external_snapshot_abort().
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-30-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:05 +0000 (17:11 +0200)]
block: mark bdrv_set_backing_hd() as GRAPH_UNLOCKED
The function bdrv_set_backing_hd() calls bdrv_drain_all_begin(), which
must be called with the graph unlocked.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-29-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:04 +0000 (17:11 +0200)]
block: call bdrv_set_backing_hd() while unlocked in bdrv_open_backing_file()
This is in preparation to mark bdrv_set_backing_hd() as
GRAPH_UNLOCKED.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-28-f.ebner@proxmox.com>
[kwolf: Removed an extra blank line] Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:03 +0000 (17:11 +0200)]
block/commit: switch to bdrv_set_backing_hd_drained() variant
This is in preparation to mark bdrv_set_backing_hd() as
GRAPH_UNLOCKED.
Switch to using the bdrv_set_backing_hd_drained() variant. For the
first pair of calls to avoid draining and locking twice in a row
within the individual calls. For the third call, so that the drained
and locked section can also cover bdrv_cow_bs().
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-27-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:02 +0000 (17:11 +0200)]
block/mirror: switch to bdrv_set_backing_hd_drained() variant
This is in preparation to mark bdrv_set_backing_hd() as
GRAPH_UNLOCKED.
Switch to using the bdrv_set_backing_hd_drained() variant, so that the
drained and locked section can also cover the calls to
bdrv_skip_filters() and bdrv_cow_bs().
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-26-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Many write-locked sections are also drained sections. A new
bdrv_graph_wrunlock_drained() wrapper around bdrv_graph_wrunlock() is
introduced, which will begin a drained section first. A global
variable is used so bdrv_graph_wrunlock() knows if it also needs
to end such a drained section. Both the aio_poll call in
bdrv_graph_wrlock() and the aio_bh_poll() in bdrv_graph_wrunlock()
can re-enter a write-locked section. While for the latter, ending the
drain could be moved to before the call, the former requires that the
variable is a counter and not just a boolean.
Since the wrapper calls bdrv_drain_all_begin(), which must be called
with the graph unlocked, mark the wrapper as GRAPH_UNLOCKED too.
The switch to the new helpers was generated with the following
commands and then manually checked:
find . -name '*.c' -exec sed -i -z 's/bdrv_drain_all_begin();\n\s*bdrv_graph_wrlock();/bdrv_graph_wrlock_drained();/g' {} ';'
find . -name '*.c' -exec sed -i -z 's/bdrv_graph_wrunlock();\n\s*bdrv_drain_all_end();/bdrv_graph_wrunlock();/g' {} ';'
Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-25-f.ebner@proxmox.com>
[kwolf: Removed redundant GRAPH_UNLOCKED] Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:11:00 +0000 (17:11 +0200)]
block: never use atomics to access bs->quiesce_counter
All accesses of bs->quiesce_counter are in the main thread, either
after a GLOBAL_STATE_CODE() macro or in a function with GRAPH_WRLOCK
annotation.
This is essentially a revert of 414c2ec358 ("block: access
quiesce_counter with atomic ops"). At that time, neither the
GLOBAL_STATE_CODE() macro nor the GRAPH_WRLOCK annotation existed.
Even if the field was only accessed in the main thread back then (did
not check if that is actually the case), it wouldn't have been easy to
verify.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-24-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Stefan Hajnoczi [Sun, 13 Jul 2025 05:46:04 +0000 (01:46 -0400)]
Merge tag 'pull-tcg-20250711' of https://gitlab.com/rth7680/qemu into staging
fpu: Process float_muladd_negate_result after rounding
tcg: Use uintptr_t in tcg_malloc implementation
linux-user: Hold the fd-trans lock across fork
linux-user: Implement fchmodat2 syscall
linux-user: Check for EFAULT failure in nanosleep
linux-user: Use qemu_set_cloexec() to mark pidfd as FD_CLOEXEC
linux-user/gen-vdso: Handle fseek() failure
linux-user/gen-vdso: Don't read off the end of buf[]
* tag 'pull-tcg-20250711' of https://gitlab.com/rth7680/qemu:
linux-user: Use qemu_set_cloexec() to mark pidfd as FD_CLOEXEC
tcg: Use uintptr_t in tcg_malloc implementation
linux-user: Hold the fd-trans lock across fork
linux-user/mips/o32: Drop sa_restorer functionality
linux-user/gen-vdso: Don't read off the end of buf[]
linux-user/gen-vdso: Handle fseek() failure
linux-user: Check for EFAULT failure in nanosleep
linux-user: Implement fchmodat2 syscall
fpu: Process float_muladd_negate_result after rounding
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Stefan Hajnoczi [Sun, 13 Jul 2025 05:45:30 +0000 (01:45 -0400)]
Merge tag 'migration-20250711-pull-request' of https://gitlab.com/farosas/qemu into staging
Migration pull request
- General cleanups around: postcopy, bg-snapshot, migration hooks,
migration completion and formatting of 'info migrate'.
- Overhaul of postcopy blocktime tracking.
# -----BEGIN PGP SIGNATURE-----
#
# iQJEBAABCAAuFiEEqhtIsKIjJqWkw2TPx5jcdBvsMZ0FAmhxGdgQHGZhcm9zYXNA
# c3VzZS5kZQAKCRDHmNx0G+wxnahoD/9uNXirlmRk3tDnhiJsiYx+HnXYPFEORSZq
# zlpUyqvhQ1POp3Fa5pRf+bJ5mmPw8h8PdOR2StMpnW2Xa1OatAZj5m1uityAVWOl
# EkVfZLl0j6j9HCCmE3c4dztOGIBsd9YY0GWizL05XHYZPrdX4zOpolMN4m53RwQY
# HUVD6T2y9eFDnCO6MsoA9EfmkFYCRvqlS0VzTcYzQFN4H+QHlcpDfweqJpTLPa+1
# trahAN9PBuMjoewjDqwkNkf0CLaCXHszAfj6yv62Vi8Cbp9DDPywIYJKFnxspElW
# Fjg1b4MdsbYZNmeKgIawzgTOL1RrojvKkoi7KWp3D7M+/ZZl9kBwQuUcBXKI7N0R
# Y0GNfkkTycn18nM0JU/6QWSuVeiPbLArxQUGP1cLgvcHSSNgD9JxWbNBu5+1fFOG
# Gg3qnyYatJ6xJDiCrdKqV8fwozNlm/G6b9BiCDeVq+4nA2OKQ0shiNA1GZHvVSQL
# X4uAPexETdHfA/LeA2w5sgVBEw7BewBdjLntZDIFsyBnLrvqrDcU5Aav0wiHoI8U
# QBC2aIpJfMLHiIQ93mVX96NltXC7KvJTIZVl3iwfiYEYCvQtTYgdJ09ELXFJYxFX
# XpTTazqpmPSfuZpPRgx9YbDP/kS8Fg/PTOlPeD0T/frFgd1S6Thh6OW455PavMp8
# ht2lE4sxjA==
# =vtRD
# -----END PGP SIGNATURE-----
# gpg: Signature made Fri 11 Jul 2025 10:04:08 EDT
# gpg: using RSA key AA1B48B0A22326A5A4C364CFC798DC741BEC319D
# gpg: issuer "farosas@suse.de"
# gpg: Good signature from "Fabiano Rosas <farosas@suse.de>" [unknown]
# gpg: aka "Fabiano Almeida Rosas <fabiano.rosas@suse.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: AA1B 48B0 A223 26A5 A4C3 64CF C798 DC74 1BEC 319D
* tag 'migration-20250711-pull-request' of https://gitlab.com/farosas/qemu: (26 commits)
migration: Rename save_live_complete_precopy_thread to save_complete_precopy_thread
migration/postcopy: Add latency distribution report for blocktime
migration/postcopy: blocktime allows track / report non-vCPU faults
migration/postcopy: Optimize blocktime fault tracking with hashtable
migration/postcopy: Cleanup the total blocktime accounting
migration/postcopy: Cache the tid->vcpu mapping for blocktime
migration/postcopy: Initialize blocktime context only until listen
migration/postcopy: Report fault latencies in blocktime
migration/postcopy: Add blocktime fault counts per-vcpu
migration/postcopy: Bring blocktime layer to ns level
migration/postcopy: Drop PostcopyBlocktimeContext.start_time
migration/postcopy: Make all blocktime vars 64bits
migration/postcopy: Drop all atomic ops in blocktime feature
migration/postcopy: Push blocktime start/end into page req mutex
migration: Add option to set postcopy-blocktime
migration/postcopy: Avoid clearing dirty bitmap for postcopy too
migration: Rewrite the migration complete detect logic
migration/ram: Add tracepoints for ram_save_complete()
migration/ram: One less indent for ram_find_and_save_block()
migration: qemu_savevm_complete*() helpers
...
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Stefan Hajnoczi [Sun, 13 Jul 2025 05:45:17 +0000 (01:45 -0400)]
Merge tag 'pull-target-arm-20250711' of https://gitlab.com/pm215/qemu into staging
target-arm queue:
* New board type max78000fthr
* Enable use of CXL on Arm 'virt' board
* Some more tidyup of ID register handling
* Refactor AT insns and PMU regs into separate source files
* Don't enforce NSE,NS check for EL3->EL3 returns
* hw/arm/fsl-imx8mp: Wire VIRQ and VFIQ
* Allow nested-virtualization with KVM on the 'virt' board
* system/qdev: Remove pointless NULL check in qdev_device_add_from_qdict
* hw/arm/virt-acpi-build: Don't create ITS id mappings by default
* target/arm: Remove unused helper_sme2_luti4_4b
* tag 'pull-target-arm-20250711' of https://gitlab.com/pm215/qemu: (36 commits)
tests/functional: Add a test for the MAX78000 arm machine
docs/system: arm: Add max78000 board description
target/arm: Remove helper_sme2_luti4_4b
hw/arm/virt-acpi-build: Don't create ITS id mappings by default
system/qdev: Remove pointless NULL check in qdev_device_add_from_qdict
hw/arm/virt: Allow virt extensions with KVM
hw/arm/arm_gicv3_kvm: Add a migration blocker with kvm nested virt
target/arm: Enable feature ARM_FEATURE_EL2 if EL2 is supported
target/arm/kvm: Add helper to detect EL2 when using KVM
hw/arm: Allow setting KVM vGIC maintenance IRQ
hw/arm/fsl-imx8mp: Wire VIRQ and VFIQ
target/arm: Don't enforce NSE,NS check for EL3->EL3 returns
target/arm: Split out performance monitor regs to cpregs-pmu.c
target/arm: Split out AT insns to tcg/cpregs-at.c
target/arm: Drop stub for define_tlb_insn_regs
arm/kvm: shorten one overly long line
arm/cpu: store clidr into the idregs array
arm/cpu: fix trailing ',' for SET_IDREG
arm/cpu: store id_aa64afr{0,1} into the idregs array
arm/cpu: store id_afr0 into the idregs array
...
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Stefan Hajnoczi [Sun, 13 Jul 2025 05:44:51 +0000 (01:44 -0400)]
Merge tag 'pull-request-2025-07-11' of https://gitlab.com/thuth/qemu into staging
* s390x: Allow to select different entries when booting via pxelinux.cfg
* Link s390-ccw.img statically
* Fix broken bamboo functional test
* s390x code cleanups and refactorings
* tag 'pull-request-2025-07-11' of https://gitlab.com/thuth/qemu:
target/s390x: Have s390_cpu_halt() not return anything
target/s390x: Expose s390_count_running_cpus() method
target/s390x: Remove unused s390_cpu_[un]halt() user stubs
tests/functional/test_ppc_bamboo: Replace broken link with working assets
tests/functional: Add dependency to the keymap_targets
pc-bios: Update the s390 bios images with the pxelinux.cfg loadparm changes
pc-bios/s390-ccw: link statically
tests/functional: Add a test for s390x pxelinux.cfg network booting
pc-bios/s390-ccw: Add a boot menu for booting via pxelinux.cfg
pc-bios/s390-ccw: Make get_boot_index() from menu.c global
pc-bios/s390-ccw: Allow up to 31 entries for pxelinux.cfg
pc-bios/s390-ccw: Allow to select a different pxelinux.cfg entry via loadparm
hw/s390x/s390-pci-bus.c: Use g_assert_not_reached() in functions taking an ett
target/s390x/tcg: Use vaddr in s390_probe_access()
target/s390x/kvm: Use vaddr in find/insert_hw_breakpoint()
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This has two problems:
(1) it doesn't check errors, which Coverity complains about
(2) we use F_GETFL when we mean F_GETFD
Deal with both of these problems by using qemu_set_cloexec() instead.
That function will assert() if the fcntls fail, which is fine (we are
inside fork_start()/fork_end() so we know nothing can mess around
with our file descriptors here, and we just got this one from
pidfd_open()).
(As we are touching the if() statement here, we correct the
indentation.)
Coverity: CID 1508111 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20250711141217.1429412-1-peter.maydell@linaro.org>
Juraj Marcin [Thu, 26 Jun 2025 08:52:32 +0000 (10:52 +0200)]
migration: Rename save_live_complete_precopy_thread to save_complete_precopy_thread
Recent patch [1] renames the save_live_complete_precopy handler to
save_complete, as the machine is not live in most cases when this
handler is executed. The same is true also for
save_live_complete_precopy_thread, therefore this patch removes the
"live" keyword from the handler itself and related types to keep the
naming unified.
In contrast to save_complete, this handler is only executed at the end
of precopy, therefore the "precopy" keyword is retained.
Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Cédric Le Goater <clg@redhat.com> Signed-off-by: Juraj Marcin <jmarcin@redhat.com> Link: https://lore.kernel.org/r/20250626085235.294690-1-jmarcin@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de>
Peter Xu [Fri, 13 Jun 2025 14:12:17 +0000 (10:12 -0400)]
migration/postcopy: Add latency distribution report for blocktime
Add the latency distribution too for blocktime, using order-of-two buckets.
It accounts for all the faults, from either vCPU or non-vCPU threads. With
prior rework, it's very easy to achieve by adding an array to account for
faults in each buckets.
Sample output for HMP (while for QMP it's simply an array):
Postcopy Latency Distribution:
[ 1 us - 2 us ]: 0
[ 2 us - 4 us ]: 0
[ 4 us - 8 us ]: 1
[ 8 us - 16 us ]: 2
[ 16 us - 32 us ]: 2
[ 32 us - 64 us ]: 3
[ 64 us - 128 us ]: 10169
[ 128 us - 256 us ]: 50151
[ 256 us - 512 us ]: 12876
[ 512 us - 1 ms ]: 97
[ 1 ms - 2 ms ]: 42
[ 2 ms - 4 ms ]: 44
[ 4 ms - 8 ms ]: 93
[ 8 ms - 16 ms ]: 138
[ 16 ms - 32 ms ]: 0
[ 32 ms - 65 ms ]: 0
[ 65 ms - 131 ms ]: 0
[ 131 ms - 262 ms ]: 0
[ 262 ms - 524 ms ]: 0
[ 524 ms - 1 sec ]: 0
[ 1 sec - 2 sec ]: 0
[ 2 sec - 4 sec ]: 0
[ 4 sec - 8 sec ]: 0
[ 8 sec - 16 sec ]: 0
Cc: Markus Armbruster <armbru@redhat.com> Acked-by: Dr. David Alan Gilbert <dave@treblig.org> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20250613141217.474825-15-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de>
When used to report page fault latencies, the blocktime feature can be
almost useless when KVM async page fault is enabled, because in most cases
such remote fault will kickoff async page faults, then it's not trackable
from blocktime layer.
After all these recent rewrites to blocktime layer, it's finally so easy to
also support tracking non-vCPU faults. It'll be even faster if we could
always index fault records with TIDs, unfortunately we need to maintain the
blocktime API which report things in vCPU indexes.
Of course this can work not only for kworkers, but also any guest accesses
that may reach a missing page, for example, very likely when in the QEMU
main thread too (and all other threads whenever applicable).
In this case, we don't care about "how long the threads are blocked", but
we only care about "how long the fault will be resolved".
Cc: Markus Armbruster <armbru@redhat.com> Cc: Dr. David Alan Gilbert <dave@treblig.org> Reviewed-by: Fabiano Rosas <farosas@suse.de> Tested-by: Mario Casquero <mcasquer@redhat.com> Link: https://lore.kernel.org/r/20250613141217.474825-14-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de>
The old algorithm was almost OK and fast on inserts, except that the lookup
is slow and won't scale if there are a lot of vCPUs: when a page is copied
during postcopy, mark_postcopy_blocktime_end() will walk the whole array
trying to find which vCPUs are blocked by the address. So it needs
constant O(N) walk for each page resolution.
Alexey (the author of postcopy blocktime) mentioned the perf issue and how
to optimize it in a piece of comment in the page resolution path. The
comment was (interestingly..) not complete, but it's relatively clear what
he wanted to say about this perf issue.
Issue 2: Wrong Accounting on re-entrancies
==========================================
People might think that each vCPU should only and always get one fault at a
time, so that when the blocktime layer captured one fault on one vCPU, we
should never see another fault message on this vCPU.
It's almost correct, except in some extreme rare cases.
Case 1: it's possible the fault thread processes the userfaultfd messages
too fast so it can see >1 messages on one vCPU before the previous one was
resolved.
Case 2: it's theoretically also possible one vCPU can get even more than
one message on the same fault address if a fault is retried by the
kernel (e.g., handle_userfault() got interrupted before page resolution).
As this info might be important, instead of using commit message, I put
more details into the code as comment, when introducing an array
maintaining concurrent faults on one vCPU. Please refer to the comments
for details on both cases, especially case 1 which can be tricky.
Case 1 sounds rare, but it can be easily reproduced locally for me when we
run blocktime together with the migration-test on the vanilla postcopy.
New Design
==========
This patch should do almost what Alexey mentioned, but slightly
differently: instead of having an array to maintain vCPU fault addresses,
for each of the fault message we push a message into a hash, indexed by the
fault address.
With the hash, it can replace the old two structs: both the vcpu_addr[]
array, and also the array to store the start time of the fault. However
due to above we need one more counter array to account concurrent faults on
the same vCPU - that should even be needed in the old code, it's just that
the old code was buggy and it will blindly overwrite an existing
entry.. now we'll start to really track everything.
The hash structure might be more efficient than tree to maintain such
addr->(cpu, fault_time) information, so that the insert() and lookup()
paths should ideally both be ~O(1). After all, we do not need to sort.
Here we need to do one remove() though after the lookup(). It could be
slow but only if many vCPUs faulted exactly on the same address (so when
the list of cpu entries is long), which should be unlikely. Even with that,
it's still a worst case O(N) (consider 400 vCPUs faulted on the same
address and how likely is it..) rather than a constant O(N) complexity.
When at it, touch up the tracepoints to make them slightly more useful.
One tracepoint is added when walking all the fault entries.
Peter Xu [Fri, 13 Jun 2025 14:12:14 +0000 (10:12 -0400)]
migration/postcopy: Cleanup the total blocktime accounting
The variable vcpu_total_blocktime isn't easy to follow. In reality, it
wants to capture the case where all vCPUs are stopped, and now there will
be some vCPUs starts running.
The name now starts to conflict with vcpu_blocktime_total[], meanwhile it's
actually not necessary to have the variable at all: since nobody is
touching smp_cpus_down except ourselves, we can safely do the calculation
at the end before decrementing smp_cpus_down.
Hopefully this makes the logic easier to read, side benefit is we drop one
temp var.
Peter Xu [Fri, 13 Jun 2025 14:12:13 +0000 (10:12 -0400)]
migration/postcopy: Cache the tid->vcpu mapping for blocktime
Looking up the vCPU index for each fault can be expensive when there're
hundreds of vCPUs. Provide a cache for tid->vcpu instead with a hash
table, then lookup from there.
When at it, add another counter to record how many non-vCPU faults it gets.
For example, the main thread can also access a guest page that was missing.
These kind of faults are not accounted by blocktime so far.
Peter Xu [Fri, 13 Jun 2025 14:12:12 +0000 (10:12 -0400)]
migration/postcopy: Initialize blocktime context only until listen
Before this patch, the blocktime context can be created very early, because
postcopy_ram_supported_by_host() <- migrate_caps_check() can happen during
migration object init.
The trick here is the blocktime context needs system vCPU information,
which seems to be possible to change after that point. I didn't verify it,
but it doesn't sound right.
Now move it out and initialize the context only when postcopy listen
starts. That is already during a migration so it should be guaranteed the
vCPU topology can never change on both sides.
While at it, assert that the ctx isn't created instead this time; the old
"if" trick isn't needed when we're sure it will only happen once now.
Peter Xu [Fri, 13 Jun 2025 14:12:11 +0000 (10:12 -0400)]
migration/postcopy: Report fault latencies in blocktime
Blocktime so far only cares about the time one vcpu (or the whole system)
got blocked. It would be also be helpful if it can also report the latency
of page requests, which could be very sensitive during postcopy.
Blocktime itself is sometimes not very important, especially when one
thinks about KVM async PF support, which means vCPUs are literally almost
not blocked at all because the guest OS is smart enough to switch to
another task when a remote fault is needed.
However, latency is still sensitive and important because even if the guest
vCPU is running on threads that do not need a remote fault, the workload
that accesses some missing page is still affected.
Add two entries to the report, showing how long it takes to resolve a
remote fault. Mention in the QAPI doc that this is not the real average
fault latency, but only the ones that was requested for a remote fault.
Unwrap get_vcpu_blocktime_list() so we don't need to walk the list twice,
meanwhile add the entry checks in qtests for all postcopy tests.
Cc: Markus Armbruster <armbru@redhat.com> Cc: Dr. David Alan Gilbert <dave@treblig.org> Reviewed-by: Fabiano Rosas <farosas@suse.de> Tested-by: Mario Casquero <mcasquer@redhat.com> Link: https://lore.kernel.org/r/20250613141217.474825-9-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de>
Peter Xu [Fri, 13 Jun 2025 14:12:09 +0000 (10:12 -0400)]
migration/postcopy: Bring blocktime layer to ns level
With 64-bit fields, it is trivial. The caution is when exposing any values
in QMP, it was still declared with milliseconds (ms). Hence it's needed to
do the convertion when exporting the values to existing QMP queries.
Peter Xu [Fri, 13 Jun 2025 14:12:07 +0000 (10:12 -0400)]
migration/postcopy: Make all blocktime vars 64bits
I am guessing it was used to be 32bits because of the atomic ops. Now all
the atomic ops are gone and we're protected by a mutex instead, it's ok we
can switch to 64 bits.
Reasons to move over:
- Allow further patches to change the unit from ms to us: with postcopy
preempt mode, we're really into hundreds of microseconds level on
blocktime. We'd better be able to trap those.
- This also paves way for some other tricks that the original version
used to avoid overflows, e.g., start_time was almost only useful before
to make sure the sampled timestamp won't overflow a 32-bit field.
- This prepares further reports on top of existing data collected,
e.g. average page fault latencies. When average operation is taken into
account, milliseconds are simply too coarse grained.
When at it:
- Rename page_fault_vcpu_time to vcpu_blocktime_start.
- Rename vcpu_blocktime to vcpu_blocktime_total.
- Touch up the trace-events to not dump blocktime ctx pointer
Peter Xu [Fri, 13 Jun 2025 14:12:05 +0000 (10:12 -0400)]
migration/postcopy: Push blocktime start/end into page req mutex
The postcopy blocktime feature was tricky that it used quite some atomic
operations over quite a few arrays and vars, without explaining how that
would be thread safe. The thread safety here is about concurrency between
the fault thread and the fault resolution threads, possible to access the
same chunk of data. All these atomic ops can be expensive too before
knowing clearly how it works.
OTOH, postcopy has one page_request_mutex used to serialize the received
bitmap updates. So far it's ok - we don't yet have a lot of threads
contending the lock. It might change after multifd will be supported, but
that's a separate story. What is important is, with that mutex, it's
pretty lightweight to move all the blocktime maintenance into the mutex
critical section. It's because the blocktime layer is lightweighted:
almost "remember which vcpu faulted on which address", and "ok we get some
fault resolved, calculate how long it takes". It's also an optional
feature for now (but I have thought of changing that, maybe in the future).
Let's push the blocktime layer into the mutex, so that it's always
thread-safe even without any atomic ops.
To achieve that, I'll need to add a tid parameter on fault path so that
it'll start to pass the faulted thread ID into deeper the stack, but not
too deep. When at it, add a comment for the shared fault handler (for
example, vhost-user devices running with postcopy), to mention a TODO. One
reason it might not be trivial is that vhost-user's userfaultfds should be
opened by vhost-user process, so it's pretty hard to control making sure
the TID feature will be around. It wasn't supported before, so keep it
like that for now.
Now we should be as ease when everything is protected by a mutex that we
always take anyway.
One side effect: we can finally remove one ramblock_recv_bitmap_test() in
mark_postcopy_blocktime_begin(), which was pretty weird and which also
includes a weird (but maybe necessary.. but maybe not?) operation to inject
a blocktime entry then quickly erase it.. When we're with the mutex, and
when we make sure it's invoked after checking the receive bitmap, it's not
needed anymore. Instead, we assert.
As another side effect, this paves way for removing all atomic ops in all
the mem accesses in blocktime layer.
Note that we need a stub for mark_postcopy_blocktime_begin() for Windows
builds.
Peter Xu [Fri, 13 Jun 2025 14:08:00 +0000 (10:08 -0400)]
migration: Rewrite the migration complete detect logic
There're a few things off here in that logic, rewrite it. When at it, add
rich comment to explain each of the decisions.
Since this is very sensitive path for migration, below are the list of
things changed with their reasonings.
(1) Exact pending size is only needed for precopy not postcopy
Fundamentally it's because "exact" version only does one more deep
sync to fetch the pending results, while in postcopy's case it's
never going to sync anything more than estimate as the VM on source
is stopped.
(2) Do _not_ rely on threshold_size anymore to decide whether postcopy
should complete
threshold_size was calculated from the expected downtime and
bandwidth only during precopy as an efficient way to decide when to
switchover. It's not sensible to rely on threshold_size in postcopy.
For precopy, if switchover is decided, the migration will complete
soon. It's not true for postcopy. Logically speaking, postcopy
should only complete the migration if all pending data is flushed.
Here it used to work because save_complete() used to implicitly
contain save_live_iterate() when there's pending size.
Even if that looks benign, having RAMs to be migrated in postcopy's
save_complete() has other bad side effects:
(a) Since save_complete() needs to be run once at a time, it means
when moving RAM there's no way moving other things (rather than
round-robin iterating the vmstate handlers like what we do with
ITERABLE phase). Not an immediate concern, but it may stop working
in the future when there're more than one iterables (e.g. vfio
postcopy).
(b) postcopy recovery, unfortunately, only works during ITERABLE
phase. IOW, if src QEMU moves RAM during postcopy's save_complete()
and network failed, then it'll crash both QEMUs... OTOH if it failed
during iteration it'll still be recoverable. IOW, this change should
further reduce the window QEMU split brain and crash in extreme cases.
If we enable the ram_save_complete() tracepoints, we'll see this
before this patch:
This shouldn't be super important, the movement makes sure there's
only one in_postcopy check, then we are clear on what we do with the
two completely differnt use cases (precopy v.s. postcopy).
(4) Trivial touch up on threshold_size comparision
Peter Xu [Fri, 13 Jun 2025 14:07:56 +0000 (10:07 -0400)]
migration: Rename save_live_complete_precopy to save_complete
Now after merging the precopy and postcopy version of complete() hook,
rename the precopy version from save_live_complete_precopy() to
save_complete().
Dropping the "live" when at it, because it's in most cases not live when
happening (in precopy).
No functional change intended.
Reviewed-by: Juraj Marcin <jmarcin@redhat.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20250613140801.474264-7-peterx@redhat.com
[peterx: squash the fixup that covers a few more doc spots, per Juraj] Signed-off-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de>
Peter Xu [Fri, 13 Jun 2025 14:07:55 +0000 (10:07 -0400)]
migration: Drop save_live_complete_postcopy hook
The hook is only defined in two vmstate users ("ram" and "block dirty
bitmap"), meanwhile both of them define the hook exactly the same as the
precopy version. Hence, this postcopy version isn't needed.
Peter Xu [Fri, 13 Jun 2025 14:07:53 +0000 (10:07 -0400)]
migration/docs: Move docs for postcopy blocktime feature
Move it out of vanilla postcopy session, but instead a standalone feature.
When at it, removing the NOTE because it's incorrect now after introduction
of max-postcopy-bandwidth, which can control the throughput even for
postcopy phase.
This is only found when I started to look into making the blocktime feature
more useful (so as to avoid using bpftrace, even though I'm not sure which
one will be harder to use..).
So the old dump would look like this:
Postcopy vCPU Blocktime: 0-1,4,10,21,33,46,48,59
Even though there're actually 40 vcpus, and the string will merge same
elements and also sort them.
To fix it, simply loop over the uint32List manually. Now it looks like:
Cc: Dr. David Alan Gilbert <dave@treblig.org> Cc: Alexey Perevalov <a.perevalov@samsung.com> Cc: Markus Armbruster <armbru@redhat.com> Tested-by: Mario Casquero <mcasquer@redhat.com> Reviewed-by: Juraj Marcin <jmarcin@redhat.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20250613140801.474264-3-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de>
I followed Dave's suggestion, and some more modifications on top:
- Added all elements into the picture
- Use size_to_str() and drop most of the units: benefit is more friendly
to most human eyes, bad side effect is lose of details, but that should
be corner case per my uses, and one can still leverage the QMP interface
when necessary.
- Sub-grouping for "Transfers" ("Channels" and "Page Types").
Suggested-by: Dr. David Alan Gilbert <dave@treblig.org> Tested-by: Li Zhijian <lizhijian@fujitsu.com> Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> Acked-by: Dr. David Alan Gilbert <dave@treblig.org> Reviewed-by: Juraj Marcin <jmarcin@redhat.com> Tested-by: Mario Casquero <mcasquer@redhat.com> Link: https://lore.kernel.org/r/20250613140801.474264-2-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de>
tests/functional: Add a test for the MAX78000 arm machine
Runs a binary from the max78000test repo used in
developing the qemu implementation of the max78000
to verify that the machine and implemented devices
generally still work.
Signed-off-by: Jackson Donaldson <jcksn@duck.com> Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20250711110626.624534-3-jcksn@duck.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jackson Donaldson <jcksn@duck.com>
Message-id: 20250711110626.624534-2-jcksn@duck.com
[PMM: Moved doc to correct place in index; made underlines correct
length; added missing trailing newline; added SPDX] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
target/s390x: Have s390_cpu_halt() not return anything
Since halting a vCPU and how many left running do not need
to be tied together, split the s390_count_running_cpus()
call out of s390_cpu_halt() to the single caller using it:
s390_handle_wait().
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250708095746.12697-4-philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
In order to simplify the next commit where s390_count_running_cpus()
is split out of s390_cpu_halt(), make its prototype public as a
preliminary step.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250708095746.12697-3-philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
target/s390x: Remove unused s390_cpu_[un]halt() user stubs
Since commit da944885469 ("target/s390x: make helper.c
sysemu-only") target/s390x/helper.c is only built for
system mode, so s390_cpu_halt() and s390_cpu_unhalt()
are never called from user mode.
Fixes: da944885469 ("target/s390x: make helper.c sysemu-only") Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250708095746.12697-2-philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
Thomas Huth [Mon, 7 Jul 2025 18:47:36 +0000 (20:47 +0200)]
tests/functional/test_ppc_bamboo: Replace broken link with working assets
The old image that we used for testing the bamboo machine has disappeared
from the internet. Fortunately there is another kernel + initrd provided
by Cédric that can be used for testing this machine, too.
Reported-by: Stefan Hajnoczi <stefanha@gmail.com> Suggested-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Cédric Le Goater <clg@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20250707184736.88660-1-thuth@redhat.com>
Thomas Huth [Tue, 1 Jul 2025 10:48:27 +0000 (12:48 +0200)]
tests/functional: Add dependency to the keymap_targets
When doing a "configure" in a an empty build directory, followed by
a "make check" without a normal build in between, the vnc functional
test currently fails since the keymaps have not been built yet.
Thus add a dependency to the keymap_targets here to make sure that
the keymaps are built before running the functional tests.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20250701104827.363904-1-thuth@redhat.com>
Thomas Huth [Wed, 9 Jul 2025 08:34:43 +0000 (10:34 +0200)]
tests/functional: Add a test for s390x pxelinux.cfg network booting
Check the various ways of booting a kernel via pxelinux.cfg file,
e.g. by specifying the config file name via the MAC address or the
UUID of the guest. Also check whether we can successfully load an
alternate kernel via the "loadparm" parameter here and whether the
boot menu shows up with "-boot menu=on".
Reviewed-by: Jared Rossi <jrossi@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20250709083443.41574-6-thuth@redhat.com>
Thomas Huth [Wed, 9 Jul 2025 08:34:41 +0000 (10:34 +0200)]
pc-bios/s390-ccw: Make get_boot_index() from menu.c global
We are going to reuse this function for selecting an entry from
the pxelinux.cfg menu, so rename this function with a "menu_"
prefix and make it available globally.
Reviewed-by: Jared Rossi <jrossi@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20250709083443.41574-4-thuth@redhat.com>
Thomas Huth [Wed, 9 Jul 2025 08:34:40 +0000 (10:34 +0200)]
pc-bios/s390-ccw: Allow up to 31 entries for pxelinux.cfg
We're going to support a menu for the pxelinux.cfg code, and to be able
to reuse some functionality from menu.c, we should align the maximum
amount of possible entries with the MAX_BOOT_ENTRIES constant that is
used there. Thus replace MAX_PXELINUX_ENTRIES with MAX_BOOT_ENTRIES.
Reviewed-by: Jared Rossi <jrossi@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20250709083443.41574-3-thuth@redhat.com>
Thomas Huth [Wed, 9 Jul 2025 08:34:39 +0000 (10:34 +0200)]
pc-bios/s390-ccw: Allow to select a different pxelinux.cfg entry via loadparm
Since we're linking the network booting code into the main firmware
binary nowadays, we can support the "loadparm" parameter now quite
easily for pxelinux.cfg config files that contain multiple entries.
Reviewed-by: Jared Rossi <jrossi@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20250709083443.41574-2-thuth@redhat.com>
Peter Maydell [Thu, 10 Jul 2025 16:15:52 +0000 (17:15 +0100)]
hw/s390x/s390-pci-bus.c: Use g_assert_not_reached() in functions taking an ett
The s390-pci-bus.c code, Coverity complains about a possible overflow
because get_table_index() can return -1 if the ett value passed in is
not one of the three permitted ZPCI_ETT_PT, ZPCI_ETT_ST, ZPCI_ETT_RT,
but the caller in table_translate() doesn't check this and instead
uses the return value directly in a calculation of the guest address
to read from.
In fact this case cannot happen, because:
* get_table_index() is called only from table_translate()
* the only caller of table_translate() loops through the ett values
in the order RT, ST, PT until table_translate() returns 0
* table_translate() will return 0 for the error cases and when
translate_iscomplete() returns true
* translate_iscomplete() is always true for ZPCI_ETT_PT
So table_translate() is always called with a valid ett value.
Instead of having the various functions called from table_translate()
return a default or dummy value when the ett argument is out of range,
use g_assert_not_reached() to indicate that this is impossible.
Coverity: CID 1547609 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
Message-ID: <20250710161552.1287399-1-peter.maydell@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
target/s390x/tcg: Use vaddr in s390_probe_access()
Commit 70ebd9ce1cb ("s390x/tcg: Fault-safe memset") passed
vaddr type to access_prepare(), and commit b6c636f2cd6
("s390x/tcg: Fault-safe memmove") to do_access_get_byte(),
but declared S390Access::vaddr[1,2] as target_ulong.
Directly declare these as vaddr type, and have
s390_probe_access() use that type as argument.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20250707171059.3064-3-philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
target/s390x/kvm: Use vaddr in find/insert_hw_breakpoint()
Since commit b8a6eb1862a both kvm_arch_insert_hw_breakpoint()
and kvm_arch_remove_hw_breakpoint() use a vaddr type. Use the
same type for the callees.
Fixes: b8a6eb1862a ("sysemu/kvm: Use vaddr for kvm_arch_[insert|remove]_hw_breakpoint") Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20250707171059.3064-2-philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
Page size of TLB entry comes from CSR STLBPS and pwcl register. With
huge page, it is dir_base + dir_width from pwcl register. With normal
page, it is field of PTBASE from pwcl register.
So it is ok to check validity in function helper_ldpte() and function
helper_csrwr_stlbps(). And it is unnecessary in tlb entry fill path.
Signed-off-by: Bibo Mao <maobibo@loongson.cn> Reviewed-by: Song Gao <gaosong@loongson.cn>
Function helper_csrwr_stlbps() is emulation with CSR STLBPS register
write operation. However there is only parameter checking action, and
no register updating action. Here update value of CSR_STLBPS when
parameter passes to check.
Signed-off-by: Bibo Mao <maobibo@loongson.cn> Reviewed-by: Song Gao <gaosong@loongson.cn>
Geoffrey Thomas [Fri, 14 Mar 2025 12:47:42 +0000 (08:47 -0400)]
linux-user: Hold the fd-trans lock across fork
If another thread is holding target_fd_trans_lock during a fork,
then the lock becomes permanently locked in the child and the
emulator deadlocks at the next interaction with the fd-trans table.
As with other locks, acquire the lock in fork_start() and release
it in fork_end().
Cc: qemu-stable@nongnu.org Signed-off-by: Geoffrey Thomas <geofft@ldpreload.com> Fixes: c093364f4d91 "fd-trans: Fix race condition on reallocation of the translation table."
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2846 Buglink: https://github.com/astral-sh/uv/issues/6105 Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20250314124742.4965-1-geofft@ldpreload.com>
linux-user/mips/o32: Drop sa_restorer functionality
The Linux kernel dropped support for sa_restorer on O32 MIPS in the
release 2.5.48 because it was unused. See the comment in
arch/mips/include/uapi/asm/signal.h.
Applications using the kernels UAPI headers will not reserve enough
space for qemu-user to copy the sigaction.sa_restorer field to.
Unrelated data may be overwritten.
Align qemu-user with the kernel by also dropping sa_restorer support.
Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20250709-mips-sa-restorer-v1-1-fc17120e4afe@t-8ch.de>
Peter Maydell [Thu, 10 Jul 2025 17:07:07 +0000 (18:07 +0100)]
linux-user/gen-vdso: Don't read off the end of buf[]
In gen-vdso we load in a file and assume it's a valid ELF file. In
particular we assume it's big enough to be able to read the ELF
information in e_ident in the ELF header.
Add a check that the total file length is at least big enough for all
the e_ident bytes, which is good enough for the code in gen-vdso.c.
This will catch the most obvious possible bad input file (truncated)
and allow us to run the sanity checks like "not actually an ELF file"
without potentially crashing.
The code in elf32_process() and elf64_process() still makes
assumptions about the file being well-formed, but this is OK because
we only run it on the vdso binaries that we create ourselves in the
build process by running the compiler.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20250710170707.1299926-3-peter.maydell@linaro.org>
Peter Maydell [Thu, 10 Jul 2025 16:43:54 +0000 (17:43 +0100)]
linux-user: Check for EFAULT failure in nanosleep
target_to_host_timespec() returns an error if the memory the guest
passed us isn't actually readable. We check for this everywhere
except the callsite in the TARGET_NR_nanosleep case, so this mistake
was caught by a Coverity heuristic.
Add the missing error checks to the calls that convert between the
host and target timespec structs.
Coverity: CID 1507104 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20250710164355.1296648-1-peter.maydell@linaro.org>
Peter Maydell [Thu, 10 Jul 2025 11:31:23 +0000 (12:31 +0100)]
linux-user: Implement fchmodat2 syscall
The fchmodat2 syscall is new from Linux 6.6; it is like the
existing fchmodat syscall except that it takes a flags parameter.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3019 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20250710113123.1109461-1-peter.maydell@linaro.org>
fpu: Process float_muladd_negate_result after rounding
Changing the sign before rounding affects the correctness of
the asymmetric rouding modes: float_round_up and float_round_down.
Reported-by: WANG Rui <wangrui@loongson.cn> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
hw/arm/virt-acpi-build: Don't create ITS id mappings by default
Commit d6afe18b7242 ("hw/arm/virt-acpi-build: Fix ACPI IORT and MADT tables
when its=off") moved ITS group node generation under the its=on condition.
However, it still creates rc_its_idmaps unconditionally, which results in
duplicate ID mappings in the IORT table.
Fixes:d6afe18b7242 ("hw/arm/virt-acpi-build: Fix ACPI IORT and MADT tables when its=off") Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Donald Dutile <ddutile@redhat.com> Tested-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
system/qdev: Remove pointless NULL check in qdev_device_add_from_qdict
Coverity reported a unnecessary NULL check:
qemu/system/qdev-monitor.c: 720 in qdev_device_add_from_qdict()
683 /* create device */
684 dev = qdev_new(driver);
...
719 err_del_dev:
>>> CID 1590192: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "dev" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
720 if (dev) {
721 object_unparent(OBJECT(dev));
722 object_unref(OBJECT(dev));
723 }
724 return NULL;
725 }
Indeed, unlike qdev_try_new() which can return NULL,
qdev_new() always returns a heap pointer (or aborts).
Remove the unnecessary assignment and check.
Fixes: f3a85056569 ("qdev/qbus: add hidden device support")
Resolves: Coverity CID 1590192 (Null pointer dereferences) Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Thu, 10 Jul 2025 10:39:33 +0000 (11:39 +0100)]
hw/arm/virt: Allow virt extensions with KVM
Up to now virt support on guest has been only supported with TCG.
Now it becomes feasible to use it with KVM acceleration.
Check neither in-kernel GICv3 nor aarch64=off is used along with KVM
EL2.
Signed-off-by: Haibo Xu <haibo.xu@linaro.org> Signed-off-by: Miguel Luis <miguel.luis@oracle.com> Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20250707164129.1167837-6-eric.auger@redhat.com
[PMM: make "kernel doesn't have EL2 support" error message
distinct from the old "QEMU doesn't have KVM EL2 support" one] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Eric Auger [Mon, 7 Jul 2025 16:40:30 +0000 (18:40 +0200)]
hw/arm/arm_gicv3_kvm: Add a migration blocker with kvm nested virt
We may be miss some NV related GIC register save/restore. Until
we complete the study, let's add a migration blocker when the
maintenance IRQ is set.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Message-id: 20250707164129.1167837-5-eric.auger@redhat.com Suggested-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
target/arm: Enable feature ARM_FEATURE_EL2 if EL2 is supported
KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
In case the host does support NV, expose the feature.
Signed-off-by: Haibo Xu <haibo.xu@linaro.org> Signed-off-by: Miguel Luis <miguel.luis@oracle.com> Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20250707164129.1167837-4-eric.auger@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
projects / thirdparty / qemu.git / log
