[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-commits] [qemu/qemu] dea97c: block-coroutine-wrapper: Take AioCont
|
From: |
Richard Henderson |
|
Subject: |
[Qemu-commits] [qemu/qemu] dea97c: block-coroutine-wrapper: Take AioContext lock in n... |
|
Date: |
Tue, 30 May 2023 09:54:27 -0700 |
Branch: refs/heads/staging
Home: https://github.com/qemu/qemu
Commit: dea97c1fbd4e1bc36d776ff949ce568b7a435e71
https://github.com/qemu/qemu/commit/dea97c1fbd4e1bc36d776ff949ce568b7a435e71
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/block-backend.c
M include/block/block-common.h
M scripts/block-coroutine-wrapper.py
Log Message:
-----------
block-coroutine-wrapper: Take AioContext lock in no_co_wrappers
All of the functions that currently take a BlockDriverState, BdrvChild
or BlockBackend as their first parameter expect the associated
AioContext to be locked when they are called. In the case of
no_co_wrappers, they are called from bottom halves directly in the main
loop, so no other caller can be expected to take the lock for them. This
can result in assertion failures because a lock that isn't taken is
released in nested event loops.
Looking at the first parameter is already done by co_wrappers to decide
where the coroutine should run, so doing the same in no_co_wrappers is
only consistent. Take the lock in the generated bottom halves to fix the
problem.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230525124713.401149-2-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: ae400dbb8f439021bca5b8a7f60907bbd2cf5291
https://github.com/qemu/qemu/commit/ae400dbb8f439021bca5b8a7f60907bbd2cf5291
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block.c
Log Message:
-----------
block: Clarify locking rules for bdrv_open(_inherit)()
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>
Commit: c6e0a6de62c5fa99ef06a9bb49c8072bcf93f431
https://github.com/qemu/qemu/commit/c6e0a6de62c5fa99ef06a9bb49c8072bcf93f431
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block.c
M block/block-backend.c
M block/qapi-sysemu.c
M blockdev.c
M qemu-nbd.c
M tests/unit/test-block-iothread.c
Log Message:
-----------
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>
Commit: 4c20dd24b1178c78c47bf323173360b85e65e1e1
https://github.com/qemu/qemu/commit/4c20dd24b1178c78c47bf323173360b85e65e1e1
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/block-backend.c
Log Message:
-----------
block-backend: Fix blk_new_open() for iothreads
This fixes blk_new_open() to not assume that bs is in the main context.
In particular, the BlockBackend must be created with the right
AioContext because it will refuse to move to a different context
afterwards. (blk->allow_aio_context_change is false.)
Use this opportunity to use blk_insert_bs() instead of duplicating the
bdrv_root_attach_child() call. This is consistent with what
blk_new_with_bs() does. Add comments to document the locking rules.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230525124713.401149-5-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 2626d27f50f3c993936db04bc6e92d553e1dc914
https://github.com/qemu/qemu/commit/2626d27f50f3c993936db04bc6e92d553e1dc914
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block.c
M block/mirror.c
Log Message:
-----------
mirror: Hold main AioContext lock for calling bdrv_open_backing_file()
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>
Commit: aa269ff888d70158fe0c26ed17814046bdc19bd5
https://github.com/qemu/qemu/commit/aa269ff888d70158fe0c26ed17814046bdc19bd5
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block.c
M block/qcow2.c
Log Message:
-----------
qcow2: Fix open with 'file' in iothread
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>
Commit: 6e0121593288252d9e62448578368678eea3446c
https://github.com/qemu/qemu/commit/6e0121593288252d9e62448578368678eea3446c
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/raw-format.c
M tests/unit/test-block-iothread.c
Log Message:
-----------
raw-format: Fix open with 'file' in iothread
When opening the 'file' child moves bs to an iothread, we need to hold
the AioContext lock of it before we can call raw_apply_options() (and
more specifically, bdrv_getlength() inside of it).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230525124713.401149-8-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 9102f2ebdb71809af1c5ee4a4a255927e3dabca4
https://github.com/qemu/qemu/commit/9102f2ebdb71809af1c5ee4a4a255927e3dabca4
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/copy-before-write.c
Log Message:
-----------
copy-before-write: Fix open with child in iothread
The AioContext lock must not be held for bdrv_open_child(), but it is
necessary for the following operations, in particular those using nested
event loops in coroutine wrappers.
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-9-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 8dc8a60c9e52fa446a5135180592c0cd4213acfb
https://github.com/qemu/qemu/commit/8dc8a60c9e52fa446a5135180592c0cd4213acfb
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block.c
Log Message:
-----------
block: Take AioContext lock in bdrv_open_driver()
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>
Commit: 8823407c06a920eb2af217660094fdd8aff6c176
https://github.com/qemu/qemu/commit/8823407c06a920eb2af217660094fdd8aff6c176
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block.c
Log Message:
-----------
block: Fix AioContext locking in bdrv_insert_node()
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>
Commit: bc05c638e5063695ff85e274204f46e333d1a743
https://github.com/qemu/qemu/commit/bc05c638e5063695ff85e274204f46e333d1a743
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M tests/qemu-iotests/256
M tests/qemu-iotests/iotests.py
Log Message:
-----------
iotests: Make verify_virtio_scsi_pci_or_ccw() public
It has no internal callers, so its only use is being called from
individual test cases. If the name starts with an underscore, it is
considered private and linters warn against calling it. 256 only gets
away with it currently because it's on the exception list for linters.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230525124713.401149-12-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 80e7faaac6f363c9902fbab3f5cc463e4e3d933f
https://github.com/qemu/qemu/commit/80e7faaac6f363c9902fbab3f5cc463e4e3d933f
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
A tests/qemu-iotests/tests/iothreads-create
A tests/qemu-iotests/tests/iothreads-create.out
Log Message:
-----------
iotests: Test blockdev-create in iothread
If blockdev-create references an existing node in an iothread (e.g. as
it's 'file' child), then suddenly all of the image creation code must
run in that AioContext, too. Test that this actually works.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230525124713.401149-13-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 2d1962958177cb80a491e4767c41bf6d82dbbc83
https://github.com/qemu/qemu/commit/2d1962958177cb80a491e4767c41bf6d82dbbc83
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/block-backend.c
Log Message:
-----------
block-backend: split blk_do_set_aio_context()
blk_set_aio_context() is not fully transactional because
blk_do_set_aio_context() updates blk->ctx outside the transaction. Most
of the time this goes unnoticed but a BlockDevOps.drained_end() callback
that invokes blk_get_aio_context() fails assert(ctx == blk->ctx). This
happens because blk->ctx is only assigned after
BlockDevOps.drained_end() is called and we're in an intermediate state
where BlockDrvierState nodes already have the new context and the
BlockBackend still has the old context.
Making blk_set_aio_context() fully transactional solves this assertion
failure because the BlockBackend's context is updated as part of the
transaction (before BlockDevOps.drained_end() is called).
Split blk_do_set_aio_context() in order to solve this assertion failure.
This helper function actually serves two different purposes:
1. It drives blk_set_aio_context().
2. It responds to BdrvChildClass->change_aio_ctx().
Get rid of the helper function. Do #1 inside blk_set_aio_context() and
do #2 inside blk_root_set_aio_ctx_commit(). This simplifies the code.
The only drawback of the fully transactional approach is that
blk_set_aio_context() must contend with blk_root_set_aio_ctx_commit()
being invoked as part of the AioContext change propagation. This can be
solved by temporarily setting blk->allow_aio_context_change to true.
Future patches call blk_get_aio_context() from
BlockDevOps->drained_end(), so this patch will become necessary.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-2-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 26462a700c8c5d30802c2254a35b5064762e00f0
https://github.com/qemu/qemu/commit/26462a700c8c5d30802c2254a35b5064762e00f0
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M hw/scsi/scsi-bus.c
M include/hw/qdev-core.h
Log Message:
-----------
hw/qdev: introduce qdev_is_realized() helper
Add a helper function to check whether the device is realized without
requiring the Big QEMU Lock. The next patch adds a second caller. The
goal is to avoid spreading DeviceState field accesses throughout the
code.
Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-3-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 4382f167cfbc21a49424e6dd9347681008363128
https://github.com/qemu/qemu/commit/4382f167cfbc21a49424e6dd9347681008363128
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M hw/scsi/scsi-bus.c
M hw/scsi/virtio-scsi.c
Log Message:
-----------
virtio-scsi: avoid race between unplug and transport event
Only report a transport reset event to the guest after the SCSIDevice
has been unrealized by qdev_simple_device_unplug_cb().
qdev_simple_device_unplug_cb() sets the SCSIDevice's qdev.realized field
to false so that scsi_device_find/get() no longer see it.
scsi_target_emulate_report_luns() also needs to be updated to filter out
SCSIDevices that are unrealized.
Change virtio_scsi_push_event() to take event information as an argument
instead of the SCSIDevice. This allows virtio_scsi_hotunplug() to emit a
VIRTIO_SCSI_T_TRANSPORT_RESET event after the SCSIDevice has already
been unrealized.
These changes ensure that the guest driver does not see the SCSIDevice
that's being unplugged if it responds very quickly to the transport
reset event.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-4-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: ca66df878ef7fd6f5fdaedcebbbadcf12b60bc08
https://github.com/qemu/qemu/commit/ca66df878ef7fd6f5fdaedcebbbadcf12b60bc08
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M hw/scsi/virtio-scsi.c
Log Message:
-----------
virtio-scsi: stop using aio_disable_external() during unplug
This patch is part of an effort to remove the aio_disable_external()
API because it does not fit in a multi-queue block layer world where
many AioContexts may be submitting requests to the same disk.
The SCSI emulation code is already in good shape to stop using
aio_disable_external(). It was only used by commit 9c5aad84da1c
("virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi
disk") to ensure that virtio_scsi_hotunplug() works while the guest
driver is submitting I/O.
Ensure virtio_scsi_hotunplug() is safe as follows:
1. qdev_simple_device_unplug_cb() -> qdev_unrealize() ->
device_set_realized() calls qatomic_set(&dev->realized, false) so
that future scsi_device_get() calls return NULL because they exclude
SCSIDevices with realized=false.
That means virtio-scsi will reject new I/O requests to this
SCSIDevice with VIRTIO_SCSI_S_BAD_TARGET even while
virtio_scsi_hotunplug() is still executing. We are protected against
new requests!
2. scsi_qdev_unrealize() already contains a call to
scsi_device_purge_requests() so that in-flight requests are cancelled
synchronously. This ensures that no in-flight requests remain once
qdev_simple_device_unplug_cb() returns.
Thanks to these two conditions we don't need aio_disable_external()
anymore.
Cc: Zhengui Li <lizhengui@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-5-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 75d33e852536361367c8460abd8b04e3fe9921ee
https://github.com/qemu/qemu/commit/75d33e852536361367c8460abd8b04e3fe9921ee
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/export/vhost-user-blk-server.c
M include/qemu/vhost-user-server.h
M util/vhost-user-server.c
Log Message:
-----------
util/vhost-user-server: rename refcount to in_flight counter
The VuServer object has a refcount field and ref/unref APIs. The name is
confusing because it's actually an in-flight request counter instead of
a refcount.
Normally a refcount destroys the object upon reaching zero. The VuServer
counter is used to wake up the vhost-user coroutine when there are no
more requests.
Avoid confusing by renaming refcount and ref/unref to in_flight and
inc/dec.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-6-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 8f5e9a8ee189b44ffa90cc6db61e25499b9d786a
https://github.com/qemu/qemu/commit/8f5e9a8ee189b44ffa90cc6db61e25499b9d786a
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/export/vhost-user-blk-server.c
M include/qemu/vhost-user-server.h
M util/vhost-user-server.c
Log Message:
-----------
block/export: wait for vhost-user-blk requests when draining
Each vhost-user-blk request runs in a coroutine. When the BlockBackend
enters a drained section we need to enter a quiescent state. Currently
any in-flight requests race with bdrv_drained_begin() because it is
unaware of vhost-user-blk requests.
When blk_co_preadv/pwritev()/etc returns it wakes the
bdrv_drained_begin() thread but vhost-user-blk request processing has
not yet finished. The request coroutine continues executing while the
main loop thread thinks it is in a drained section.
One example where this is unsafe is for blk_set_aio_context() where
bdrv_drained_begin() is called before .aio_context_detached() and
.aio_context_attach(). If request coroutines are still running after
bdrv_drained_begin(), then the AioContext could change underneath them
and they race with new requests processed in the new AioContext. This
could lead to virtqueue corruption, for example.
(This example is theoretical, I came across this while reading the
code and have not tried to reproduce it.)
It's easy to make bdrv_drained_begin() wait for in-flight requests: add
a .drained_poll() callback that checks the VuServer's in-flight counter.
VuServer just needs an API that returns true when there are requests in
flight. The in-flight counter needs to be atomic.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-7-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: e1054cd4aad03a493a5d1cded7508f7c348205bf
https://github.com/qemu/qemu/commit/e1054cd4aad03a493a5d1cded7508f7c348205bf
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/export/vhost-user-blk-server.c
M util/vhost-user-server.c
Log Message:
-----------
block/export: stop using is_external in vhost-user-blk server
vhost-user activity must be suspended during bdrv_drained_begin/end().
This prevents new requests from interfering with whatever is happening
in the drained section.
Previously this was done using aio_set_fd_handler()'s is_external
argument. In a multi-queue block layer world the aio_disable_external()
API cannot be used since multiple AioContext may be processing I/O, not
just one.
Switch to BlockDevOps->drained_begin/end() callbacks.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-8-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 9998f70f6625f15f2ae36f612509ea34733c11d3
https://github.com/qemu/qemu/commit/9998f70f6625f15f2ae36f612509ea34733c11d3
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M hw/i386/kvm/xen_xenstore.c
Log Message:
-----------
hw/xen: do not use aio_set_fd_handler(is_external=true) in xen_xenstore
There is no need to suspend activity between aio_disable_external() and
aio_enable_external(), which is mainly used for the block layer's drain
operation.
This is part of ongoing work to remove the aio_disable_external() API.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Reviewed-by: Paul Durrant <paul@xen.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-9-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: ff82b7835b2fbbd0a17d616f6929601a97a6497d
https://github.com/qemu/qemu/commit/ff82b7835b2fbbd0a17d616f6929601a97a6497d
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/block-backend.c
M include/sysemu/block-backend-global-state.h
Log Message:
-----------
block: add blk_in_drain() API
The BlockBackend quiesce_counter is greater than zero during drained
sections. Add an API to check whether the BlockBackend is in a drained
section.
The next patch will use this API.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-10-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: ab61335025b1274bd7042219203524045b23e0d3
https://github.com/qemu/qemu/commit/ab61335025b1274bd7042219203524045b23e0d3
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/io.c
M include/block/block_int-common.h
M include/sysemu/block-backend-common.h
M tests/unit/test-bdrv-drain.c
Log Message:
-----------
block: drain from main loop thread in bdrv_co_yield_to_drain()
For simplicity, always run BlockDevOps .drained_begin/end/poll()
callbacks in the main loop thread. This makes it easier to implement the
callbacks and avoids extra locks.
Move the function pointer declarations from the I/O Code section to the
Global State section for BlockDevOps, BdrvChildClass, and BlockDriver.
Narrow IO_OR_GS_CODE() to GLOBAL_STATE_CODE() where appropriate.
The test-bdrv-drain test case calls bdrv_drain() from an IOThread. This
is now only allowed from coroutine context, so update the test case to
run in a coroutine.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-11-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: f6eac904f6825d47adc6102c8d7b59b8ba5b778e
https://github.com/qemu/qemu/commit/f6eac904f6825d47adc6102c8d7b59b8ba5b778e
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M hw/block/dataplane/xen-block.c
M hw/block/dataplane/xen-block.h
M hw/block/xen-block.c
M hw/xen/xen-bus.c
Log Message:
-----------
xen-block: implement BlockDevOps->drained_begin()
Detach event channels during drained sections to stop I/O submission
from the ring. xen-block is no longer reliant on aio_disable_external()
after this patch. This will allow us to remove the
aio_disable_external() API once all other code that relies on it is
converted.
Extend xen_device_set_event_channel_context() to allow ctx=NULL. The
event channel still exists but the event loop does not monitor the file
descriptor. Event channel processing can resume by calling
xen_device_set_event_channel_context() with a non-NULL ctx.
Factor out xen_device_set_event_channel_context() calls in
hw/block/dataplane/xen-block.c into attach/detach helper functions.
Incidentally, these don't require the AioContext lock because
aio_set_fd_handler() is thread-safe.
It's safer to register BlockDevOps after the dataplane instance has been
created. The BlockDevOps .drained_begin/end() callbacks depend on the
dataplane instance, so move the blk_set_dev_ops() call after
xen_block_dataplane_create().
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-12-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: fb5cba2c7ee34d3c44e87374f307ebec5673cfe0
https://github.com/qemu/qemu/commit/fb5cba2c7ee34d3c44e87374f307ebec5673cfe0
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M hw/xen/xen-bus.c
Log Message:
-----------
hw/xen: do not set is_external=true on evtchn fds
is_external=true suspends fd handlers between aio_disable_external() and
aio_enable_external(). The block layer's drain operation uses this
mechanism to prevent new I/O from sneaking in between
bdrv_drained_begin() and bdrv_drained_end().
The previous commit converted the xen-block device to use BlockDevOps
.drained_begin/end() callbacks. It no longer relies on is_external=true
so it is safe to pass is_external=false.
This is part of ongoing work to remove the aio_disable_external() API.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-13-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 195332c1d9b7e510097495bda2a038467a5869f7
https://github.com/qemu/qemu/commit/195332c1d9b7e510097495bda2a038467a5869f7
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/export/vduse-blk.c
Log Message:
-----------
block/export: rewrite vduse-blk drain code
vduse_blk_detach_ctx() waits for in-flight requests using
AIO_WAIT_WHILE(). This is not allowed according to a comment in
bdrv_set_aio_context_commit():
/*
* Take the old AioContex when detaching it from bs.
* At this point, new_context lock is already acquired, and we are now
* also taking old_context. This is safe as long as bdrv_detach_aio_context
* does not call AIO_POLL_WHILE().
*/
Use this opportunity to rewrite the drain code in vduse-blk:
- Use the BlockExport refcount so that vduse_blk_exp_delete() is only
called when there are no more requests in flight.
- Implement .drained_poll() so in-flight request coroutines are stopped
by the time .bdrv_detach_aio_context() is called.
- Remove AIO_WAIT_WHILE() from vduse_blk_detach_ctx() to solve the
.bdrv_detach_aio_context() constraint violation. It's no longer
needed due to the previous changes.
- Always handle the VDUSE file descriptor, even in drained sections. The
VDUSE file descriptor doesn't submit I/O, so it's safe to handle it in
drained sections. This ensures that the VDUSE kernel code gets a fast
response.
- Suspend virtqueue fd handlers in .drained_begin() and resume them in
.drained_end(). This eliminates the need for the
aio_set_fd_handler(is_external=true) flag, which is being removed from
QEMU.
This is a long list but splitting it into individual commits would
probably lead to git bisect failures - the changes are all related.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-14-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 3d499a43a25571dcacb3a25330357af5e0be9bca
https://github.com/qemu/qemu/commit/3d499a43a25571dcacb3a25330357af5e0be9bca
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/export/export.c
M block/export/vduse-blk.c
M include/block/export.h
Log Message:
-----------
block/export: don't require AioContext lock around blk_exp_ref/unref()
The FUSE export calls blk_exp_ref/unref() without the AioContext lock.
Instead of fixing the FUSE export, adjust blk_exp_ref/unref() so they
work without the AioContext lock. This way it's less error-prone.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-15-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 17b69c0fc1462be70419e622b63755a4390bfe31
https://github.com/qemu/qemu/commit/17b69c0fc1462be70419e622b63755a4390bfe31
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block/export/fuse.c
Log Message:
-----------
block/fuse: do not set is_external=true on FUSE fd
This is part of ongoing work to remove the aio_disable_external() API.
Use BlockDevOps .drained_begin/end/poll() instead of
aio_set_fd_handler(is_external=true).
As a side-effect the FUSE export now follows AioContext changes like the
other export types.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-16-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: bd58ab40c3fcfdd94f5524626ae13c43818bd23a
https://github.com/qemu/qemu/commit/bd58ab40c3fcfdd94f5524626ae13c43818bd23a
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M hw/block/dataplane/virtio-blk.c
M hw/scsi/virtio-scsi-dataplane.c
M hw/virtio/virtio.c
Log Message:
-----------
virtio: make it possible to detach host notifier from any thread
virtio_queue_aio_detach_host_notifier() does two things:
1. It removes the fd handler from the event loop.
2. It processes the virtqueue one last time.
The first step can be peformed by any thread and without taking the
AioContext lock.
The second step may need the AioContext lock (depending on the device
implementation) and runs in the thread where request processing takes
place. virtio-blk and virtio-scsi therefore call
virtio_queue_aio_detach_host_notifier() from a BH that is scheduled in
AioContext.
The next patch will introduce a .drained_begin() function that needs to
call virtio_queue_aio_detach_host_notifier(). .drained_begin() functions
cannot call aio_poll() to wait synchronously for the BH. It is possible
for a .drained_poll() callback to asynchronously wait for the BH, but
that is more complex than necessary here.
Move the virtqueue processing out to the callers of
virtio_queue_aio_detach_host_notifier() so that the function can be
called from any thread. This is in preparation for the next patch.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20230516190238.8401-17-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 1665d9326fd2dd97f1f4061decd67702956ec53c
https://github.com/qemu/qemu/commit/1665d9326fd2dd97f1f4061decd67702956ec53c
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M hw/block/dataplane/virtio-blk.c
M hw/block/virtio-blk.c
Log Message:
-----------
virtio-blk: implement BlockDevOps->drained_begin()
Detach ioeventfds during drained sections to stop I/O submission from
the guest. virtio-blk is no longer reliant on aio_disable_external()
after this patch. This will allow us to remove the
aio_disable_external() API once all other code that relies on it is
converted.
Take extra care to avoid attaching/detaching ioeventfds if the data
plane is started/stopped during a drained section. This should be rare,
but maybe the mirror block job can trigger it.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20230516190238.8401-18-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 766aa2de0f29b657148e04599320d771c36fd126
https://github.com/qemu/qemu/commit/766aa2de0f29b657148e04599320d771c36fd126
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M hw/scsi/scsi-bus.c
M hw/scsi/scsi-disk.c
M hw/scsi/trace-events
M hw/scsi/virtio-scsi-dataplane.c
M hw/scsi/virtio-scsi.c
M include/hw/scsi/scsi.h
Log Message:
-----------
virtio-scsi: implement BlockDevOps->drained_begin()
The virtio-scsi Host Bus Adapter provides access to devices on a SCSI
bus. Those SCSI devices typically have a BlockBackend. When the
BlockBackend enters a drained section, the SCSI device must temporarily
stop submitting new I/O requests.
Implement this behavior by temporarily stopping virtio-scsi virtqueue
processing when one of the SCSI devices enters a drained section. The
new scsi_device_drained_begin() API allows scsi-disk to message the
virtio-scsi HBA.
scsi_device_drained_begin() uses a drain counter so that multiple SCSI
devices can have overlapping drained sections. The HBA only sees one
pair of .drained_begin/end() calls.
After this commit, virtio-scsi no longer depends on hw/virtio's
ioeventfd aio_set_event_notifier(is_external=true). This commit is a
step towards removing the aio_disable_external() API.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20230516190238.8401-19-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 03d7162a21e60d87cfa39a8d078c784487fa5f30
https://github.com/qemu/qemu/commit/03d7162a21e60d87cfa39a8d078c784487fa5f30
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M hw/virtio/virtio.c
Log Message:
-----------
virtio: do not set is_external=true on host notifiers
Host notifiers can now use is_external=false since virtio-blk and
virtio-scsi no longer rely on is_external=true for drained sections.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20230516190238.8401-20-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 60f782b6b78211c125970768be726c9f380dbd61
https://github.com/qemu/qemu/commit/60f782b6b78211c125970768be726c9f380dbd61
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block.c
M block/blkio.c
M block/curl.c
M block/export/fuse.c
M block/export/vduse-blk.c
M block/io.c
M block/io_uring.c
M block/iscsi.c
M block/linux-aio.c
M block/nfs.c
M block/nvme.c
M block/ssh.c
M block/win32-aio.c
M hw/i386/kvm/xen_xenstore.c
M hw/virtio/virtio.c
M hw/xen/xen-bus.c
M include/block/aio.h
M io/channel-command.c
M io/channel-file.c
M io/channel-socket.c
M migration/rdma.c
M tests/unit/meson.build
M tests/unit/test-aio.c
M tests/unit/test-bdrv-drain.c
R tests/unit/test-fdmon-epoll.c
M tests/unit/test-nested-aio-poll.c
M util/aio-posix.c
M util/aio-posix.h
M util/aio-win32.c
M util/async.c
M util/fdmon-epoll.c
M util/fdmon-io_uring.c
M util/fdmon-poll.c
M util/main-loop.c
M util/qemu-coroutine-io.c
M util/vhost-user-server.c
Log Message:
-----------
aio: remove aio_disable_external() API
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>
Commit: f89f54d52bf8fdc6de1c90367f9bdd65e40fa382
https://github.com/qemu/qemu/commit/f89f54d52bf8fdc6de1c90367f9bdd65e40fa382
Author: Richard Henderson <richard.henderson@linaro.org>
Date: 2023-05-30 (Tue, 30 May 2023)
Changed paths:
M block.c
M block/blkio.c
M block/block-backend.c
M block/copy-before-write.c
M block/curl.c
M block/export/export.c
M block/export/fuse.c
M block/export/vduse-blk.c
M block/export/vhost-user-blk-server.c
M block/io.c
M block/io_uring.c
M block/iscsi.c
M block/linux-aio.c
M block/mirror.c
M block/nfs.c
M block/nvme.c
M block/qapi-sysemu.c
M block/qcow2.c
M block/raw-format.c
M block/ssh.c
M block/win32-aio.c
M blockdev.c
M hw/block/dataplane/virtio-blk.c
M hw/block/dataplane/xen-block.c
M hw/block/dataplane/xen-block.h
M hw/block/virtio-blk.c
M hw/block/xen-block.c
M hw/i386/kvm/xen_xenstore.c
M hw/scsi/scsi-bus.c
M hw/scsi/scsi-disk.c
M hw/scsi/trace-events
M hw/scsi/virtio-scsi-dataplane.c
M hw/scsi/virtio-scsi.c
M hw/virtio/virtio.c
M hw/xen/xen-bus.c
M include/block/aio.h
M include/block/block-common.h
M include/block/block_int-common.h
M include/block/export.h
M include/hw/qdev-core.h
M include/hw/scsi/scsi.h
M include/qemu/vhost-user-server.h
M include/sysemu/block-backend-common.h
M include/sysemu/block-backend-global-state.h
M io/channel-command.c
M io/channel-file.c
M io/channel-socket.c
M migration/rdma.c
M qemu-nbd.c
M scripts/block-coroutine-wrapper.py
M tests/qemu-iotests/256
M tests/qemu-iotests/iotests.py
A tests/qemu-iotests/tests/iothreads-create
A tests/qemu-iotests/tests/iothreads-create.out
M tests/unit/meson.build
M tests/unit/test-aio.c
M tests/unit/test-bdrv-drain.c
M tests/unit/test-block-iothread.c
R tests/unit/test-fdmon-epoll.c
M tests/unit/test-nested-aio-poll.c
M util/aio-posix.c
M util/aio-posix.h
M util/aio-win32.c
M util/async.c
M util/fdmon-epoll.c
M util/fdmon-io_uring.c
M util/fdmon-poll.c
M util/main-loop.c
M util/qemu-coroutine-io.c
M util/vhost-user-server.c
Log Message:
-----------
Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging
Block layer patches
- Fix blockdev-create with iothreads
- Remove aio_disable_external() API
# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmR2JIARHGt3b2xmQHJl
# ZGhhdC5jb20ACgkQfwmycsiPL9brtA/9HVdAdtJxW78J60TE2lTqE9XlqMOEHBZl
# 8GN72trjP2geY/9mVsv/XoFie4ecqFsYjwAWWUuXZwLgAo53jh7oFN7gBH5iGyyD
# +EukYEfjqoykX5BkoK0gbMZZUe5Y4Dr2CNXYw4bNg8kDzj2RLifGA1XhdL3HoiVt
# PHZrhwBR7ddww6gVOnyJrfGL8fMkW/ZNeKRhrTZuSP+63oDOeGTsTumD+YKJzfPs
# p5WlwkuPjcqbO+w32FeVOHVhNI4swkN5svz3fkr8NuflfA7kH6nBQ5wymObbaTLc
# Erx03lrtP1+6nw43V11UnYt6iDMg4EBUQwtzNaKFnk3rMIdjoQYxIM5FTBWL2rYD
# Dg6PhkncXQ1WNWhUaFqpTFLB52XAYsSa4/y2QAGP6nWbqAUAUknQ3exaMvWiq7Z0
# nZeyyhIWvpJIHGCArWRdqqh+zsBdsmUVuPGyZnZgL/cXoJboYiHMyMJSUWE0XxML
# NGrncwxdsBXkVGGwTdHpBT64dcu3ENRgwtraqRLQm+tp5MKNTJB/+Ug2/p1vonHT
# UOoHz//UPskn8sHIyevoHXeu2Ns0uIHzrAXr+7Ay+9UYyIH6a07F4b2BGqkfyi/i
# 8wQsDmJ/idx5C4q1+jS+GuIbpnjIx6nxXwXMqpscUXZmM4Am8OMkiKxQAa1wExGF
# paId+HHwyks=
# =yuER
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue 30 May 2023 09:29:52 AM PDT
# gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg: issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
* tag 'for-upstream' of https://repo.or.cz/qemu/kevin: (32 commits)
aio: remove aio_disable_external() API
virtio: do not set is_external=true on host notifiers
virtio-scsi: implement BlockDevOps->drained_begin()
virtio-blk: implement BlockDevOps->drained_begin()
virtio: make it possible to detach host notifier from any thread
block/fuse: do not set is_external=true on FUSE fd
block/export: don't require AioContext lock around blk_exp_ref/unref()
block/export: rewrite vduse-blk drain code
hw/xen: do not set is_external=true on evtchn fds
xen-block: implement BlockDevOps->drained_begin()
block: drain from main loop thread in bdrv_co_yield_to_drain()
block: add blk_in_drain() API
hw/xen: do not use aio_set_fd_handler(is_external=true) in xen_xenstore
block/export: stop using is_external in vhost-user-blk server
block/export: wait for vhost-user-blk requests when draining
util/vhost-user-server: rename refcount to in_flight counter
virtio-scsi: stop using aio_disable_external() during unplug
virtio-scsi: avoid race between unplug and transport event
hw/qdev: introduce qdev_is_realized() helper
block-backend: split blk_do_set_aio_context()
...
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Compare: https://github.com/qemu/qemu/compare/7fe6cb68117a...f89f54d52bf8
- [Qemu-commits] [qemu/qemu] dea97c: block-coroutine-wrapper: Take AioContext lock in n...,
Richard Henderson <=