Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
@rth7680
rth7680 committed May 30, 2023
2 parents 7fe6cb6 + 60f782b commit f89f54d
Show file tree
Hide file tree
Showing 70 changed files with 931 additions and 562 deletions.
46 changes: 31 additions & 15 deletions block.c
View file
Expand Up @@ -1613,6 +1613,7 @@ static int no_coroutine_fn GRAPH_UNLOCKED
bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, const char *node_name,
QDict *options, int open_flags, Error **errp)
{
AioContext *ctx;
Error *local_err = NULL;
int i, ret;
GLOBAL_STATE_CODE();
Expand Down Expand Up @@ -1660,15 +1661,21 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, const char *node_name,
bs->supported_read_flags |= BDRV_REQ_REGISTERED_BUF;
bs->supported_write_flags |= BDRV_REQ_REGISTERED_BUF;

/* Get the context after .bdrv_open, it can change the context */
ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);

ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not refresh total sector count");
aio_context_release(ctx);
return ret;
}

bdrv_graph_rdlock_main_loop();
bdrv_refresh_limits(bs, NULL, &local_err);
bdrv_graph_rdunlock_main_loop();
aio_context_release(ctx);

if (local_err) {
error_propagate(errp, local_err);
Expand Down Expand Up @@ -3478,6 +3485,8 @@ int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
* itself, all options starting with "${bdref_key}." are considered part of the
* BlockdevRef.
*
* The caller must hold the main AioContext lock.
*
* TODO Can this be unified with bdrv_open_image()?
*/
int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
Expand Down Expand Up @@ -3644,6 +3653,9 @@ bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
* BlockdevRef.
*
* The BlockdevRef will be removed from the options QDict.
*
* @parent can move to a different AioContext in this function. Callers must
* make sure that their AioContext locking is still correct after this.
*/
BdrvChild *bdrv_open_child(const char *filename,
QDict *options, const char *bdref_key,
Expand All @@ -3668,6 +3680,9 @@ BdrvChild *bdrv_open_child(const char *filename,

/*
* Wrapper on bdrv_open_child() for most popular case: open primary child of bs.
*
* @parent can move to a different AioContext in this function. Callers must
* make sure that their AioContext locking is still correct after this.
*/
int bdrv_open_file_child(const char *filename,
QDict *options, const char *bdref_key,
Expand Down Expand Up @@ -3810,9 +3825,7 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs,
* should be opened. If specified, neither options nor a filename may be given,
* nor can an existing BDS be reused (that is, *pbs has to be NULL).
*
* The caller must always hold @filename AioContext lock, because this
* function eventually calls bdrv_refresh_total_sectors() which polls
* when called from non-coroutine context.
* The caller must always hold the main AioContext lock.
*/
static BlockDriverState * no_coroutine_fn
bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
Expand Down Expand Up @@ -4100,11 +4113,7 @@ bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
return NULL;
}

/*
* The caller must always hold @filename AioContext lock, because this
* function eventually calls bdrv_refresh_total_sectors() which polls
* when called from non-coroutine context.
*/
/* The caller must always hold the main AioContext lock. */
BlockDriverState *bdrv_open(const char *filename, const char *reference,
QDict *options, int flags, Error **errp)
{
Expand Down Expand Up @@ -5390,12 +5399,17 @@ static void bdrv_delete(BlockDriverState *bs)
* empty set of options. The reference to the QDict belongs to the block layer
* after the call (even on failure), so if the caller intends to reuse the
* dictionary, it needs to use qobject_ref() before calling bdrv_open.
*
* The caller holds the AioContext lock for @bs. It must make sure that @bs
* stays in the same AioContext, i.e. @options must not refer to nodes in a
* different AioContext.
*/
BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *options,
int flags, Error **errp)
{
ERRP_GUARD();
int ret;
AioContext *ctx = bdrv_get_aio_context(bs);
BlockDriverState *new_node_bs = NULL;
const char *drvname, *node_name;
BlockDriver *drv;
Expand All @@ -5416,8 +5430,14 @@ BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *options,

GLOBAL_STATE_CODE();

aio_context_release(ctx);
aio_context_acquire(qemu_get_aio_context());
new_node_bs = bdrv_new_open_driver_opts(drv, node_name, options, flags,
errp);
aio_context_release(qemu_get_aio_context());
aio_context_acquire(ctx);
assert(bdrv_get_aio_context(bs) == ctx);

options = NULL; /* bdrv_new_open_driver() eats options */
if (!new_node_bs) {
error_prepend(errp, "Could not create node: ");
Expand Down Expand Up @@ -7043,6 +7063,8 @@ void bdrv_img_create(const char *filename, const char *fmt,
return;
}

aio_context_acquire(qemu_get_aio_context());

/* Create parameter list */
create_opts = qemu_opts_append(create_opts, drv->create_opts);
create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
Expand Down Expand Up @@ -7192,6 +7214,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
qemu_opts_del(opts);
qemu_opts_free(create_opts);
error_propagate(errp, local_err);
aio_context_release(qemu_get_aio_context());
}

AioContext *bdrv_get_aio_context(BlockDriverState *bs)
Expand Down Expand Up @@ -7282,9 +7305,6 @@ static void bdrv_detach_aio_context(BlockDriverState *bs)
bs->drv->bdrv_detach_aio_context(bs);
}

if (bs->quiesce_counter) {
aio_enable_external(bs->aio_context);
}
bs->aio_context = NULL;
}

Expand All @@ -7294,10 +7314,6 @@ static void bdrv_attach_aio_context(BlockDriverState *bs,
BdrvAioNotifier *ban, *ban_tmp;
GLOBAL_STATE_CODE();

if (bs->quiesce_counter) {
aio_disable_external(new_context);
}

bs->aio_context = new_context;

if (bs->drv && bs->drv->bdrv_attach_aio_context) {
Expand Down
15 changes: 5 additions & 10 deletions block/blkio.c
View file
Expand Up @@ -306,23 +306,18 @@ static void blkio_attach_aio_context(BlockDriverState *bs,
{
BDRVBlkioState *s = bs->opaque;

aio_set_fd_handler(new_context,
s->completion_fd,
false,
blkio_completion_fd_read,
NULL,
aio_set_fd_handler(new_context, s->completion_fd,
blkio_completion_fd_read, NULL,
blkio_completion_fd_poll,
blkio_completion_fd_poll_ready,
bs);
blkio_completion_fd_poll_ready, bs);
}

static void blkio_detach_aio_context(BlockDriverState *bs)
{
BDRVBlkioState *s = bs->opaque;

aio_set_fd_handler(bdrv_get_aio_context(bs),
s->completion_fd,
false, NULL, NULL, NULL, NULL, NULL);
aio_set_fd_handler(bdrv_get_aio_context(bs), s->completion_fd, NULL, NULL,
NULL, NULL, NULL);
}

/* Call with s->blkio_lock held to submit I/O after enqueuing a new request */
Expand Down
104 changes: 59 additions & 45 deletions block/block-backend.c
View file
Expand Up @@ -389,6 +389,8 @@ BlockBackend *blk_new(AioContext *ctx, uint64_t perm, uint64_t shared_perm)
* Both sets of permissions can be changed later using blk_set_perm().
*
* Return the new BlockBackend on success, null on failure.
*
* Callers must hold the AioContext lock of @bs.
*/
BlockBackend *blk_new_with_bs(BlockDriverState *bs, uint64_t perm,
uint64_t shared_perm, Error **errp)
Expand All @@ -406,11 +408,15 @@ BlockBackend *blk_new_with_bs(BlockDriverState *bs, uint64_t perm,

/*
* Creates a new BlockBackend, opens a new BlockDriverState, and connects both.
* The new BlockBackend is in the main AioContext.
* By default, the new BlockBackend is in the main AioContext, but if the
* parameters connect it with any existing node in a different AioContext, it
* may end up there instead.
*
* Just as with bdrv_open(), after having called this function the reference to
* @options belongs to the block layer (even on failure).
*
* Called without holding an AioContext lock.
*
* TODO: Remove @filename and @flags; it should be possible to specify a whole
* BDS tree just by specifying the @options QDict (or @reference,
* alternatively). At the time of adding this function, this is not possible,
Expand All @@ -422,6 +428,7 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
{
BlockBackend *blk;
BlockDriverState *bs;
AioContext *ctx;
uint64_t perm = 0;
uint64_t shared = BLK_PERM_ALL;

Expand Down Expand Up @@ -451,16 +458,24 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
shared = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
}

blk = blk_new(qemu_get_aio_context(), perm, shared);
aio_context_acquire(qemu_get_aio_context());
bs = bdrv_open(filename, reference, options, flags, errp);
aio_context_release(qemu_get_aio_context());
if (!bs) {
blk_unref(blk);
return NULL;
}

blk->root = bdrv_root_attach_child(bs, "root", &child_root,
BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
perm, shared, blk, errp);
/* bdrv_open() could have moved bs to a different AioContext */
ctx = bdrv_get_aio_context(bs);
blk = blk_new(bdrv_get_aio_context(bs), perm, shared);
blk->perm = perm;
blk->shared_perm = shared;

aio_context_acquire(ctx);
blk_insert_bs(blk, bs, errp);
bdrv_unref(bs);
aio_context_release(ctx);

if (!blk->root) {
blk_unref(blk);
return NULL;
Expand Down Expand Up @@ -901,6 +916,8 @@ void blk_remove_bs(BlockBackend *blk)

/*
* Associates a new BlockDriverState with @blk.
*
* Callers must hold the AioContext lock of @bs.
*/
int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
{
Expand Down Expand Up @@ -1270,6 +1287,13 @@ blk_check_byte_request(BlockBackend *blk, int64_t offset, int64_t bytes)
return 0;
}

/* Are we currently in a drained section? */
bool blk_in_drain(BlockBackend *blk)
{
GLOBAL_STATE_CODE(); /* change to IO_OR_GS_CODE(), if necessary */
return qatomic_read(&blk->quiesce_counter);
}

/* To be called between exactly one pair of blk_inc/dec_in_flight() */
static void coroutine_fn blk_wait_while_drained(BlockBackend *blk)
{
Expand Down Expand Up @@ -2394,9 +2418,14 @@ void blk_op_unblock_all(BlockBackend *blk, Error *reason)

AioContext *blk_get_aio_context(BlockBackend *blk)
{
BlockDriverState *bs = blk_bs(blk);
BlockDriverState *bs;
IO_CODE();

if (!blk) {
return qemu_get_aio_context();
}

bs = blk_bs(blk);
if (bs) {
AioContext *ctx = bdrv_get_aio_context(blk_bs(blk));
assert(ctx == blk->ctx);
Expand All @@ -2411,52 +2440,31 @@ static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb)
return blk_get_aio_context(blk_acb->blk);
}

static int blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context,
bool update_root_node, Error **errp)
int blk_set_aio_context(BlockBackend *blk, AioContext *new_context,
Error **errp)
{
bool old_allow_change;
BlockDriverState *bs = blk_bs(blk);
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
int ret;

if (bs) {
bdrv_ref(bs);

if (update_root_node) {
/*
* update_root_node MUST be false for blk_root_set_aio_ctx_commit(),
* as we are already in the commit function of a transaction.
*/
ret = bdrv_try_change_aio_context(bs, new_context, blk->root, errp);
if (ret < 0) {
bdrv_unref(bs);
return ret;
}
}
/*
* Make blk->ctx consistent with the root node before we invoke any
* other operations like drain that might inquire blk->ctx
*/
blk->ctx = new_context;
if (tgm->throttle_state) {
bdrv_drained_begin(bs);
throttle_group_detach_aio_context(tgm);
throttle_group_attach_aio_context(tgm, new_context);
bdrv_drained_end(bs);
}
GLOBAL_STATE_CODE();

bdrv_unref(bs);
} else {
if (!bs) {
blk->ctx = new_context;
return 0;
}

return 0;
}
bdrv_ref(bs);

int blk_set_aio_context(BlockBackend *blk, AioContext *new_context,
Error **errp)
{
GLOBAL_STATE_CODE();
return blk_do_set_aio_context(blk, new_context, true, errp);
old_allow_change = blk->allow_aio_context_change;
blk->allow_aio_context_change = true;

ret = bdrv_try_change_aio_context(bs, new_context, NULL, errp);

blk->allow_aio_context_change = old_allow_change;

bdrv_unref(bs);
return ret;
}

typedef struct BdrvStateBlkRootContext {
Expand All @@ -2468,8 +2476,14 @@ static void blk_root_set_aio_ctx_commit(void *opaque)
{
BdrvStateBlkRootContext *s = opaque;
BlockBackend *blk = s->blk;
AioContext *new_context = s->new_ctx;
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;

blk_do_set_aio_context(blk, s->new_ctx, false, &error_abort);
blk->ctx = new_context;
if (tgm->throttle_state) {
throttle_group_detach_aio_context(tgm);
throttle_group_attach_aio_context(tgm, new_context);
}
}

static TransactionActionDrv set_blk_root_context = {
Expand Down

0 comments on commit f89f54d

Please sign in to comment.