Akihiko Odaki [Mon, 26 May 2025 05:29:13 +0000 (14:29 +0900)]
qemu-thread: Avoid futex abstraction for non-Linux
qemu-thread used to abstract pthread primitives into futex for the
QemuEvent implementation of POSIX systems other than Linux. However,
this abstraction has one key difference: unlike futex, pthread
primitives require an explicit destruction, and it must be ordered after
wait and wake operations.
It would be easier to perform destruction if a wait operation ensures
the corresponding wake operation finishes as POSIX semaphore does, but
that requires to protect state accesses in qemu_event_set() and
qemu_event_wait() with a mutex. On the other hand, real futex does not
need such a protection but needs complex barrier and atomic operations
to ensure ordering between the two functions.
Add special implementations of qemu_event_set() and qemu_event_wait()
using pthread primitives. qemu_event_wait() will ensure qemu_event_set()
finishes, and these functions will avoid complex barrier and atomic
operations to ensure ordering between them.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Tested-by: Phil Dennis-Jordan <phil@philjordan.eu> Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu> Link: https://lore.kernel.org/r/20250526-event-v4-5-5b784cc8e1de@daynix.com Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Akihiko Odaki [Thu, 29 May 2025 05:45:50 +0000 (14:45 +0900)]
futex: Check value after qemu_futex_wait()
futex(2) - Linux manual page
https://man7.org/linux/man-pages/man2/futex.2.html
> Note that a wake-up can also be caused by common futex usage patterns
> in unrelated code that happened to have previously used the futex
> word's memory location (e.g., typical futex-based implementations of
> Pthreads mutexes can cause this under some conditions). Therefore,
> callers should always conservatively assume that a return value of 0
> can mean a spurious wake-up, and use the futex word's value (i.e.,
> the user-space synchronization scheme) to decide whether to continue
> to block or not.
Tom Lendacky [Fri, 28 Mar 2025 20:30:24 +0000 (15:30 -0500)]
i386/kvm: Prefault memory on page state change
A page state change is typically followed by an access of the page(s) and
results in another VMEXIT in order to map the page into the nested page
table. Depending on the size of page state change request, this can
generate a number of additional VMEXITs. For example, under SNP, when
Linux is utilizing lazy memory acceptance, memory is typically accepted in
4M chunks. A page state change request is submitted to mark the pages as
private, followed by validation of the memory. Since the guest_memfd
currently only supports 4K pages, each page validation will result in
VMEXIT to map the page, resulting in 1024 additional exits.
When performing a page state change, invoke KVM_PRE_FAULT_MEMORY for the
size of the page state change in order to pre-map the pages and avoid the
additional VMEXITs. This helps speed up boot times.
Paolo Bonzini [Thu, 5 Jun 2025 09:12:15 +0000 (11:12 +0200)]
rust: make TryFrom macro more resilient
If the enum includes values such as "Ok", "Err", or "Error", the TryInto
macro can cause errors. Be careful and qualify identifiers with the full
path, or in the case of TryFrom<>::Error do not use the associated type
at all.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Zhao Liu [Mon, 26 May 2025 11:54:21 +0000 (13:54 +0200)]
rust/hpet: Drop BqlCell wrapper for num_timers
Now that the num_timers field is initialized as a property, someone may
change its default value using qdev_prop_set_uint8(), but the value is
fixed after the Rust code sees it first. Since there is no need to modify
it after realize(), it is not to be necessary to have a BqlCell wrapper.
Paolo Bonzini [Mon, 26 May 2025 11:52:11 +0000 (13:52 +0200)]
rust/hpet: change type of num_timers to usize
Remove the need to convert after every read of the BqlCell. Because the
vmstate uses a u8 as the size of the VARRAY, this requires switching
the VARRAY to use num_timers_save; which in turn requires ensuring that
the num_timers_save is always there. For simplicity do this by
removing support for version 1, which QEMU has not been producing for
~15 years.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Tue, 3 Jun 2025 15:45:39 +0000 (17:45 +0200)]
rust: qemu-api: add bindings to Error
Provide an implementation of std::error::Error that bridges the Rust
anyhow::Error and std::panic::Location types with QEMU's Error*.
It also has several utility methods, analogous to error_propagate(),
that convert a Result into a return value + Error** pair. One important
difference is that these propagation methods *panic* if *errp is NULL,
unlike error_propagate() which eats subsequent errors[1]. The reason
for this is that in C you have an error_set*() call at the site where
the error is created, and calls to error_propagate() are relatively rare.
In Rust instead, even though these functions do "propagate" a
qemu_api::Error into a C Error**, there is no error_setg() anywhere that
could check for non-NULL errp and call abort(). error_propagate()'s
behavior of ignoring subsequent errors is generally considered weird,
and there would be a bigger risk of triggering it from Rust code.
[1] This is actually a violation of the preconditions of error_propagate(),
so it should not happen. But you never know...
Reviewed-by: Zhao Liu <zhao1.liu@intel.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Mon, 26 May 2025 07:25:50 +0000 (09:25 +0200)]
util/error: allow non-NUL-terminated err->src
Rust makes the current file available as a statically-allocated string,
but without a NUL terminator. Allow this by storing an optional maximum
length in the Error.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Fri, 23 May 2025 15:59:52 +0000 (17:59 +0200)]
subprojects: add the foreign crate
This is a cleaned up and separated version of the patches at
https://lore.kernel.org/all/20240701145853.1394967-4-pbonzini@redhat.com/
https://lore.kernel.org/all/20240701145853.1394967-5-pbonzini@redhat.com/
Its first user will be the Error bindings; for example a QEMU Error ** can be
converted to a Rust Option using
unsafe { Option::<Error>::from_foreign(c_error) }
Reviewed-by: Zhao Liu <zhao1.liu@intel.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Mon, 26 May 2025 10:10:18 +0000 (12:10 +0200)]
subprojects: add the anyhow crate
This is a standard replacement for Box<dyn Error> which is more efficient (it only
occcupies one word) and provides a backtrace of the error. This could be plumbed
into &error_abort in the future.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Since the previous commit, python/setup.cfg applies to scripts/qapi/ as
well. Configuration files in scripts/qapi/ override python/setup.cfg.
scripts/qapi/.flake8 and scripts/qapi/.isort.cfg actually match
python/setup.cfg exactly, and can go.
The differences between scripts/qapi/mypy.ini and python/setup.cfg are
harmless: namespace_packages being set to True is a requirement for the
PEP420 nested package structure of QEMU but not for scripts/qapi, but
has no effect on type checking the QAPI code. warn_unused_ignores is
used in python/ to be able to target a wide variety of mypy versions;
some of which that have added new ignore categories that are not present
in older versions.
Ultimately, scripts/qapi/mypy.ini can be removed without any real change
in behavior to how mypy enforces type safety there.
The pylint config is being left in place because the settings differ
enough from the python/ directory settings that we need a chit-chat on
how to merge them O:-)
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20250604200354.459501-7-jsnow@redhat.com
Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20250604200354.459501-6-jsnow@redhat.com
John Snow [Wed, 4 Jun 2025 20:03:52 +0000 (16:03 -0400)]
python: add qapi static analysis tests
Update the python tests to also check QAPI and the QAPI Sphinx
extensions. The docs/sphinx/qapidoc_legacy.py file is not included in
these checks, as it is destined for removal soon. mypy is also not
called on the QAPI Sphinx extensions, owing to difficulties supporting
Sphinx 3.x - 8.x while maintaining static type checking support. mypy
*is* called on all of the QAPI tools themselves, though.
flake8, isort and mypy use the tool configuration from the existing
python directory (in setup.cfg). pylint continues to use the special
configuration located in scripts/qapi/ - that configuration is more
permissive. If we wish to unify the two configurations, that's a
separate series and a discussion for a later date.
The list of pylint ignores is also updated, owing again to the wide
window of pylint version support: newer versions require pragmas to
occasionally silence the "too many positional arguments" warning, but
older versions do not have such a warning category and will instead yelp
about an unrecognized option. Silence that warning, too.
As a result of this patch, one would be able to run any of the following
tests locally from the qemu.git/python directory and have it cover the
QAPI tooling as well. All of the following options run the python tests,
static analysis tests, and linter checks; but with different
combinations of dependencies and interpreters.
- "make check-minreqs" Run tests specifically under our oldest supported
Python and our oldest supported dependencies. This is the test that
runs on GitLab as "check-python-minreqs". This helps ensure we do not
regress support on older platforms accidentally.
- "make check-tox" Runs the tests under the newest supported
dependencies, but under each supported version of Python in turn. At
time of writing, this is Python 3.8 to 3.13 inclusive. This test helps
catch bleeding-edge problems before they become problems for developer
workstations. This is the GitLab test "check-python-tox" and is an
optionally run, may-fail test due to the unpredictable nature of new
dependencies being released into the ecosystem that may cause
regressions.
- "make check-dev" Runs the tests under the newest supported
dependencies using whatever version of Python the user happens to have
installed. This is a quick convenience check that does not map to any
particular GitLab test.
(Note! check-dev may be busted on Fedora 41 and bleeding edge versions
of setuptools. That's unrelated to this patch and I'll address it
separately and soon. Thank you for your patience, --mgmt)
Finally, finally, finally: this means that QAPI tooling will be linted
and type-checked from the GitLab pipelines.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20250604200354.459501-5-jsnow@redhat.com
[Edited license choice per review --js] Signed-off-by: John Snow <jsnow@redhat.com>
John Snow [Wed, 4 Jun 2025 20:03:51 +0000 (16:03 -0400)]
python: update missing dependencies from minreqs
We pin all dependencies for the "check-minreqs" test because pip lacks a
dependency resolver that installs "the oldest possible package that
meets dependency criteria". So, in order to test our stated minimum
requirements, we pin all of our dependencies (and their dependencies,
transitively) at the oldest possible versions that still work and pass
tests; proving that our minimum requirements are correct.
(It also ensures no new features accidentally sneak in from developers
on newer platforms.)
A few transitive dependencies were omitted from the pinned dependency
file by accident; as a result, pip's dependency solver can pull in newer
dependencies, which we don't want. This patch corrects the previous
oversight and pins the missing dependencies.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20250604200354.459501-4-jsnow@redhat.com
John Snow [Wed, 4 Jun 2025 20:03:50 +0000 (16:03 -0400)]
docs/qapidoc: linting fixes
This restores the linting baseline in qapidoc. The order of some imports
change slightly here due to configuring isort a little better:
previously, isort was having difficulty understanding that "compat" and
"qapidoc_legacy" were local modules because docs/sphinx "isn't a python
package". Configuring this manually, isort chooses a different import
ordering, which _is_ intentional here.
Also: extra ignores are added for pylint. The most recent versions of
pylint don't require these ignores, but the oldest versions we support
do, so in the extra ignores go.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20250604200354.459501-3-jsnow@redhat.com
Stefan Hajnoczi [Thu, 5 Jun 2025 15:00:29 +0000 (11:00 -0400)]
Merge tag 'pull-vfio-20250605' of https://github.com/legoater/qemu into staging
vfio queue:
* Fixed OpRegion detection in IGD
* Added prerequisite rework for IOMMU nesting support
* Added prerequisite rework for vfio-user
* Added prerequisite rework for VFIO live update
* Modified memory_get_xlat_addr() to return a MemoryRegion
* tag 'pull-vfio-20250605' of https://github.com/legoater/qemu:
vfio: move vfio-cpr.h
vfio: vfio_find_ram_discard_listener
MAINTAINERS: Add reviewer for CPR
vfio/iommufd: Save vendor specific device info
vfio/iommufd: Implement [at|de]tach_hwpt handlers
vfio/iommufd: Add properties and handlers to TYPE_HOST_IOMMU_DEVICE_IOMMUFD
backends/iommufd: Add a helper to invalidate user-managed HWPT
vfio/container: pass MemoryRegion to DMA operations
vfio: return mr from vfio_get_xlat_addr
vfio/igd: Fix incorrect error propagation in vfio_pci_igd_opregion_detect()
vfio/iommufd: Add comment emphasizing no movement of hiod->realize() call
vfio: refactor out IRQ signalling setup
vfio: move config space read into vfio_pci_config_setup()
vfio: move more cleanup into vfio_pci_put_device()
vfio: add more VFIOIOMMUClass docs
vfio/igd: OpRegion not found fix error typo
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
* tag 'for-upstream' of https://repo.or.cz/qemu/kevin: (24 commits)
hw/core/qdev-properties-system: Add missing return in set_drive_helper()
iotests: fix 240
block/io: remove duplicate GLOBAL_STATE_CODE() in bdrv_do_drained_end()
iotests/graph-changes-while-io: add test case with removal of lower snapshot
iotests/graph-changes-while-io: remove image file after test
block: mark bdrv_drained_begin() and friends as GRAPH_UNLOCKED
blockdev: drain while unlocked in external_snapshot_action()
blockdev: drain while unlocked in internal_snapshot_action()
block: move drain outside of quorum_del_child()
block: move drain outside of bdrv_root_unref_child()
block: move drain outside of quorum_add_child()
block: move drain outside of bdrv_attach_child()
block: move drain outside of bdrv_root_attach_child()
block: move drain outside of bdrv_set_backing_hd_drained()
block: move drain outside of bdrv_attach_child_common(_abort)()
block: move drain outside of bdrv_try_change_aio_context()
block: move drain outside of bdrv_change_aio_context() and mark GRAPH_RDLOCK
block: mark bdrv_child_change_aio_context() GRAPH_RDLOCK
block: mark change_aio_ctx() callback and instances as GRAPH_RDLOCK(_PTR)
block: mark bdrv_parent_change_aio_context() GRAPH_RDLOCK
...
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Zhenzhong Duan [Wed, 4 Jun 2025 06:21:15 +0000 (14:21 +0800)]
vfio/iommufd: Save vendor specific device info
Some device information returned by ioctl(IOMMU_GET_HW_INFO) are vendor
specific. Save them as raw data in a union supporting different vendors,
then vendor IOMMU can query the raw data with its fixed format for
capability directly.
Because IOMMU_GET_HW_INFO is only supported in linux, so declare those
capability related structures with CONFIG_LINUX.
Suggested-by: Eric Auger <eric.auger@redhat.com> Suggested-by: Nicolin Chen <nicolinc@nvidia.com> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Link: https://lore.kernel.org/qemu-devel/20250604062115.4004200-5-zhenzhong.duan@intel.com Signed-off-by: Cédric Le Goater <clg@redhat.com>
Zhenzhong Duan [Wed, 4 Jun 2025 06:21:13 +0000 (14:21 +0800)]
vfio/iommufd: Add properties and handlers to TYPE_HOST_IOMMU_DEVICE_IOMMUFD
Enhance HostIOMMUDeviceIOMMUFD object with 3 new members, specific
to the iommufd BE + 2 new class functions.
IOMMUFD BE includes IOMMUFD handle, devid and hwpt_id. IOMMUFD handle
and devid are used to allocate/free ioas and hwpt. hwpt_id is used to
re-attach IOMMUFD backed device to its default VFIO sub-system created
hwpt, i.e., when vIOMMU is disabled by guest. These properties are
initialized in hiod::realize() after attachment.
2 new class functions are [at|de]tach_hwpt(). They are used to
attach/detach hwpt. VFIO and VDPA can have different implementions,
so implementation will be in sub-class instead of HostIOMMUDeviceIOMMUFD,
e.g., in HostIOMMUDeviceIOMMUFDVFIO.
Add two wrappers host_iommu_device_iommufd_[at|de]tach_hwpt to wrap the
two functions.
John Levon [Wed, 21 May 2025 21:55:34 +0000 (22:55 +0100)]
vfio/container: pass MemoryRegion to DMA operations
Pass through the MemoryRegion to DMA operation handlers of vfio
containers. The vfio-user container will need this later, to translate
the vaddr into an offset for the dma map vfio-user message; CPR will
also will need this.
Originally-by: John Johnson <john.g.johnson@oracle.com> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> Signed-off-by: John Levon <john.levon@nutanix.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Reviewed-by: Steve Sistare <steven.sistare@oracle.com> Link: https://lore.kernel.org/qemu-devel/20250521215534.2688540-1-john.levon@nutanix.com Signed-off-by: Cédric Le Goater <clg@redhat.com>
Steve Sistare [Mon, 19 May 2025 13:26:43 +0000 (06:26 -0700)]
vfio: return mr from vfio_get_xlat_addr
Modify memory_get_xlat_addr and vfio_get_xlat_addr to return the memory
region that the translated address is found in. This will be needed by
CPR in a subsequent patch to map blocks using IOMMU_IOAS_MAP_FILE.
Also return the xlat offset, so we can simplify the interface by removing
the out parameters that can be trivially derived from mr and xlat.
Lastly, rename the functions to to memory_translate_iotlb() and
vfio_translate_iotlb().
Signed-off-by: Steve Sistare <steven.sistare@oracle.com> Acked-by: David Hildenbrand <david@redhat.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Reviewed-by: John Levon <john.levon@nutanix.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com> Link: https://lore.kernel.org/qemu-devel/1747661203-136490-1-git-send-email-steven.sistare@oracle.com Signed-off-by: Cédric Le Goater <clg@redhat.com>
Tomita Moeko [Thu, 22 May 2025 15:16:36 +0000 (23:16 +0800)]
vfio/igd: Fix incorrect error propagation in vfio_pci_igd_opregion_detect()
In vfio_pci_igd_opregion_detect(), errp will be set when the device does
not have OpRegion or is hotplugged. This errp will be propagated to
pci_qdev_realize(), which interprets it as failure, causing unexpected
termination on devices without OpRegion like SR-IOV VFs or discrete
GPUs. Fix it by not setting errp in vfio_pci_igd_opregion_detect().
This patch also checks if the device has OpRegion before hotplug status
to prevent unwanted warning messages on non-IGD devices.
Zhenzhong Duan [Wed, 21 May 2025 11:03:01 +0000 (19:03 +0800)]
vfio/iommufd: Add comment emphasizing no movement of hiod->realize() call
The nested IOMMU support needs device and hwpt id which are generated
only after attachment. Hiod encapsulates these information in realize()
and passes to vIOMMU.
John Levon [Tue, 20 May 2025 15:03:52 +0000 (16:03 +0100)]
vfio: move config space read into vfio_pci_config_setup()
Small cleanup that reduces duplicate code for vfio-user and reduces the
size of vfio_realize(); while we're here, correct that name to
vfio_pci_realize().
Fiona Ebner [Fri, 23 May 2025 07:02:11 +0000 (09:02 +0200)]
hw/core/qdev-properties-system: Add missing return in set_drive_helper()
Currently, changing the 'drive' property of e.g. a scsi-hd object will
result in an assertion failure if the aio context of the block node
it's replaced with doesn't match the current aio context:
The problematic scenario is already detected, but a 'return' statement
was missing.
Cc: qemu-stable@nongnu.org Fixes: d1a58c176a ("qdev: allow setting drive property for realized device") Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250523070211.280498-1-f.ebner@proxmox.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Stefan Hajnoczi [Thu, 29 May 2025 20:31:47 +0000 (16:31 -0400)]
iotests: fix 240
Commit 2e8e18c2e463 ("virtio-scsi: add iothread-vq-mapping parameter")
removed the limitation that virtio-scsi devices must successfully set
the AioContext on their BlockBackends. This was made possible thanks to
the QEMU multi-queue block layer.
This change broke qemu-iotests 240, which checks that adding a
virtio-scsi device with a drive that is already in another AioContext
will fail.
Update the test to take the relaxed behavior into account. I considered
removing this test case entirely, but the code coverage still seems
valuable.
Fixes: 2e8e18c2e463 ("virtio-scsi: add iothread-vq-mapping parameter") Reported-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Tested-by: Eric Blake <eblake@redhat.com>
Message-ID: <20250529203147.180338-1-stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:59 +0000 (17:10 +0200)]
block/io: remove duplicate GLOBAL_STATE_CODE() in bdrv_do_drained_end()
Both commit ab61335025 ("block: drain from main loop thread in
bdrv_co_yield_to_drain()") and commit d05ab380db ("block: Mark drain
related functions GRAPH_RDLOCK") introduced a GLOBAL_STATE_CODE()
macro in bdrv_do_drained_end(). The assertion of being in the main
thread cannot change here, so keep only the earlier instance.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-23-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Andrey Drobyshev [Fri, 30 May 2025 15:10:58 +0000 (17:10 +0200)]
iotests/graph-changes-while-io: add test case with removal of lower snapshot
This case is catching potential deadlock which takes place when job-dismiss
is issued when I/O requests are processed in a separate iothread.
See https://mail.gnu.org/archive/html/qemu-devel/2025-04/msg04421.html
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
[FE: re-use top image and rename snap1->mid as suggested by Kevin Wolf
remove image file after test as suggested by Kevin Wolf
add type annotation for function argument to make mypy happy] Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-22-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:10:57 +0000 (17:10 +0200)]
iotests/graph-changes-while-io: remove image file after test
Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-21-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:10:56 +0000 (17:10 +0200)]
block: mark bdrv_drained_begin() and friends as GRAPH_UNLOCKED
All of bdrv_drain_all_begin(), bdrv_drain_all() and
bdrv_drained_begin() poll and are not allowed to be called with the
block graph lock held. Mark the function as such.
Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-20-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:10:55 +0000 (17:10 +0200)]
blockdev: drain while unlocked in external_snapshot_action()
This is in preparation to mark bdrv_drained_begin() as GRAPH_UNLOCKED.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-19-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:10:54 +0000 (17:10 +0200)]
blockdev: drain while unlocked in internal_snapshot_action()
This is in preparation to mark bdrv_drained_begin() as GRAPH_UNLOCKED.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-18-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:10:53 +0000 (17:10 +0200)]
block: move drain outside of quorum_del_child()
The quorum_del_child() callback runs under the graph lock, so it is
not allowed to drain. It is only called as the .bdrv_del_child()
callback, which is only called in the bdrv_del_child() function, which
also runs under the graph lock.
The bdrv_del_child() function is called by qmp_x_blockdev_change().
A drained section was already introduced there by commit "block: move
drain out of quorum_add_child()".
This finally finishes moving out the drain to places that are not
under the graph lock started in "block: move draining out of
bdrv_change_aio_context() and mark GRAPH_RDLOCK".
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-17-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:10:52 +0000 (17:10 +0200)]
block: move drain outside of bdrv_root_unref_child()
This is part of resolving the deadlock mentioned in commit "block:
move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK".
bdrv_root_unref_child() is called by:
1. blk_remove_bs(), where a drained section is introduced.
2. bdrv_unref_child(), which runs under the graph lock, so the drain
will be moved further up to its callers.
3. block_job_remove_all_bdrv(), where a drained section is introduced.
For all callers of bdrv_unref_child() and its generated
bdrv_co_unref_child() coroutine variant, a drained section is
introduced, they are not explicilty listed here. The caller
quorum_del_child() holds the graph lock, so it is not actually allowed
to drain. This will be addressed in the next commit.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-16-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:10:51 +0000 (17:10 +0200)]
block: move drain outside of quorum_add_child()
This is part of resolving the deadlock mentioned in commit "block:
move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK".
The quorum_add_child() callback runs under the graph lock, so it is
not allowed to drain. It is only called as the .bdrv_add_child()
callback, which is only called in the bdrv_add_child() function, which
also runs under the graph lock.
The bdrv_add_child() function is called by qmp_x_blockdev_change(),
where a drained section is introduced.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-15-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:10:50 +0000 (17:10 +0200)]
block: move drain outside of bdrv_attach_child()
This is part of resolving the deadlock mentioned in commit "block:
move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK".
The function bdrv_attach_child() runs under the graph lock, so it is
not allowed to drain. It is called by:
1. replication_start()
2. quorum_add_child()
3. bdrv_open_child_common()
4. Throughout test-bdrv-graph-mod.c and test-bdrv-drain.c unit tests.
In all callers, a drained section is introduced.
The function quorum_add_child() runs under the graph lock, so it is
not actually allowed to drain. This will be addressed by the following
commit.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-14-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:49 +0000 (17:10 +0200)]
block: move drain outside of bdrv_root_attach_child()
This is part of resolving the deadlock mentioned in commit "block:
move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK".
The function bdrv_root_attach_child() runs under the graph lock, so it
is not allowed to drain. It is called by:
1. blk_insert_bs(), where a drained section is introduced.
2. block_job_add_bdrv(), which holds the graph lock itself.
block_job_add_bdrv() is called by:
1. mirror_start_job()
2. stream_start()
3. commit_start()
4. backup_job_create()
5. block_job_create()
6. In the test_blockjob_common_drain_node() unit test
In all callers, a drained section is introduced.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-13-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:48 +0000 (17:10 +0200)]
block: move drain outside of bdrv_set_backing_hd_drained()
This is part of resolving the deadlock mentioned in commit "block:
move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK".
The function bdrv_set_backing_hd_drained() holds the graph lock, so it
is not allowed to drain. It is called by:
1. bdrv_set_backing_hd(), where a drained section is introduced,
replacing the previously present bs-specific drains.
2. stream_prepare(), where a drained section is introduced replacing
the previously present bs-specific drains.
The drain_bs variable in bdrv_set_backing_hd_drained() is now
superfluous and thus dropped.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-12-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:10:47 +0000 (17:10 +0200)]
block: move drain outside of bdrv_attach_child_common(_abort)()
This is part of resolving the deadlock mentioned in commit "block:
move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK".
The function bdrv_attach_child_common_abort() is used only as the
abort callback in bdrv_attach_child_common_drv transactions, so the
tran_finalize() calls of such transactions need to be in drained
sections too.
All code paths are covered:
The bdrv_attach_child_common_drv transactions are only used in
bdrv_attach_child_common(), so it is enough to check callers of
bdrv_attach_child_common() following the transactions.
bdrv_attach_child_common() is called by:
1. bdrv_attach_child_noperm(), which does not finalize the
transaction yet.
2. bdrv_root_attach_child(), where a drained section is introduced.
bdrv_attach_child_noperm() is called by:
1. bdrv_attach_child(), where a drained section is introduced.
2. bdrv_set_file_or_backing_noperm(), which does not finalize the
transaction yet.
3. bdrv_append(), where a drained section is introduced.
bdrv_set_file_or_backing_noperm() is called by:
1. bdrv_set_backing_hd_drained(), where a drained section is
introduced.
2. bdrv_reopen_parse_file_or_backing(), which does not finalize the
transaction yet. Draining the old child bs currently happens under
the graph lock there. This is replaced with an assertion, because
the drain will be moved further up to the caller.
bdrv_reopen_parse_file_or_backing() is called by:
1. bdrv_reopen_prepare(), which does not finalize the transaction yet.
bdrv_reopen_prepare() is called by:
1. bdrv_reopen_multiple(), which does finalize the transaction. It is
called after bdrv_reopen_queue(), which starts a drained section.
The drained section ends, when bdrv_reopen_queue_free() is called
at the end of bdrv_reopen_multiple().
This resolves all code paths.
The functions bdrv_set_backing_hd_drained(), bdrv_attach_child() and
bdrv_root_attach_child() run under the graph lock, so they are not
actually allowed to drain. This will be addressed in the following
commits.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20250530151125.955508-11-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:10:46 +0000 (17:10 +0200)]
block: move drain outside of bdrv_try_change_aio_context()
This is part of resolving the deadlock mentioned in commit "block:
move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK".
Convert the function to a _locked() version that has to be called with
the graph lock held and add a convenience wrapper that has to be
called with the graph unlocked, which drains and takes the lock
itself. Since bdrv_try_change_aio_context() is global state code, the
wrapper is too.
Callers are adapted to use the appropriate variant, depending on
whether the caller already holds the lock. In the
test_set_aio_context() unit test, prior drains can be removed, because
draining already happens inside the new wrapper.
Note that bdrv_attach_child_common_abort(), bdrv_attach_child_common()
and bdrv_root_unref_child() hold the graph lock and are not actually
allowed to drain either. This will be addressed in the following
commits.
Functions like qmp_blockdev_mirror() query the nodes to act on before
draining and locking. In theory, draining could invalidate those nodes.
This kind of issue is not addressed by these commits.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-10-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:45 +0000 (17:10 +0200)]
block: move drain outside of bdrv_change_aio_context() and mark GRAPH_RDLOCK
This is in preparation to mark bdrv_drained_begin() as GRAPH_UNLOCKED.
Note that even if bdrv_drained_begin() were already marked as
GRAPH_UNLOCKED, TSA would not complain about the instance in
bdrv_change_aio_context() before this change, because it is preceded
by a bdrv_graph_rdunlock_main_loop() call. It is not correct to
release the lock here, and in case the caller holds a write lock, it
wouldn't actually release the lock.
In combination with block-stream, there is a deadlock that can happen
because of this [0]. In particular, it can happen that
main thread IO thread
1. acquires write lock
in blk_co_do_preadv_part():
2. have non-zero blk->in_flight
3. try to acquire read lock
4. begin drain
Steps 3 and 4 might be switched. Draining will poll and get stuck,
because it will see the non-zero in_flight counter. But the IO thread
will not make any progress either, because it cannot acquire the read
lock.
After this change, all paths to bdrv_change_aio_context() drain:
bdrv_change_aio_context() is called by:
1. bdrv_child_cb_change_aio_ctx() which is only called via the
change_aio_ctx() callback, see below.
2. bdrv_child_change_aio_context(), see below.
3. bdrv_try_change_aio_context(), where a drained section is
introduced.
The change_aio_ctx() callback is called by:
1. bdrv_attach_child_common_abort(), where a drained section is
introduced.
2. bdrv_attach_child_common(), where a drained section is introduced.
3. bdrv_parent_change_aio_context(), see below.
bdrv_child_change_aio_context() is called by:
1. bdrv_change_aio_context(), i.e. recursive, so being in a drained
section is invariant.
2. child_job_change_aio_ctx(), which is only called via the
change_aio_ctx() callback, see above.
bdrv_parent_change_aio_context() is called by:
1. bdrv_change_aio_context(), i.e. recursive, so being in a drained
section is invariant.
This resolves all code paths. Note that bdrv_attach_child_common()
and bdrv_attach_child_common_abort() hold the graph write lock and
callers of bdrv_try_change_aio_context() might too, so they are not
actually allowed to drain either. This will be addressed in the
following commits.
More granular draining is not trivially possible, because
bdrv_change_aio_context() can recursively call itself e.g. via
bdrv_child_change_aio_context().
Reported-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-9-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:44 +0000 (17:10 +0200)]
block: mark bdrv_child_change_aio_context() GRAPH_RDLOCK
This is a small step in preparation to mark bdrv_drained_begin() as
GRAPH_UNLOCKED. More concretely, it is in preparation to move the
drain out of bdrv_change_aio_context() and marking that function as
GRAPH_RDLOCK.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-8-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:43 +0000 (17:10 +0200)]
block: mark change_aio_ctx() callback and instances as GRAPH_RDLOCK(_PTR)
This is a small step in preparation to mark bdrv_drained_begin() as
GRAPH_UNLOCKED. More concretely, it is in preparation to move the
drain out of bdrv_change_aio_context() and marking that function as
GRAPH_RDLOCK.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-7-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:42 +0000 (17:10 +0200)]
block: mark bdrv_parent_change_aio_context() GRAPH_RDLOCK
This is a small step in preparation to mark bdrv_drained_begin() as
GRAPH_UNLOCKED. More concretely, it allows marking the
change_aio_ctx() callback GRAPH_RDLOCK_PTR, which is the next step.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-6-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:41 +0000 (17:10 +0200)]
block: move drain outside of read-locked bdrv_inactivate_recurse()
This is in preparation to mark bdrv_drained_begin() as GRAPH_UNLOCKED.
More granular draining is not trivially possible, because
bdrv_inactivate_recurse() can recursively call itself.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-5-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:40 +0000 (17:10 +0200)]
block/snapshot: move drain outside of read-locked bdrv_snapshot_delete()
This is in preparation to mark bdrv_drained_begin() as GRAPH_UNLOCKED.
More granular draining is not trivially possible, because
bdrv_snapshot_delete() can recursively call itself.
The return value of bdrv_all_delete_snapshot() changes from -1 to
-errno propagated from failed sub-calls. This is fine for the existing
callers of bdrv_all_delete_snapshot().
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-4-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:39 +0000 (17:10 +0200)]
block: move drain outside of read-locked bdrv_reopen_queue_child()
This is in preparation to mark bdrv_drained_begin() as GRAPH_UNLOCKED.
More granular draining is not trivially possible, because
bdrv_reopen_queue_child() can recursively call itself.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-3-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fiona Ebner [Fri, 30 May 2025 15:10:38 +0000 (17:10 +0200)]
block: remove outdated comments about AioContext locking
AioContext locking was removed in commit b49f4755c7 ("block: remove
AioContext locking").
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20250530151125.955508-2-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Stefan Hajnoczi [Wed, 4 Jun 2025 15:43:30 +0000 (11:43 -0400)]
Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging
* rust: use native Meson support for clippy and rustdoc
* rust: add "bits", a custom bitflags implementation
* target/i386: Remove FRED dependency on WRMSRNS
* target/i386: Add the immediate form MSR access instruction support
* TDX fixes
* tag 'for-upstream' of https://gitlab.com/bonzini/qemu:
rust: qemu-api-macros: add from_bits and into_bits to #[derive(TryInto)]
rust: pl011: use the bits macro
rust: add "bits", a custom bitflags implementation
i386/tdvf: Fix build on 32-bit host
i386/tdx: Fix build on 32-bit host
meson: use config_base_arch for target libraries
target/i386: Add the immediate form MSR access instruction support
target/i386: Add a new CPU feature word for CPUID.7.1.ECX
target/i386: Remove FRED dependency on WRMSRNS
rust: use native Meson support for clippy and rustdoc
rust: cell: remove support for running doctests with "cargo test --doc"
rust: add qemu-api doctests to "meson test"
build, dockerfiles: add support for detecting rustdoc
rust: use "objects" for Rust executables as well
meson: update to version 1.8.1
rust: bindings: allow ptr_offset_with_cast
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Paolo Bonzini [Tue, 3 Jun 2025 15:44:54 +0000 (17:44 +0200)]
rust: add "bits", a custom bitflags implementation
One common thing that device emulation does is manipulate bitmasks, for example
to check whether two bitmaps have common bits. One example in the pl011 crate
is the checks for pending interrupts, where an interrupt cause corresponds to
at least one interrupt source from a fixed set.
Unfortunately, this is one case where Rust *can* provide some kind of
abstraction but it does so with a rather Perl-ish There Is More Way To
Do It. It is not something where a crate like "bilge" helps, because
it only covers the packing of bits in a structure; operations like "are
all bits of Y set in X" almost never make sense for bit-packed structs;
you need something else, there are several crates that do it and of course
we're going to roll our own.
In particular I examined three:
- bitmask (https://docs.rs/bitmask/0.5.0/bitmask/) does not support const
at all. This is a showstopper because one of the ugly things in the
current pl011 code is the ugliness of code that defines interrupt masks
at compile time:
You would have to use roughly the same code---"bitmask" only helps with
defining the struct.
- bitmask_enum (https://docs.rs/bitmask-enum/2.2.5/bitmask_enum/) does not
have a good separation of "valid" and "invalid" bits, so for example "!x"
will invert all 16 bits if you choose u16 as the representation -- even if
you only defined 10 bits. This makes it easier to introduce subtle bugs
in comparisons.
- bitflags (https://docs.rs/bitflags/2.6.0/bitflags/) is generally the most
used such crate and is the one that I took most inspiration from with
respect to the syntax. It's a pretty sophisticated implementation,
with a lot of bells and whistles such as an implementation of "Iter"
that returns the bits one at a time.
The main thing that all of them lack, however, is a way to simplify the
ugly definitions like the above. "bitflags" includes const methods that
perform AND/OR/XOR of masks (these are necessary because Rust operator
overloading does not support const yet, and therefore overloaded operators
cannot be used in the definition of a "static" variable), but they become
even more verbose and unmanageable, like
This was the main reason to create "bits", which allows something like
bits!(Interrupt: E | MS | RT | TX | RX)
and expands it 1) add "Interrupt::" in front of all identifiers 2) convert
operators to the wordy const functions like "union". It supports boolean
operators "&", "|", "^", "!" and parentheses, with a relatively simple
recursive descent parser that's implemented in qemu_api_macros.
Since I don't remember exactly how the macro was developed, I cannot exclude
that it contains code from "bitflags". Therefore, I am conservatively leaving
in the MIT and Apache 2.0 licenses from bitflags. In fact, I think there
would be a benefit in being able to push code back to "bitflags" anyway
whenever applicable, so that the two libraries do not diverge too much,
so that's another reason to use this.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Fixed commit introduced common dependencies for target libraries. Alas,
it wrongly reused the 'target' variable, which was previously set from
another loop.
Thus, some dependencies were missing depending on order of target list,
as found here [1].
The fix is to use the correct config_base_arch instead.
Kudos to Thomas Huth who had this right, before I reimplement it, and
introduce this bug.
Xin Li (Intel) [Fri, 3 Jan 2025 08:48:27 +0000 (00:48 -0800)]
target/i386: Add the immediate form MSR access instruction support
The immediate form of MSR access instructions are primarily motivated by
performance, not code size: by having the MSR number in an immediate, it
is available *much* earlier in the pipeline, which allows the hardware
much more leeway about how a particular MSR is handled.
Paolo Bonzini [Sat, 5 Apr 2025 12:18:17 +0000 (14:18 +0200)]
rust: use native Meson support for clippy and rustdoc
Meson has support for invoking clippy and rustdoc on all crates (1.7.0 for
clippy, 1.8.0 for rustdoc). Use it instead of the homegrown version; this
requires disabling the multiple_crate_versions lint (the only one that was
enabled from the "cargo" group)---which was not particularly useful anyway
because all dependencies are converted by hand into Meson subprojects.
rustfmt is still not supported.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Sat, 5 Apr 2025 08:33:09 +0000 (10:33 +0200)]
rust: add qemu-api doctests to "meson test"
Doctests are weird. They are essentially integration tests, but they're
"ran" by executing rustdoc --test, which takes a compiler-ish
command line. This is supported by Meson 1.8.0.
Because they run the linker and need all the .o files, run them in the
build jobs rather than the test jobs.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Fri, 2 May 2025 17:06:43 +0000 (19:06 +0200)]
build, dockerfiles: add support for detecting rustdoc
rustdoc is effectively a custom version of rustc, and it is necessary to
specify it in order to run doctests from Meson. Add the relevant configure
option and environment variables.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Thu, 27 Feb 2025 12:36:49 +0000 (13:36 +0100)]
rust: use "objects" for Rust executables as well
libqemuutil is not meant be linked as a whole; if modules are enabled, doing
so results in undefined symbols (corresponding to QMP commands) in
rust/qemu-api/rust-qemu-api-integration.
Support for "objects" in Rust executables is available in Meson 1.8.0; use it
to switching to the same dependencies that C targets use: link_with for
libqemuutil, and objects for everything else.
Reported-by: Bernhard Beschow <shentey@gmail.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Wed, 11 Dec 2024 17:14:19 +0000 (18:14 +0100)]
rust: bindings: allow ptr_offset_with_cast
This is produced by recent versions of bindgen:
warning: use of `offset` with a `usize` casted to an `isize`
--> /builds/bonzini/qemu/rust/target/debug/build/qemu_api-35cb647f4db404b8/out/bindings.inc.rs:39:21
|
39 | let byte = *(core::ptr::addr_of!((*this).storage) as *const u8).offset(byte_index as isize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(core::ptr::addr_of!((*this).storage) as *const u8).add(byte_index)`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
= note: `#[warn(clippy::ptr_offset_with_cast)]` on by default
warning: use of `offset` with a `usize` casted to an `isize`
--> /builds/bonzini/qemu/rust/target/debug/build/qemu_api-35cb647f4db404b8/out/bindings.inc.rs:68:13
|
68 | (core::ptr::addr_of_mut!((*this).storage) as *mut u8).offset(byte_index as isize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(core::ptr::addr_of_mut!((*this).storage) as *mut u8).add(byte_index)`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
This seems to be new in bindgen 0.71.0, possibly related to bindgen
commit 33006185b7878 ("Add raw_ref_macros feature", 2024-11-22).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* tag 'pull-qapi-2025-06-03' of https://repo.or.cz/qemu/armbru:
qapi: Improve documentation around job state @concluded
qapi: Tidy up references to job state CONCLUDED
qapi: Mention both job-cancel and block-job-cancel in doc comments
qapi: Refer to job-FOO instead of deprecated block-job-FOO in docs
qapi: Spell JSON null correctly in blockdev-reopen documentation
qapi: Use proper markup instead of CAPS for emphasis in doc comments
qapi: Fix capitalization in doc comments
qapi: Correct spelling of QEMU in doc comments
qapi: Drop a problematic (Since: 2.11) from query-hotpluggable-cpus
qapi: Avoid breaking lines within (since X.Y)
qapi: Move (since X.Y) to end of description
qapi: Tidy up whitespace in doc comments
qapi: Tidy up run-together sentences in doc comments
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
qapi: Improve documentation around job state @concluded
We use "the query list" in a few places. It's not entirely obvious
what that means. It's actually the output of query-jobs or
query-block-jobs.
Documentation of @auto-dismiss talks about the job disappearing from
the query list when it reaches state @concluded. This is less than
precise. The job doesn't merely disappear from the query list, it
disappears, period.
Documentation of JobStatus @concluded explains "the job will remain in
the query list until it is dismissed". Again less than precise. It
remains in state @concluded until dismissed.
Rephrase without use of "the query list" for clarity and precision.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-14-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
When talking about the job state machine, we refer to the states like
READY, ABORTING, CONCLUDED, and so forth. Except in two places, where
we use JOB_STATUS_CONCLUDED. Replace by CONCLUDED for consistency.
We should arguably use the JobStatus enum values instead. Left for
another day.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-13-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
qapi: Mention both job-cancel and block-job-cancel in doc comments
Several doc comments mention block-job-cancel where the more generic
job-cancel would also work. Adjust them to mention both.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-12-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
qapi: Refer to job-FOO instead of deprecated block-job-FOO in docs
We deprecated several block-job-FOO commands in commit b836bf2ab68 (qapi/block-core: deprecate some block-job- APIs). Update
the doc comments to refer to their replacements instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-11-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
qapi: Spell JSON null correctly in blockdev-reopen documentation
The doc comment misspells JSON null as NULL. Fix that.
Cc: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-10-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
qapi: Use proper markup instead of CAPS for emphasis in doc comments
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-9-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-8-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Improve awkward phrasing in migrate-incoming While there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-7-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
qapi: Drop a problematic (Since: 2.11) from query-hotpluggable-cpus
There is a (Since: 2.11) in a query-hotpluggable-cpus example.
Versioning information ought to be in the command description, not
examples. The command description is basically empty (there is a TODO
about it).
What exactly didn't work before 2.11 is not quite clear from the
documentation. The example was added in commit 4dc3b151882 (s390x:
implement query-hotpluggable-cpus), which suggests the command failed
for the s390x target until then. This was almost eight years ago, and
I doubt anyone still cares about this detail. Simply delete
the problematic (Since: 2.11).
Cc: David Hildenbrand <david@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-6-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-5-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
By convention, we put (since X.Y) at the end of the description. Move
the ones that somehow ended up in the middle of the description to the
end.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-4-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-3-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
qapi: Tidy up run-together sentences in doc comments
Fixes: a937b6aa739f (qapi: Reformat doc comments to conform to current conventions) Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250527073916.1243024-2-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>