#
755ae381 |
| 08-Dec-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Fix AioContext locking in qmp_block_resize()
The AioContext must be unlocked before calling blk_co_unref(), because it takes the AioContext lock internally in blk_unref_bh(), which is schedul
block: Fix AioContext locking in qmp_block_resize()
The AioContext must be unlocked before calling blk_co_unref(), because it takes the AioContext lock internally in blk_unref_bh(), which is scheduled in the main thread. If we don't unlock, the AioContext is locked twice and nested event loops such as in bdrv_graph_wrlock() will deadlock.
Cc: <qemu-stable@nongnu.org> Fixes: https://issues.redhat.com/browse/RHEL-15965 Fixes: 0c7d204f50c382c6baac8c94bd57af4a022b3888 Signed-off-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20231208124352.30295-1-kwolf@redhat.com>
show more ...
|
#
6bc0bcc8 |
| 15-Nov-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Fix deadlocks in bdrv_graph_wrunlock()
bdrv_graph_wrunlock() calls aio_poll(), which may run callbacks that have a nested event loop. Nested event loops can depend on other iothreads making p
block: Fix deadlocks in bdrv_graph_wrunlock()
bdrv_graph_wrunlock() calls aio_poll(), which may run callbacks that have a nested event loop. Nested event loops can depend on other iothreads making progress, so in order to allow them to make progress it must not hold the AioContext lock of another thread while calling aio_poll().
This introduces a @bs parameter to bdrv_graph_wrunlock() whose AioContext is temporarily dropped (which matches bdrv_graph_wrlock()), and a bdrv_graph_wrunlock_ctx() that can be used if the BlockDriverState doesn't necessarily exist any more when unlocking.
This also requires a change to bdrv_schedule_unref(), which was relying on the incorrectly taken lock. It needs to take the lock itself now. While this is a separate bug, it can't be fixed a separate patch because otherwise the intermediate state would either deadlock or try to release a lock that we don't even hold.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231115172012.112727-3-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [kwolf: Fixed up bdrv_schedule_unref()] Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
ccd6a379 |
| 27-Oct-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_replace_node() GRAPH_WRLOCK
Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_replace_node(). Its callers may already want to hold t
block: Mark bdrv_replace_node() GRAPH_WRLOCK
Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_replace_node(). Its callers may already want to hold the graph lock and so wouldn't be able to call functions that take it internally.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-17-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
79bb7627 |
| 27-Oct-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_chain_contains() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_chain_contains() need to hold a reader lock for the graph because it cal
block: Mark bdrv_chain_contains() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_chain_contains() need to hold a reader lock for the graph because it calls bdrv_filter_or_cow_bs(), which accesses bs->file/backing.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-11-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
ad74751f |
| 27-Oct-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_skip_filters() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_skip_filters() need to hold a reader lock for the graph because it calls b
block: Mark bdrv_skip_filters() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_skip_filters() need to hold a reader lock for the graph because it calls bdrv_filter_child(), which accesses bs->file/backing.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-9-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
430da832 |
| 27-Oct-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_skip_implicit_filters() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_skip_implicit_filters() need to hold a reader lock for the graph
block: Mark bdrv_skip_implicit_filters() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_skip_implicit_filters() need to hold a reader lock for the graph because it calls bdrv_filter_child(), which accesses bs->file/backing.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-8-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
372b69f5 |
| 27-Oct-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_filter_or_cow_bs() need to hold a reader lock for the graph because it
block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_filter_or_cow_bs() need to hold a reader lock for the graph because it calls bdrv_filter_or_cow_child(), which accesses bs->file/backing.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-7-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
06717986 |
| 27-Oct-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_has_zero_init() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_has_zero_init() need to hold a reader lock for the graph because it calls
block: Mark bdrv_has_zero_init() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_has_zero_init() need to hold a reader lock for the graph because it calls bdrv_filter_bs(), which accesses bs->file/backing.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-3-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
d3256f88 |
| 16-Oct-2023 |
David Woodhouse <dwmw@amazon.co.uk> |
hw/xen: automatically assign device index to block devices
There's no need to force the user to assign a vdev. We can automatically assign one, starting at xvda and searching until we find the first
hw/xen: automatically assign device index to block devices
There's no need to force the user to assign a vdev. We can automatically assign one, starting at xvda and searching until we find the first disk name that's unused.
This means we can now allow '-drive if=xen,file=xxx' to work without an explicit separate -driver argument, just like if=virtio.
Rip out the legacy handling from the xenpv machine, which was scribbling over any disks configured by the toolstack, and didn't work with anything but raw images.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> Acked-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Paul Durrant <paul@xen.org>
show more ...
|
#
61a3a5a7 |
| 31-Oct-2023 |
Fiona Ebner <f.ebner@proxmox.com> |
blockjob: introduce block-job-change QMP command
which will allow changing job-type-specific options after job creation.
In the JobVerbTable, the same allow bits as for set-speed are used, because
blockjob: introduce block-job-change QMP command
which will allow changing job-type-specific options after job creation.
In the JobVerbTable, the same allow bits as for set-speed are used, because set-speed can be considered an existing change command.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-ID: <20231031135431.393137-2-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
30282385 |
| 19-Oct-2023 |
Fiona Ebner <f.ebner@proxmox.com> |
blockdev: mirror: avoid potential deadlock when using iothread
The bdrv_getlength() function is a generated co-wrapper and uses AIO_WAIT_WHILE() to wait for the spawned coroutine. AIO_WAIT_WHILE() e
blockdev: mirror: avoid potential deadlock when using iothread
The bdrv_getlength() function is a generated co-wrapper and uses AIO_WAIT_WHILE() to wait for the spawned coroutine. AIO_WAIT_WHILE() expects the lock to be acquired exactly once.
Fix a case where it may be acquired twice. This can happen when the source node is explicitly specified as the @replaces parameter or if the source node is a filter node.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Message-ID: <20231019131936.414246-4-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
277f2007 |
| 29-Sep-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_op_is_blocked() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_op_is_blocked() need to hold a reader lock for the graph because it calls
block: Mark bdrv_op_is_blocked() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_op_is_blocked() need to hold a reader lock for the graph because it calls bdrv_get_device_or_node_name(), which accesses the parents list of a node.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-18-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
b7cfc7d5 |
| 29-Sep-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_refresh_filename() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_refresh_filename() need to hold a reader lock for the graph because it
block: Mark bdrv_refresh_filename() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_refresh_filename() need to hold a reader lock for the graph because it accesses the children list of a node.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-11-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
15f3f1fe |
| 29-Sep-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_get_xdbg_block_graph() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_get_xdbg_block_graph() need to hold a reader lock for the graph be
block: Mark bdrv_get_xdbg_block_graph() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_get_xdbg_block_graph() need to hold a reader lock for the graph because it accesses the children list of a node.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-10-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
a32e7818 |
| 29-Sep-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_snapshot_fallback() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_snapshot_fallback() need to hold a reader lock for the graph because
block: Mark bdrv_snapshot_fallback() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_snapshot_fallback() need to hold a reader lock for the graph because it accesses the children list of a node.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-8-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
2b3912f1 |
| 29-Sep-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_first_blk() and bdrv_is_root_node() GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_first_blk() and bdrv_is_root_node() need to hold a reader lock fo
block: Mark bdrv_first_blk() and bdrv_is_root_node() GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_first_blk() and bdrv_is_root_node() need to hold a reader lock for the graph. These functions are the only functions in block-backend.c that access the parent list of a node.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-5-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
9def6082 |
| 11-Sep-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_add/del_child() and caller GRAPH_WRLOCK
The functions read the parents list in the generic block layer, so we need to hold the graph lock already there. The BlockDriver implementati
block: Mark bdrv_add/del_child() and caller GRAPH_WRLOCK
The functions read the parents list in the generic block layer, so we need to hold the graph lock already there. The BlockDriver implementations actually modify the graph, so it has to be a writer lock.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230911094620.45040-22-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
bce73bc2 |
| 11-Sep-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_get_cumulative_perm() and callers GRAPH_RDLOCK
The function reads the parents list, so it needs to hold the graph lock.
This happens to result in BlockDriver.bdrv_set_perm() to be
block: Mark bdrv_get_cumulative_perm() and callers GRAPH_RDLOCK
The function reads the parents list, so it needs to hold the graph lock.
This happens to result in BlockDriver.bdrv_set_perm() to be called with the graph lock held. For consistency, make it the same for all of the BlockDriver callbacks for updating permissions and annotate the function pointers with GRAPH_RDLOCK_PTR.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230911094620.45040-15-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
bd1386cc |
| 22-May-2023 |
Eric Blake <eblake@redhat.com> |
cutils: Adjust signature of parse_uint[_full]
It's already confusing that we have two very similar functions for wrapping the parse of a 64-bit unsigned value, differing mainly on whether they permi
cutils: Adjust signature of parse_uint[_full]
It's already confusing that we have two very similar functions for wrapping the parse of a 64-bit unsigned value, differing mainly on whether they permit leading '-'. Adjust the signature of parse_uint() and parse_uint_full() to be like all of qemu_strto*(): put the result parameter last, use the same types (uint64_t and unsigned long long have the same width, but are not always the same type), and mark endptr const (this latter change only affects the rare caller of parse_uint). Adjust all callers in the tree.
While at it, note that since cutils.c already includes:
QEMU_BUILD_BUG_ON(sizeof(int64_t) != sizeof(long long));
we are guaranteed that the result of parse_uint* cannot exceed UINT64_MAX (or the build would have failed), so we can drop pre-existing dead comparisons in opts-visitor.c that were never false.
Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-8-eblake@redhat.com> [eblake: Drop dead code spotted by Markus] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
#
c6e0a6de |
| 25-May-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Take main AioContext lock when calling bdrv_open()
The function documentation already says that all callers must hold the main AioContext lock, but not all of them do. This can cause assertio
block: Take main AioContext lock when calling bdrv_open()
The function documentation already says that all callers must hold the main AioContext lock, but not all of them do. This can cause assertion failures when functions called by bdrv_open() try to drop the lock. Fix a few more callers to take the lock before calling bdrv_open().
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230525124713.401149-4-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
d53c89ae |
| 10-May-2023 |
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
blockdev: qmp_transaction: drop extra generic layer
Let's simplify things:
First, actions generally don't need access to common BlkActionState structure. The only exclusion are backup actions that
blockdev: qmp_transaction: drop extra generic layer
Let's simplify things:
First, actions generally don't need access to common BlkActionState structure. The only exclusion are backup actions that need block_job_txn.
Next, for transaction actions of Transaction API is more native to allocated state structure in the action itself.
So, do the following transformation:
1. Let all actions be represented by a function with corresponding structure as arguments.
2. Instead of array-map marshaller, let's make a function, that calls corresponding action directly.
3. BlkActionOps and BlkActionState structures become unused
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20230510150624.310640-7-vsementsov@yandex-team.ru> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
c85feafa |
| 10-May-2023 |
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
blockdev: use state.bitmap in block-dirty-bitmap-add action
Other bitmap related actions use the .bitmap pointer in .abort action, let's do same here:
1. It helps further refactoring, as bitmap-add
blockdev: use state.bitmap in block-dirty-bitmap-add action
Other bitmap related actions use the .bitmap pointer in .abort action, let's do same here:
1. It helps further refactoring, as bitmap-add is the only bitmap action that uses state.action in .abort
2. It must be safe: transaction actions rely on the fact that on .abort() the state is the same as at the end of .prepare(), so that in .abort() we could precisely rollback the changes done by .prepare(). The only way to remove the bitmap during transaction should be block-dirty-bitmap-remove action, but it postpones actual removal to .commit(), so we are OK on any rollback path. (Note also that bitmap-remove is the only bitmap action that has .commit() phase, except for simple g_free the state on .clean())
3. Again, other bitmap actions behave this way: keep the bitmap pointer during the transaction.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20230510150624.310640-6-vsementsov@yandex-team.ru> [kwolf: Also remove the now unused BlockDirtyBitmapState.prepared] Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
c85f34cf |
| 10-May-2023 |
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
blockdev: transaction: refactor handling transaction properties
Only backup supports GROUPED mode. Make this logic more clear. And avoid passing extra thing to each action.
Signed-off-by: Vladimir
blockdev: transaction: refactor handling transaction properties
Only backup supports GROUPED mode. Make this logic more clear. And avoid passing extra thing to each action.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20230510150624.310640-5-vsementsov@yandex-team.ru> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
30c96b55 |
| 10-May-2023 |
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
blockdev: qmp_transaction: refactor loop to classic for
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230510150624
blockdev: qmp_transaction: refactor loop to classic for
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230510150624.310640-4-vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
24039696 |
| 10-May-2023 |
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
blockdev: transactions: rename some things
Look at qmp_transaction(): dev_list is not obvious name for list of actions. Let's look at qapi spec, this argument is "actions". Let's follow the common p
blockdev: transactions: rename some things
Look at qmp_transaction(): dev_list is not obvious name for list of actions. Let's look at qapi spec, this argument is "actions". Let's follow the common practice of using same argument names in qapi scheme and code.
To be honest, rename props to properties for same reason.
Next, we have to rename global map of actions, to not conflict with new name for function argument.
Rename also dev_entry loop variable accordingly to new name of the list.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230510150624.310640-3-vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|