aboutsummaryrefslogtreecommitdiff
path: root/block.c
AgeCommit message (Collapse)AuthorFilesLines
2023-10-12block: Protect bs->children with graph_lockKevin Wolf1-0/+2
Almost all functions that access the child links already take the graph lock now. Add locking to the remaining users and finally annotate the struct field itself as protected by the graph lock. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-22-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-10-12block: Mark bdrv_apply_auto_read_only() and callers GRAPH_RDLOCKKevin Wolf1-2/+2
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_apply_auto_read_only() need to hold a reader lock for the graph because it calls bdrv_can_set_read_only(), which indirectly accesses the parents list of a node. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-19-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-10-12block: Mark bdrv_op_is_blocked() and callers GRAPH_RDLOCKKevin Wolf1-2/+0
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>
2023-10-12qcow2: Mark qcow2_signal_corruption() and callers GRAPH_RDLOCKKevin Wolf1-1/+2
This adds GRAPH_RDLOCK annotations to declare that callers of qcow2_signal_corruption() need to hold a reader lock for the graph because it calls bdrv_get_node_name(), which accesses the parents list of a node. For some places, we know that they will hold the lock, but we don't have the GRAPH_RDLOCK annotations yet. In this case, add assume_graph_lock() with a FIXME comment. These places will be removed once everything is properly annotated. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-15-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-10-12block: Mark bdrv_get_parent_name() and callers GRAPH_RDLOCKKevin Wolf1-2/+12
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_get_parent_name() need to hold a reader lock for the graph because it accesses the parents list of a node. For some places, we know that they will hold the lock, but we don't have the GRAPH_RDLOCK annotations yet. In this case, add assume_graph_lock() with a FIXME comment. These places will be removed once everything is properly annotated. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-13-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-10-12block: Mark bdrv_primary_child() and callers GRAPH_RDLOCKKevin Wolf1-1/+10
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_primary_child() 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-12-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-10-12block: Mark bdrv_refresh_filename() and callers GRAPH_RDLOCKKevin Wolf1-5/+18
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>
2023-10-12block: Take graph rdlock in parts of reopenKevin Wolf1-22/+35
Reopen isn't easy with respect to locking because many of its functions need to iterate the graph, some change it, and then you get some drains in the middle where you can't hold any locks. Therefore just documents most of the functions to be unlocked, and take locks internally before accessing the graph. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-9-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-10-12block: Mark drain related functions GRAPH_RDLOCKEmanuele Giuseppe Esposito1-3/+3
Draining recursively traverses the graph, therefore we need to make sure that also such accesses to the graph are protected by the graph rdlock. There are 3 different drain callers to consider: 1. drain in the main loop: no issue at all, rdlock is nop. 2. drain in an iothread: rdlock only works in main loop or coroutines, so disallow it. 3. drain in a coroutine (regardless of AioContext): the drain mechanism takes care of scheduling a BH in the bs->aio_context that will then take care of perform the actual draining. This is wrong, because as pointed in (2) if bs->aio_context is an iothread then rdlock won't work. Therefore change bdrv_co_yield_to_drain to schedule the BH in the main loop. Caller (2) also implies that we need to modify test-bdrv-drain.c to disallow draining in the iothreads. For some places, we know that they will hold the lock, but we don't have the GRAPH_RDLOCK annotations yet. In this case, add assume_graph_lock() with a FIXME comment. These places will be removed once everything is properly annotated. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-6-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-10-12block: Mark bdrv_first_blk() and bdrv_is_root_node() GRAPH_RDLOCKKevin Wolf1-0/+1
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>
2023-10-12block: Take graph rdlock in bdrv_inactivate_all()Kevin Wolf1-3/+4
The function reads the parents list, so it needs to hold the graph lock. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-4-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-29block: Clean up local variable shadowingMarkus Armbruster1-4/+5
Local variables shadowing other local variables or parameters make the code needlessly hard to understand. Tracked down with -Wshadow=local. Clean up: delete inner declarations when they are actually redundant, else rename variables. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Ilya Dryomov <idryomov@gmail.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230921121312.1301864-7-armbru@redhat.com>
2023-09-20block: Mark bdrv_unref_child() GRAPH_WRLOCKKevin Wolf1-4/+7
Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_unref_child(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. 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-21-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Mark bdrv_root_unref_child() GRAPH_WRLOCKKevin Wolf1-3/+3
Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_root_unref_child(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. 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-20-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Take graph rdlock in bdrv_change_aio_context()Kevin Wolf1-0/+4
The function reads the parents list, so it needs to hold the graph lock. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Message-ID: <20230911094620.45040-19-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Take graph rdlock in bdrv_drop_intermediate()Kevin Wolf1-0/+2
The function reads the parents list, so it needs to hold the graph lock. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Message-ID: <20230911094620.45040-18-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Mark bdrv_parent_cb_change_media() GRAPH_RDLOCKKevin Wolf1-1/+5
The function reads the parents list, so it needs to hold the graph 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-17-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Mark bdrv_child_perm() GRAPH_RDLOCKKevin Wolf1-5/+6
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_child_perm() need to hold a reader lock for the graph because some implementations access the children list of a node. The callers of bdrv_child_perm() conveniently already hold the 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-16-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Mark bdrv_get_cumulative_perm() and callers GRAPH_RDLOCKKevin Wolf1-8/+27
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>
2023-09-20block: Mark bdrv_parent_perms_conflict() and callers GRAPH_RDLOCKKevin Wolf1-10/+18
The function reads the parents list, so it needs to hold the graph 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-14-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Mark bdrv_attach_child() GRAPH_WRLOCKKevin Wolf1-4/+3
Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_attach_child_common(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. 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-13-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Call transaction callbacks with lock heldKevin Wolf1-17/+44
In previous patches, we changed some transactionable functions to be marked as GRAPH_WRLOCK, but required that tran_finalize() is still called without the lock. This was because all callbacks that can be in the same transaction need to follow the same convention. Now that we don't have conflicting requirements any more, we can switch all of the transaction callbacks to be declared GRAPH_WRLOCK, too, and call tran_finalize() with the lock held. Document for each of these transactionable functions that the lock needs to be held when completing the transaction, and make sure that all callers down to the place where the transaction is finalised actually have the writer lock. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230911094620.45040-12-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Mark bdrv_attach_child_common() GRAPH_WRLOCKKevin Wolf1-48/+85
Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_attach_child_common(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. Note that the transaction callbacks still take the lock internally, so tran_finalize() must be called without the lock held. This is because bdrv_append() also calls bdrv_replace_node_noperm(), which currently requires the transaction callbacks to be called unlocked. In the next step, both of them can be switched to locked tran_finalize() calls together. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230911094620.45040-11-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Mark bdrv_replace_child_tran() GRAPH_WRLOCKKevin Wolf1-28/+50
Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_replace_child_tran(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. While a graph lock is held, polling is not allowed. Therefore draining the necessary nodes can no longer be done in bdrv_remove_child() and bdrv_replace_node_noperm(), but the callers must already make sure that they are drained. Note that the transaction callbacks still take the lock internally, so tran_finalize() must be called without the lock held. This is because bdrv_append() also calls bdrv_attach_child_noperm(), which currently requires to be called unlocked. Once it changes, the transaction callbacks can be changed, too. 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-10-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Mark bdrv_replace_child_noperm() GRAPH_WRLOCKKevin Wolf1-7/+19
Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_replace_child_noperm(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. 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-9-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Introduce bdrv_schedule_unref()Kevin Wolf1-0/+17
bdrv_unref() is called by a lot of places that need to hold the graph lock (it naturally happens in the context of operations that change the graph). However, bdrv_unref() takes the graph writer lock internally, so it can't actually be called while already holding a graph lock without causing a deadlock. bdrv_unref() also can't just become GRAPH_WRLOCK because it drains the node before closing it, and draining requires that the graph is unlocked. The solution is to defer deleting the node until we don't hold the lock any more and draining is possible again. Note that keeping images open for longer than necessary can create problems, too: You can't open an image again before it is really closed (if image locking didn't prevent it, it would cause corruption). Reopening an image immediately happens at least during bdrv_open() and bdrv_co_create(). In order to solve this problem, make sure to run the deferred unref in bdrv_graph_wrunlock(), i.e. the first possible place where we can drain again. This is also why bdrv_schedule_unref() is marked GRAPH_WRLOCK. The output of iotest 051 is updated because the additional polling changes the order of HMP output, resulting in a new "(qemu)" prompt in the test output that was previously on a separate line and filtered out. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230911094620.45040-6-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-20block: Remove unused BlockReopenQueueEntry.perms_checkedKevin Wolf1-1/+0
This field has been unused since commit 72373e40fbc ('block: bdrv_reopen_multiple: refresh permissions on updated graph'). Remove it. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230911094620.45040-2-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-11Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into stagingStefan Hajnoczi1-3/+7
Block layer patches - Optimise reqs_lock to make multiqueue actually scale - virtio: Drop out of coroutine context in virtio_load() - iotests: Fix reference output for some tests after recent changes - vpc: Avoid dynamic stack allocation - Code cleanup, improved documentation # -----BEGIN PGP SIGNATURE----- # # iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmT7VYgRHGt3b2xmQHJl # ZGhhdC5jb20ACgkQfwmycsiPL9YfOg/7BoYF6lkB7DF/jH3XLY6f8zoI+OVM7dg1 # QFEjyVO+uZiJVh0CeBNI9WgnBe7f5vXMbiStyGbWKo3BLUsjnwoQcW/Sxpw61bR2 # jZYK6UHe0RhFqTQpbt8G1iCmlpRS+sX+Cy+lxcVcbqxcnLRXCOjT6ivyA4bGbYIC # q9BHg/9hBmjuM05NTV6Axy8qjqBGVaIWE9ALTnw8H//waBr4/ydJPTl7EWHe3+tO # Stm73evgPG7aLHM6W4qdFW4gwAQ8f+f42Q+0NH1YavB/pN3LTN1B6sLQY/51du+0 # d/JCsXex0IZQXmNPhqv1h01vhOyU9WBmlwpPG2iZv3a06SXk1ys3rQt/L7uIcsZg # Z58CpcUJ517FERnkl0BWXzYhsdcW2K+RdlaiL5PX6H1A2B9LT05ouZfD47hh7kKv # oX+Ulk05PFr3JRCKQF6QDEejRKXt169bGzInTlns/wXinD/V4sCkUnr9aWQuhoWk # KhQm7WMscTTIyHP2FznO4x9kq0ALsoX/NKqBW2wgJUtqRzsd4XxPp5CXEsAir8Vt # dpne/DaV5iDI1mGFJrvkctJN545tEoezBtUzC8/9rZGE0cxHAkhvQVZUDo7xVmrq # PlGQ1ko9cNui/Gf9B6qDqaJJwSyw0S6vHurGVQJRwbyly57Fi5aisWkr4w7Rc4eA # 7u9B1RvwF/Q= # =2wGD # -----END PGP SIGNATURE----- # gpg: Signature made Fri 08 Sep 2023 13:10:32 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: virtio: Drop out of coroutine context in virtio_load() vmstate: Mark VMStateInfo.get/put() coroutine_mixed_fn block: Make more BlockDriver definitions static block/meson.build: Restore alphabetical order of files block: Remove unnecessary variable in bdrv_block_device_info block: Remove bdrv_query_block_node_info vmdk: Clean up bdrv_open_child() return value check qemu-img: Update documentation for compressed images block: Be more verbose in create fallback block/iscsi: Document why we use raw malloc() qemu-img: omit errno value in error message block: change reqs_lock to QemuMutex block: minimize bs->reqs_lock section in tracked_request_end() iotests: adapt test output for new qemu_cleanup() behavior block/vpc: Avoid dynamic stack allocation Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2023-09-08block: Be more verbose in create fallbackHanna Czenczek1-2/+4
For image creation code, we have central fallback code for protocols that do not support creating new images (like NBD or iscsi). So for them, you can only specify existing paths/exports that are overwritten to make clean new images. In such a case, if the given path cannot be opened (assuming a pre-existing image there), we print an error message that tries to describe what is going on: That with this protocol, you cannot create new images, but only overwrite existing ones; and the given path could not be opened as a pre-existing image. However, the current message is confusing, because it does not say that the protocol in question does not support creating new images, but instead that "image creation" is unsupported. This can be interpreted to mean that `qemu-img create` will not work in principle, which is not true. Be more verbose for clarity. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2217204 Signed-off-by: Hanna Czenczek <hreitz@redhat.com> Message-ID: <20230720140024.46836-1-hreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-08block: change reqs_lock to QemuMutexStefan Hajnoczi1-1/+3
CoMutex has poor performance when lock contention is high. The tracked requests list is accessed frequently and performance suffers in QEMU multi-queue block layer scenarios. It is not necessary to use CoMutex for the requests lock. The lock is always released across coroutine yield operations. It is held for relatively short periods of time and it is not beneficial to yield when the lock is held by another coroutine. Change the lock type from CoMutex to QemuMutex to improve multi-queue block layer performance. fio randread bs=4k iodepth=64 with 4 IOThreads handling a virtio-blk device with 8 virtqueues improves from 254k to 517k IOPS (+203%). Full benchmark results and configuration details are available here: https://gitlab.com/stefanha/virt-playbooks/-/commit/980c40845d540e3669add1528739503c2e817b57 In the future we may wish to introduce thread-local tracked requests lists to avoid lock contention completely. That would be much more involved though. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230808155852.2745350-3-stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-08block: spelling fixesMichael Tokarev1-1/+1
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Reviewed-by: Eric Blake <eblake@redhat.com>
2023-08-30block: add subcluster_size field to BlockDriverInfoAndrey Drobyshev1-0/+7
This is going to be used in the subsequent commit as requests alignment (in particular, during copy-on-read). This value only makes sense for the formats which support subclusters (currently QCOW2 only). If this field isn't set by driver's own bdrv_get_info() implementation, we simply set it equal to the cluster size thus treating each cluster as having a single subcluster. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230711172553.234055-2-andrey.drobyshev@virtuozzo.com>
2023-06-28block: mark another function as coroutine_fns and GRAPH_UNLOCKEDPaolo Bonzini1-5/+6
Mark functions as coroutine_fn when they are only called by other coroutine_fns and they can suspend. Because this function operates on a BlockBackend, mark it GRAPH_UNLOCKED. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-ID: <20230601115145.196465-6-pbonzini@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-06-28graph-lock: Unlock the AioContext while pollingKevin Wolf1-2/+2
If the caller keeps the AioContext lock for a block node in an iothread, polling in bdrv_graph_wrlock() deadlocks if the condition isn't fulfilled immediately. Now that all callers make sure to actually have the AioContext locked when they call bdrv_replace_child_noperm() like they should, we can change bdrv_graph_wrlock() to take a BlockDriverState whose AioContext lock the caller holds (NULL if it doesn't) and unlock it temporarily while polling. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230605085711.21261-11-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-06-28block: Fix AioContext locking in bdrv_open_backing_file()Kevin Wolf1-0/+5
bdrv_set_backing() requires the caller to hold the AioContext lock for @backing_hd. Take it in bdrv_open_backing_file() before calling the function. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230605085711.21261-9-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-06-28block: Fix AioContext locking in bdrv_open_inherit()Kevin Wolf1-1/+24
bdrv_open_inherit() calls several functions for which it needs to hold the AioContext lock, but currently doesn't. This includes calls in bdrv_append_temp_snapshot(), for which bdrv_open_inherit() is the only caller. Fix the locking in these places. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230605085711.21261-8-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-06-28block: Fix AioContext locking in bdrv_reopen_parse_file_or_backing()Kevin Wolf1-2/+33
bdrv_set_file_or_backing_noperm() requires the caller to hold the AioContext lock for the child node, but we hold the one for the parent node in bdrv_reopen_parse_file_or_backing(). Take the other one temporarily. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230605085711.21261-7-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-06-28block: Fix AioContext locking in bdrv_attach_child_common()Kevin Wolf1-1/+20
The function can move the child node to a different AioContext. In this case, it also must take the AioContext lock for the new context before calling functions that require the caller to hold the AioContext for the child node. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230605085711.21261-6-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-06-28block: Fix AioContext locking in bdrv_open_child()Kevin Wolf1-2/+11
bdrv_attach_child() requires that the caller holds the AioContext lock for the new child node. Take it in bdrv_open_child() and document that the caller must not hold any AioContext apart from the main AioContext. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230605085711.21261-5-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-06-09block.c: add newline for "Detected format" warningMichael Tokarev1-1/+1
Add the forgotten trailing newline. Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
2023-05-30aio: remove aio_disable_external() APIStefan Hajnoczi1-7/+0
All callers now pass is_external=false to aio_set_fd_handler() and aio_set_event_notifier(). The aio_disable_external() API that temporarily disables fd handlers that were registered is_external=true is therefore dead code. Remove aio_disable_external(), aio_enable_external(), and the is_external arguments to aio_set_fd_handler() and aio_set_event_notifier(). The entire test-fdmon-epoll test is removed because its sole purpose was testing aio_disable_external(). Parts of this patch were generated using the following coccinelle (https://coccinelle.lip6.fr/) semantic patch: @@ expression ctx, fd, is_external, io_read, io_write, io_poll, io_poll_ready, opaque; @@ - aio_set_fd_handler(ctx, fd, is_external, io_read, io_write, io_poll, io_poll_ready, opaque) + aio_set_fd_handler(ctx, fd, io_read, io_write, io_poll, io_poll_ready, opaque) @@ expression ctx, notifier, is_external, io_read, io_poll, io_poll_ready; @@ - aio_set_event_notifier(ctx, notifier, is_external, io_read, io_poll, io_poll_ready) + aio_set_event_notifier(ctx, notifier, io_read, io_poll, io_poll_ready) Reviewed-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20230516190238.8401-21-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-05-30block: Fix AioContext locking in bdrv_insert_node()Kevin Wolf1-0/+11
While calling bdrv_new_open_driver_opts(), the main AioContext lock must be held, not the lock of the AioContext of the block subtree it will be added to afterwards. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230525124713.401149-11-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-05-30block: Take AioContext lock in bdrv_open_driver()Kevin Wolf1-0/+7
bdrv_refresh_total_sectors() and bdrv_refresh_limits() expect to be called under the AioContext lock of the node. Take the lock. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230525124713.401149-10-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-05-30qcow2: Fix open with 'file' in iothreadKevin Wolf1-0/+6
qcow2_open() doesn't work correctly when opening the 'file' child moves bs to an iothread, for several reasons: - It uses BDRV_POLL_WHILE() to wait for the qcow2_open_entry() coroutine, which involves dropping the AioContext lock for bs when it is not in the main context - but we don't hold it, so this crashes. - It runs the qcow2_open_entry() coroutine in the current thread instead of the new AioContext of bs. - qcow2_open_entry() doesn't notify the main loop when it's done. This patches fixes these issues around delegating work to a coroutine. Temporarily dropping the main AioContext lock is not necessary because we know we run in the main thread. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230525124713.401149-7-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-05-30mirror: Hold main AioContext lock for calling bdrv_open_backing_file()Kevin Wolf1-0/+2
bdrv_open_backing_file() calls bdrv_open_inherit(), so all callers must hold the main AioContext lock. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230525124713.401149-6-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-05-30block: Take main AioContext lock when calling bdrv_open()Kevin Wolf1-0/+3
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>
2023-05-30block: Clarify locking rules for bdrv_open(_inherit)()Kevin Wolf1-8/+2
These functions specify that the caller must hold the "@filename AioContext lock". This doesn't make sense, file names don't have an AioContext. New BlockDriverStates always start in the main AioContext, so this is what we really need here. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230525124713.401149-3-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-05-19block: Call .bdrv_co_create(_opts) unlockedKevin Wolf1-1/+0
These are functions that modify the graph, so they must be able to take a writer lock. This is impossible if they already hold the reader lock. If they need a reader lock for some of their operations, they should take it internally. Many of them go through blk_*(), which will always take the lock itself. Direct calls of bdrv_*() need to take the reader lock. Note that while locking for bdrv_co_*() calls is checked by TSA, this is not the case for the mixed_coroutine_fns bdrv_*(). Holding the lock is still required when they are called from coroutine context like here! This effectively reverts 4ec8df0183, but adds some internal locking instead. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230510203601.418015-2-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-05-15block: add zoned BlockDriver check to block layerSam Li1-0/+19
Putting zoned/non-zoned BlockDrivers on top of each other is not allowed. Signed-off-by: Sam Li <faithilikerun@gmail.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> Acked-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20230508045533.175575-6-faithilikerun@gmail.com Message-id: 20230324090605.28361-6-faithilikerun@gmail.com [Adjust commit message prefix as suggested by Philippe Mathieu-Daudé <philmd@linaro.org> and clarify that the check is about zoned BlockDrivers. --Stefan] Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2023-05-10block: Mark bdrv_refresh_limits() and callers GRAPH_RDLOCKKevin Wolf1-0/+9
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_refresh_limits() 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> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20230504115750.54437-21-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
generated by cgit v1.1 at 2024-11-08 06:24:22 +0000