aboutsummaryrefslogtreecommitdiff
path: root/block
AgeCommit message (Collapse)Author
2022-12-21block/io: Check for replay-enabled in bdrv_drain_all_begin()Peter Maydell
In commit da0bd74434 we refactored bdrv_drain_all_begin() to pull out the non-polling part into bdrv_drain_all_begin_nopoll(). This change broke record-and-replay, because the "return early if replay enabled" check is now in the sub-function bdrv_drain_all_begin_nopoll(), and so it only causes us to return from that function, and not from the calling bdrv_drain_all_begin(). Fix the regression by checking whether replay is enabled in both functions. The breakage and fix can be tested via 'make check-avocado': the tests/avocado/reverse_debugging.py:ReverseDebugging_X86_64.test_x86_64_pc tests/avocado/reverse_debugging.py:ReverseDebugging_AArch64.test_aarch64_virt tests were both broken by this. Fixes: da0bd744344adb1f285 ("block: Factor out bdrv_drain_all_begin_nopoll()") Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Tested-by: Fabiano Rosas <farosas@suse.de> Message-id: 20221220174638.2156308-1-peter.maydell@linaro.org
2022-12-15block: GRAPH_RDLOCK for functions only called by co_wrappersKevin Wolf
The generated coroutine wrappers already take care to take the lock in the non-coroutine path, and assume that the lock is already taken in the coroutine path. The only thing we need to do for the wrapped function is adding the GRAPH_RDLOCK annotation. Doing so also allows us to mark the corresponding callbacks in BlockDriver as GRAPH_RDLOCK_PTR. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221207131838.239125-19-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: use co_wrapper_mixed_bdrv_rdlock in functions taking the rdlockEmanuele Giuseppe Esposito
Take the rdlock already, before we add the assertions. All these functions either read the graph recursively, or call BlockDriver callbacks that will eventually need to be protected by the graph rdlock. Do it now to all functions together, because many of these recursively call each other. For example, bdrv_co_truncate calls BlockDriver->bdrv_co_truncate, and some driver callbacks implement their own .bdrv_co_truncate by calling bdrv_flush inside. So if bdrv_flush asserts but bdrv_truncate does not take the rdlock yet, the assertion will always fail. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221207131838.239125-18-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15graph-lock: TSA annotations for lock/unlock functionsKevin Wolf
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221207131838.239125-15-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: assert that graph read and writes are performed correctlyEmanuele Giuseppe Esposito
Remove the old assert_bdrv_graph_writable, and replace it with the new version using graph-lock API. See the function documentation for more information. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221207131838.239125-14-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15graph-lock: Introduce a lock to protect block graph operationsPaolo Bonzini
Block layer graph operations are always run under BQL in the main loop. This is proved by the assertion qemu_in_main_thread() and its wrapper macro GLOBAL_STATE_CODE. However, there are also concurrent coroutines running in other iothreads that always try to traverse the graph. Currently this is protected (among various other things) by the AioContext lock, but once this is removed, we need to make sure that reads do not happen while modifying the graph. We distinguish between writer (main loop, under BQL) that modifies the graph, and readers (all other coroutines running in various AioContext), that go through the graph edges, reading ->parents and->children. The writer (main loop) has "exclusive" access, so it first waits for any current read to finish, and then prevents incoming ones from entering while it has the exclusive access. The readers (coroutines in multiple AioContext) are free to access the graph as long the writer is not modifying the graph. In case it is, they go in a CoQueue and sleep until the writer is done. If a coroutine changes AioContext, the counter in the original and new AioContext are left intact, since the writer does not care where the reader is, but only if there is one. As a result, some AioContexts might have a negative reader count, to balance the positive count of the AioContext that took the lock. This also means that when an AioContext is deleted it may have a nonzero reader count. In that case we transfer the count to a global shared counter so that the writer is always aware of all readers. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221207131838.239125-3-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Factor out bdrv_drain_all_begin_nopoll()Kevin Wolf
Provide a separate function that just quiesces the users of a node to prevent new requests from coming in, but without waiting for the already in-flight I/O to complete. This function can be used in contexts where polling is not allowed. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221207131838.239125-2-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block/dirty-bitmap: convert coroutine-only functions to co_wrapperEmanuele Giuseppe Esposito
bdrv_can_store_new_dirty_bitmap and bdrv_remove_persistent_dirty_bitmap check if they are running in a coroutine, directly calling the coroutine callback if it's the case. Except that no coroutine calls such functions, therefore that check can be removed, and function creation can be offloaded to c_w. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20221128142337.657646-15-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block-coroutine-wrapper.py: support also basic return typesEmanuele Giuseppe Esposito
Extend the regex to cover also return type, pointers included. This implies that the value returned by the function cannot be a simple "int" anymore, but the custom return type. Therefore remove poll_state->ret and instead use a per-function custom "ret" field. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20221128142337.657646-13-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block-coroutine-wrapper.py: support functions without bs argEmanuele Giuseppe Esposito
Right now, we take the first parameter of the function to get the BlockDriverState to pass to bdrv_poll_co(), that internally calls functions that figure in which aiocontext the coroutine should run. However, it is useless to pass a bs just to get its own AioContext, so instead pass it directly, and default to the main loop if no BlockDriverState is passed as parameter. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20221128142337.657646-12-eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: rename generated_co_wrapper in co_wrapper_mixedEmanuele Giuseppe Esposito
In preparation to the incoming new function specifiers, rename g_c_w with a more meaningful name and document it. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20221128142337.657646-10-eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: bdrv_create_file is a coroutine_fnEmanuele Giuseppe Esposito
It is always called in coroutine_fn callbacks, therefore it can directly call bdrv_co_create(). Rename it to bdrv_co_create_file too. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20221128142337.657646-9-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block/vmdk: add coroutine_fn annotationsEmanuele Giuseppe Esposito
These functions end up calling bdrv_create() implemented as generated_co_wrapper functions. In addition, they also happen to be always called in coroutine context, meaning all callers are coroutine_fn. This means that the g_c_w function will enter the qemu_in_coroutine() case and eventually suspend (or in other words call qemu_coroutine_yield()). Therefore we can mark such functions coroutine_fn too. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20221128142337.657646-6-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block-backend: replace bdrv_*_above with blk_*_aboveEmanuele Giuseppe Esposito
Avoid mixing bdrv_* functions with blk_*, so create blk_* counterparts for bdrv_block_status_above and bdrv_is_allocated_above. Note that since blk_co_block_status_above only calls the g_c_w function bdrv_common_block_status_above and is marked as coroutine_fn, call directly bdrv_co_common_block_status_above() to avoid using a g_c_w. Same applies to blk_co_is_allocated_above. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20221128142337.657646-5-eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block-copy: add coroutine_fn annotationsEmanuele Giuseppe Esposito
These functions end up calling bdrv_common_block_status_above(), a generated_co_wrapper function. In addition, they also happen to be always called in coroutine context, meaning all callers are coroutine_fn. This means that the g_c_w function will enter the qemu_in_coroutine() case and eventually suspend (or in other words call qemu_coroutine_yield()). Therefore we can mark such functions coroutine_fn too. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20221128142337.657646-3-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block-io: introduce coroutine_fn duplicates for ↵Emanuele Giuseppe Esposito
bdrv_common_block_status_above callers bdrv_common_block_status_above() is a g_c_w, and it is being called by many "wrapper" functions like bdrv_is_allocated(), bdrv_is_allocated_above() and bdrv_block_status_above(). Because we want to eventually split the coroutine from non-coroutine case in g_c_w, create duplicate wrappers that take care of directly calling the same coroutine functions called in the g_c_w. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20221128142337.657646-2-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Remove poll parameter from bdrv_parent_drained_begin_single()Kevin Wolf
All callers of bdrv_parent_drained_begin_single() pass poll=false now, so we don't need the parameter any more. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221118174110.55183-16-kwolf@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Don't poll in bdrv_replace_child_noperm()Kevin Wolf
In order to make sure that bdrv_replace_child_noperm() doesn't have to poll any more, get rid of the bdrv_parent_drained_begin_single() call. This is possible now because we can require that the parent is already drained through the child in question when the function is called and we don't call the parent drain callbacks more than once. The additional drain calls needed in callers cause the test case to run its code in the drain handler too early (bdrv_attach_child() drains now), so modify it to only enable the code after the test setup has completed. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221118174110.55183-15-kwolf@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Drop out of coroutine in bdrv_do_drained_begin_quiesce()Kevin Wolf
The next patch adds a parent drain to bdrv_attach_child_common(), which shouldn't be, but is currently called from coroutines in some cases (e.g. .bdrv_co_create implementations generally open new nodes). Therefore, the assertion that we're not in a coroutine doesn't hold true any more. We could just remove the assertion because there is nothing in the function that should be in conflict with running in a coroutine, but just to be on the safe side, we can reverse the caller relationship between bdrv_do_drained_begin() and bdrv_do_drained_begin_quiesce() so that the latter also just drops out of coroutine context and we can still be certain in the future that any drain code doesn't run in coroutines. As a nice side effect, the structure of bdrv_do_drained_begin() is now symmetrical with bdrv_do_drained_end(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221118174110.55183-14-kwolf@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Remove ignore_bds_parents parameter from drain_begin/end.Kevin Wolf
ignore_bds_parents is now ignored during drain_begin and drain_end, so we can just remove it there. It is still a valid optimisation for drain_all in bdrv_drained_poll(), so leave it around there. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221118174110.55183-13-kwolf@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Call drain callbacks only onceKevin Wolf
We only need to call both the BlockDriver's callback and the parent callbacks when going from undrained to drained or vice versa. A second drain section doesn't make a difference for the driver or the parent, they weren't supposed to send new requests before and after the second drain. One thing that gets in the way is the 'ignore_bds_parents' parameter in bdrv_do_drained_begin_quiesce() and bdrv_do_drained_end(): It means that bdrv_drain_all_begin() increases bs->quiesce_counter, but does not quiesce the parent through BdrvChildClass callbacks. If an additional drain section is started now, bs->quiesce_counter will be non-zero, but we would still need to quiesce the parent through BdrvChildClass in order to keep things consistent (and unquiesce it on the matching bdrv_drained_end(), even though the counter would not reach 0 yet as long as the bdrv_drain_all() section is still active). Instead of keeping track of this, let's just get rid of the parameter. It was introduced in commit 6cd5c9d7b2d as an optimisation so that during bdrv_drain_all(), we wouldn't recursively drain all parents up to the root for each node, resulting in quadratic complexity. As it happens, calling the callbacks only once solves the same problem, so as of this patch, we'll still have O(n) complexity and ignore_bds_parents is not needed any more. This patch only ignores the 'ignore_bds_parents' parameter. It will be removed in a separate patch. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221118174110.55183-12-kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Remove subtree drainsKevin Wolf
Subtree drains are not used any more. Remove them. After this, BdrvChildClass.attach/detach() don't poll any more. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221118174110.55183-11-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15stream: Replace subtree drain with a single node drainKevin Wolf
The subtree drain was introduced in commit b1e1af394d9 as a way to avoid graph changes between finding the base node and changing the block graph as necessary on completion of the image streaming job. The block graph could change between these two points because bdrv_set_backing_hd() first drains the parent node, which involved polling and can do anything. Subtree draining was an imperfect way to make this less likely (because with it, fewer callbacks are called during this window). Everyone agreed that it's not really the right solution, and it was only committed as a stopgap solution. This replaces the subtree drain with a solution that simply drains the parent node before we try to find the base node, and then call a version of bdrv_set_backing_hd() that doesn't drain, but just asserts that the parent node is already drained. This way, any graph changes caused by draining happen before we start looking at the graph and things stay consistent between finding the base node and changing the graph. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221118174110.55183-10-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Drain individual nodes during reopenKevin Wolf
bdrv_reopen() and friends use subtree drains as a lazy way of covering all the nodes they touch. Turns out that this lazy way is a lot more complicated than just draining the nodes individually, even not accounting for the additional complexity in the drain mechanism itself. Simplify the code by switching to draining the individual nodes that are already managed in the BlockReopenQueue anyway. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221118174110.55183-8-kwolf@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Inline bdrv_drain_invoke()Kevin Wolf
bdrv_drain_invoke() has now two entirely separate cases that share no code any more and are selected depending on a bool parameter. Each case has only one caller. Just inline the function. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221118174110.55183-6-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Remove drained_end_counterKevin Wolf
drained_end_counter is unused now, nobody changes its value any more. It can be removed. In cases where we had two almost identical functions that only differed in whether the caller passes drained_end_counter, or whether they would poll for a local drained_end_counter to reach 0, these become a single function. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Message-Id: <20221118174110.55183-5-kwolf@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15block: Revert .bdrv_drained_begin/end to non-coroutine_fnKevin Wolf
Polling during bdrv_drained_end() can be problematic (and in the future, we may get cases for bdrv_drained_begin() where polling is forbidden, and we don't care about already in-flight requests, but just want to prevent new requests from arriving). The .bdrv_drained_begin/end callbacks running in a coroutine is the only reason why we have to do this polling, so make them non-coroutine callbacks again. None of the callers actually yield any more. This means that bdrv_drained_end() effectively doesn't poll any more, even if AIO_WAIT_WHILE() loops are still there (their condition is false from the beginning). This is generally not a problem, but in test-bdrv-drain, some additional explicit aio_poll() calls need to be added because the test case wants to verify the final state after BHs have executed. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221118174110.55183-4-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15qed: Don't yield in bdrv_qed_co_drain_begin()Kevin Wolf
We want to change .bdrv_co_drained_begin() back to be a non-coroutine callback, so in preparation, avoid yielding in its implementation. Because we increase bs->in_flight and bdrv_drained_begin() polls, the behaviour is unchanged. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221118174110.55183-2-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-12-15Merge tag 'pull-misc-2022-12-14' of https://repo.or.cz/qemu/armbru into stagingPeter Maydell
Miscellaneous patches for 2022-12-14 # gpg: Signature made Wed 14 Dec 2022 15:23:02 GMT # gpg: using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653 # gpg: issuer "armbru@redhat.com" # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full] # gpg: aka "Markus Armbruster <armbru@pond.sub.org>" [full] # Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867 4E5F 3870 B400 EB91 8653 * tag 'pull-misc-2022-12-14' of https://repo.or.cz/qemu/armbru: ppc4xx_sdram: Simplify sdram_ddr_size() to return block/vmdk: Simplify vmdk_co_create() to return directly cleanup: Tweak and re-run return_directly.cocci io: Tidy up fat-fingered parameter name qapi: Use returned bool to check for failure (again) sockets: Use ERRP_GUARD() where obviously appropriate qemu-config: Use ERRP_GUARD() where obviously appropriate qemu-config: Make config_parse_qdict() return bool monitor: Use ERRP_GUARD() in monitor_init() monitor: Simplify monitor_fd_param()'s error handling error: Move ERRP_GUARD() to the beginning of the function error: Drop a few superfluous ERRP_GUARD() error: Drop some obviously superfluous error_propagate() Drop more useless casts from void * to pointer Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2022-12-14qapi block: Elide redundant has_FOO in generated CMarkus Armbruster
The has_FOO for pointer-valued FOO are redundant, except for arrays. They are also a nuisance to work with. Recent commit "qapi: Start to elide redundant has_FOO in generated C" provided the means to elide them step by step. This is the step for qapi/block*.json. Said commit explains the transformation in more detail. There is one instance of the invariant violation mentioned there: qcow2_signal_corruption() passes false, "" when node_name is an empty string. Take care to pass NULL then. The previous two commits cleaned up two more. Additionally, helper bdrv_latency_histogram_stats() loses its output parameters and returns a value instead. Cc: Kevin Wolf <kwolf@redhat.com> Cc: Hanna Reitz <hreitz@redhat.com> Cc: qemu-block@nongnu.org Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20221104160712.3005652-11-armbru@redhat.com> [Fixes for #ifndef LIBRBD_SUPPORTS_ENCRYPTION and MacOS squashed in]
2022-12-14block/vmdk: Simplify vmdk_co_create() to return directlyMarkus Armbruster
Cc: Fam Zheng <fam@euphon.net> Cc: Kevin Wolf <kwolf@redhat.com> Cc: Hanna Reitz <hreitz@redhat.com> Cc: qemu-block@nongnu.org Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20221122134917.1217307-3-armbru@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
2022-12-14qemu-config: Make config_parse_qdict() return boolMarkus Armbruster
This simplifies error checking. Cc: Hanna Reitz <hreitz@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20221121085054.683122-7-armbru@redhat.com>
2022-12-14error: Drop a few superfluous ERRP_GUARD()Markus Armbruster
include/qapi/error.h on ERRP_GUARD(): * It must be used when the function dereferences @errp or passes * @errp to error_prepend(), error_vprepend(), or error_append_hint(). * It is safe to use even when it's not needed, but please avoid * cluttering the source with useless code. Clean up some of this clutter. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20221121085054.683122-3-armbru@redhat.com>
2022-11-29block-backend: avoid bdrv_unregister_buf() NULL pointer derefStefan Hajnoczi
bdrv_*() APIs expect a valid BlockDriverState. Calling them with bs=NULL leads to undefined behavior. Jonathan Cameron reported this following NULL pointer dereference when a VM with a virtio-blk device and a memory-backend-file object is terminated: 1. qemu_cleanup() closes all drives, setting blk->root to NULL 2. qemu_cleanup() calls user_creatable_cleanup(), which results in a RAM block notifier callback because the memory-backend-file is destroyed. 3. blk_unregister_buf() is called by virtio-blk's BlockRamRegistrar notifier callback and undefined behavior occurs. Fixes: baf422684d73 ("virtio-blk: use BDRV_REQ_REGISTERED_BUF optimization hint") Co-authored-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20221121211923.1993171-1-stefanha@redhat.com>
2022-11-10block: Start/end drain on correct AioContextHanna Reitz
bdrv_parent_drained_{begin,end}_single() are supposed to operate on the parent, not on the child, so they should not attempt to get the context to poll from the child but the parent instead. BDRV_POLL_WHILE(c->bs) does get the context from the child, so we should replace it with AIO_WAIT_WHILE() on the parent's context instead. This problem becomes apparent when bdrv_replace_child_noperm() invokes bdrv_parent_drained_end_single() after removing a child from a subgraph that is in an I/O thread. By the time bdrv_parent_drained_end_single() is called, child->bs is NULL, and so BDRV_POLL_WHILE(c->bs, ...) will poll the main loop instead of the I/O thread; but anything that bdrv_parent_drained_end_single_no_poll() may have scheduled is going to want to run in the I/O thread, but because we poll the main loop, the I/O thread is never unpaused, and nothing is run, resulting in a deadlock. Closes: https://gitlab.com/qemu-project/qemu/-/issues/1215 Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221107151321.211175-4-hreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-11-10block-backend: Update ctx immediately after rootHanna Reitz
blk_get_aio_context() asserts that blk->ctx is always equal to the root BDS's context (if there is a root BDS). Therefore, blk_do_set_aio_context() must update blk->ctx immediately after the root BDS's context has changed. Without this patch, the next patch would break iotest 238, because bdrv_drained_begin() (called by blk_do_set_aio_context()) may then invoke bdrv_child_get_parent_aio_context() on the root child, i.e. blk_get_aio_context(). However, by this point, blk->ctx would not have been updated and thus differ from the root node's context. This patch fixes that. Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221107151321.211175-3-hreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-11-10block: Make bdrv_child_get_parent_aio_context I/OHanna Reitz
We want to use bdrv_child_get_parent_aio_context() from bdrv_parent_drained_{begin,end}_single(), both of which are "I/O or GS" functions. Prior to 3ed4f708fe1, all the implementations were I/O code anyway. 3ed4f708fe1 has put block jobs' AioContext field under the job mutex, so to make child_job_get_parent_aio_context() work in an I/O context, we need to take that lock there. Furthermore, blk_root_get_parent_aio_context() is not marked as anything, but is safe to run in an I/O context, so mark it that way now. (blk_get_aio_context() is an I/O code function.) With that done, all implementations explicitly are I/O code, so we can mark bdrv_child_get_parent_aio_context() as I/O code, too, so callers know it is safe to run from both GS and I/O contexts. Signed-off-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221107151321.211175-2-hreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-11-10block/blkio: Set BlockDriver::has_variable_length to falseAlberto Faria
Setting it to true can cause the device size to be queried from libblkio in otherwise fast paths, degrading performance. Set it to false and require users to refresh the device size explicitly instead. Fixes: 4c8f4fda0504 ("block/blkio: Tolerate device size changes") Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Alberto Faria <afaria@redhat.com> Message-Id: <20221108144433.1334074-1-afaria@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-11-10block/mirror: Fix NULL s->job in active writesHanna Reitz
There is a small gap in mirror_start_job() before putting the mirror filter node into the block graph (bdrv_append() call) and the actual job being created. Before the job is created, MirrorBDSOpaque.job is NULL. It is possible that requests come in when bdrv_drained_end() is called, and those requests would see MirrorBDSOpaque.job == NULL. Have our filter node handle that case gracefully. Signed-off-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221109165452.67927-4-hreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-11-10block/mirror: Drop mirror_wait_for_any_operation()Hanna Reitz
mirror_wait_for_free_in_flight_slot() is the only remaining user of mirror_wait_for_any_operation(), so inline the latter into the former. Signed-off-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221109165452.67927-3-hreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-11-10block/mirror: Do not wait for active writesHanna Reitz
Waiting for all active writes to settle before daring to create a background copying operation means that we will never do background operations while the guest does anything (in write-blocking mode), and therefore cannot converge. Yes, we also will not diverge, but actually converging would be even nicer. It is unclear why we did decide to wait for all active writes to settle before creating a background operation, but it just does not seem necessary. Active writes will put themselves into the in_flight bitmap and thus properly block actually conflicting background requests. It is important for active requests to wait on overlapping background requests, which we do in active_write_prepare(). However, so far it was not documented why it is important. Add such documentation now, and also to the other call of mirror_wait_on_conflicts(), so that it becomes more clear why and when requests need to actively wait for other requests to settle. Another thing to note is that of course we need to ensure that there are no active requests when the job completes, but that is done by virtue of the BDS being drained anyway, so there cannot be any active requests at that point. With this change, we will need to explicitly keep track of how many bytes are in flight in active requests so that job_progress_set_remaining() in mirror_run() can set the correct number of remaining bytes. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2123297 Signed-off-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221109165452.67927-2-hreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-11-06dmg: warn when opening dmg images containing blocks of unknown typeKevin Wolf
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Claudio Fontana <cfontana@suse.de> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20220929093035.4231-5-cfontana@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2022-11-06module: add Error arguments to module_load and module_load_qomClaudio Fontana
improve error handling during module load, by changing: bool module_load(const char *prefix, const char *lib_name); void module_load_qom(const char *type); to: int module_load(const char *prefix, const char *name, Error **errp); int module_load_qom(const char *type, Error **errp); where the return value is: -1 on module load error, and errp is set with the error 0 on module or one of its dependencies are not installed 1 on module load success 2 on module load success (module already loaded or built-in) module_load_qom_one has been introduced in: commit 28457744c345 ("module: qom module support"), which built on top of module_load_one, but discarded the bool return value. Restore it. Adapt all callers to emit errors, or ignore them, or fail hard, as appropriate in each context. Replace the previous emission of errors via fprintf in _some_ error conditions with Error and error_report, so as to emit to the appropriate target. A memory leak is also fixed as part of the module_load changes. audio: when attempting to load an audio module, report module load errors. Note that still for some callers, a single issue may generate multiple error reports, and this could be improved further. Regarding the audio code itself, audio_add() seems to ignore errors, and this should probably be improved. block: when attempting to load a block module, report module load errors. For the code paths that already use the Error API, take advantage of those to report module load errors into the Error parameter. For the other code paths, we currently emit the error, but this could be improved further by adding Error parameters to all possible code paths. console: when attempting to load a display module, report module load errors. qdev: when creating a new qdev Device object (DeviceState), report load errors. If a module cannot be loaded to create that device, now abort execution (if no CONFIG_MODULE) or exit (if CONFIG_MODULE). qom/object.c: when initializing a QOM object, or looking up class_by_name, report module load errors. qtest: when processing the "module_load" qtest command, report errors in the load of the module. Signed-off-by: Claudio Fontana <cfontana@suse.de> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20220929093035.4231-4-cfontana@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2022-11-06module: rename module_load_one to module_loadClaudio Fontana
Signed-off-by: Claudio Fontana <cfontana@suse.de> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20220929093035.4231-3-cfontana@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2022-11-01Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into stagingStefan Hajnoczi
Pull request Note that we're still discussing "block/blkio: Make driver nvme-io_uring take a "path" instead of a "filename"". I have sent the pull request now so everything is ready for the soft freeze tomorrow if we decide to go ahead with the patch. # -----BEGIN PGP SIGNATURE----- # # iQEzBAABCAAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAmNgGQkACgkQnKSrs4Gr # c8hLFgf/dnszoHO02hjoJCN2LPAxDalyYKzog+ZU8U5VdzJn2gione1jVlf3Xb0l # mhTgrioSbKLKXavGZTSwWUki/xRgCJMtG3m07EFmMsLX0QiSOIyzLr0DslQawYdZ # FlXyCCyAVTUILz7oUXBqORlfTKsGPHms6nlXQYhitTOsDbPyqbT9nNPKAlfGkqfj # Pwn+oWJmjLC0aARpcrB1bXCMbqQrtZGh4bBgfIXRUJmprWqk227bkFvXNCuXU16x # PC4oH552+6nyQyRxGpHc3o1W/8gqlxU9DTBb5arDUQaDvsDTKVkuGe2HdDI7knAT # /m57/BFVUnA35SYOxX+0piiEbawI6Q== # =UWL7 # -----END PGP SIGNATURE----- # gpg: Signature made Mon 31 Oct 2022 14:50:49 EDT # gpg: using RSA key 8695A8BFD3F97CDAAC35775A9CA4ABB381AB73C8 # gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" [ultimate] # gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>" [ultimate] # Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35 775A 9CA4 ABB3 81AB 73C8 * tag 'block-pull-request' of https://gitlab.com/stefanha/qemu: block/blkio: Make driver nvme-io_uring take a "path" instead of a "filename" block/blkio: Tolerate device size changes block/blkio: Add virtio-blk-vfio-pci BlockDriver Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2022-10-31block/vvfat: Unify the mkdir() callBin Meng
There is a difference in the mkdir() call for win32 and non-win32 platforms, and currently is handled in the codes with #ifdefs. glib provides a portable g_mkdir() API and we can use it to unify the codes without #ifdefs. Signed-off-by: Bin Meng <bin.meng@windriver.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-Id: <20221006151927.2079583-6-bmeng.cn@gmail.com> Message-Id: <20221027183637.2772968-14-alex.bennee@linaro.org>
2022-10-31block/blkio: Make driver nvme-io_uring take a "path" instead of a "filename"Alberto Faria
The nvme-io_uring driver expects a character special file such as /dev/ng0n1. Follow the convention of having a "filename" option when a regular file is expected, and a "path" option otherwise. This makes io_uring the only libblkio-based driver with a "filename" option, as it accepts a regular file (even though it can also take a block special file). Signed-off-by: Alberto Faria <afaria@redhat.com> Message-id: 20221028233854.839933-1-afaria@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2022-10-31block/blkio: Tolerate device size changesAlberto Faria
Some libblkio drivers may be able to work with regular files (e.g., io_uring) or otherwise resizable devices. Conservatively set BlockDriver::has_variable_length to true to ensure bdrv_nb_sectors() always gives up-to-date results. Also implement BlockDriver::bdrv_co_truncate for the case where no preallocation is needed and the device already has a size compatible with what was requested. Signed-off-by: Alberto Faria <afaria@redhat.com> Message-id: 20221029122031.975273-1-afaria@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2022-10-31block/blkio: Add virtio-blk-vfio-pci BlockDriverAlberto Faria
libblkio 1.1.0 [1] introduces a virtio-blk-vfio-pci driver, which accesses a virtio-blk PCI device using VFIO. Add a corresponding BlockDriver. [1] https://gitlab.com/libblkio/libblkio/-/tree/v1.1.0 Signed-off-by: Alberto Faria <afaria@redhat.com> Message-id: 20221028131635.710267-1-afaria@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2022-10-30Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into stagingStefan Hajnoczi
Block layer patches - Cleanup bs->backing and bs->file handling - Refactor bdrv_try_set_aio_context using transactions - Changes for improved coroutine_fn consistency - vhost-user-blk: fix the resize crash - io_uring: Use of io_uring_register_ring_fd() led to breakage, revert - vvfat: Fix some problems with r/w mode - Code cleanup - MAINTAINERS: Fold "Block QAPI, monitor, ..." into "Block layer core" # -----BEGIN PGP SIGNATURE----- # # iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmNazhIRHGt3b2xmQHJl # ZGhhdC5jb20ACgkQfwmycsiPL9ZyTw/8Dfck/SuxfyeLlnQItkjaV4cnqWOU8vHs # 9x0KhlptCs+HXdF/3iicpA0lHojn7mNnbdFGjPRY4E0LriQv91TQ5ycdEmrseFPf # sgeQlgdKCVU/pHjZ2wYarm2pE43Cx85a5xuufmw+7w49dNNZn14l4t+DgviuClVM # nuVaogfZFbYyetre+Qd2TgLl+gJ+0d4o7Zs5lSWLrT8t0L9AGkcWPA7Nrbl6loIE # dOautV4G7jLjuMiCeJZOGcnuRVe3gCQ5rCGBFzzH4DUtz4BmiYx4hd3LMEsP0PMM # CrsfDZS04Ztybl9M7TmJuwkAm1gx1JDMOuJuh18lbJocIOBvhkKKxY2wI5LIdZVI # ZntmU36RowkX+GGu/PYpYyMjBDClJppZCl7vnjyLYsVt6r0Vu6SmlHpJhcRYabhe # 96Kv1LXH9A6+ogKPU3Layw6JGjg01GNr1ALuT7PO3pGto/JshmOuBEJJDucoF84M # 5AfxFCohMROVldwblA6M0eKnlQBgtr5BvtgbV54BBo88VlFJgDJFQn7R09cTFUEo # UwaJoS+nIaiZ0bQQVZhZloVppUaTdVJojzfVRCZZctga96/tu1HSFnGLnbEFpUN3 # KOf+XnVNS6Ro+nPSDf9bMjbIom2JicGFfV+6yMgIoxY/d5UA2dTZfefil4TAlSod # 6PsTgg+jrm8= # =/Fw0 # -----END PGP SIGNATURE----- # gpg: Signature made Thu 27 Oct 2022 14:29:38 EDT # gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6 # gpg: issuer "kwolf@redhat.com" # gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full] # Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6 * tag 'for-upstream' of https://repo.or.cz/qemu/kevin: (58 commits) block/block-backend: blk_set_enable_write_cache is IO_CODE monitor: switch to *_co_* functions vmdk: switch to *_co_* functions vhdx: switch to *_co_* functions vdi: switch to *_co_* functions qed: switch to *_co_* functions qcow2: switch to *_co_* functions qcow: switch to *_co_* functions parallels: switch to *_co_* functions mirror: switch to *_co_* functions block: switch to *_co_* functions commit: switch to *_co_* functions vmdk: manually add more coroutine_fn annotations qcow2: manually add more coroutine_fn annotations qcow: manually add more coroutine_fn annotations blkdebug: add missing coroutine_fn annotation for indirect-called functions qcow2: add coroutine_fn annotation for indirect-called functions block: add missing coroutine_fn annotation to BlockDriverState callbacks coroutine-io: add missing coroutine_fn annotation to prototypes coroutine-lock: add missing coroutine_fn annotation to prototypes ... Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
generated by cgit v1.2.3 (git 2.26.2) at 2024-12-27 13:45:00 +0000