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

- virtio-blk: Multiqueue support (configurable iothread per queue)
- Made NBD export and hw/scsi thread-safe without AioContext lock
- Fix crash when loading snapshot on inactive node

# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmWErCsRHGt3b2xmQHJl
# ZGhhdC5jb20ACgkQfwmycsiPL9baXg//VqDXBG74IEBomEigyU/WE2y7PsXvyF93
# 11eDj1Rw5ygY9LTvJDQQh/1XwmobYKZDXUg1Wo6BE4xJX8aLGN22yWHt1v5GXxkM
# JgU7vuK8nDI60ClCD8/vevMsNdDXddWvy4Y5VmM0i+UXhvSdHRES8NSRV3s/b+TJ
# Vx33rlURPwhvdNLcL+iZvcH32FbpLolx7v467tUpa2xXhAm4sVYsGV0rZxrljPMI
# DmYWMqAA9tOJdeaWh1yi8nVb6nSdeRs6pSRfWnL2rPVdtl7xfK2Adj5DcEcrKRY+
# Ot/M+TYAw8AVwlNXAV9x8jPLy4TNdTBn8j6eYO+52lIyT8A9ybmdPTGHXO35H5bO
# DKYFEjPTCxCfjdZ+eoKoKiso3XsWCKTg+eD9VOEvyOKxS7w6Hv+k2aiMueYylPqs
# cU9047fEvVGlw/HWlF5LFamyC4gcrVIIUPjQlfxf1Si9gwErXOmqeRsnRk78S8Gg
# 1xMOUckR7dBJeVMX/53aMZXSegVW9P+iXYr2XXZQOaGYaq06dU52ZxvTDz1JzoH4
# l68d2f1vAzYcFXh5WZ4/fQAuvaQFtytTUOg4esbKcwMAK9V8Ud/A4jbMdmkfwmbp
# xJ7ebWdXj79Q8qQScOkhUWCAVO/Ix4yopokEUq4Y7N08nYdlRkNRYUKZJZyxAStJ
# MfGu0NqbwK4=
# =GxkV
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 21 Dec 2023 16:20:43 EST
# 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: (33 commits)
  virtio-blk: add iothread-vq-mapping parameter
  qdev: add IOThreadVirtQueueMappingList property type
  qdev-properties: alias all object class properties
  string-output-visitor: show structs as "<omitted>"
  block-coroutine-wrapper: use qemu_get_current_aio_context()
  block: remove outdated AioContext locking comments
  job: remove outdated AioContext locking comments
  scsi: remove outdated AioContext lock comment
  docs: remove AioContext lock from IOThread docs
  aio: remove aio_context_acquire()/aio_context_release() API
  aio-wait: draw equivalence between AIO_WAIT_WHILE() and AIO_WAIT_WHILE_UNLOCKED()
  scsi: remove AioContext locking
  block: remove bdrv_co_lock()
  block: remove AioContext locking
  graph-lock: remove AioContext locking
  aio: make aio_context_acquire()/aio_context_release() a no-op
  tests: remove aio_context_acquire() tests
  scsi: assert that callbacks run in the correct AioContext
  virtio-scsi: replace AioContext lock with tmf_bh_lock
  dma-helpers: don't lock AioContext in dma_blk_cb()
  ...

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
  • Loading branch information
Stefan Hajnoczi committed Dec 21, 2023
2 parents 191710c + ec25ed8 commit 38717bc
Show file tree
Hide file tree
Showing 82 changed files with 1,337 additions and 2,041 deletions.
363 changes: 46 additions & 317 deletions block.c

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions block/backup.c
Expand Up @@ -496,10 +496,10 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
block_copy_set_speed(bcs, speed);

/* Required permissions are taken by copy-before-write filter target */
bdrv_graph_wrlock(target);
bdrv_graph_wrlock();
block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL,
&error_abort);
bdrv_graph_wrunlock(target);
bdrv_graph_wrunlock();

return &job->common;

Expand Down
8 changes: 4 additions & 4 deletions block/blklogwrites.c
Expand Up @@ -251,9 +251,9 @@ static int blk_log_writes_open(BlockDriverState *bs, QDict *options, int flags,
ret = 0;
fail_log:
if (ret < 0) {
bdrv_graph_wrlock(NULL);
bdrv_graph_wrlock();
bdrv_unref_child(bs, s->log_file);
bdrv_graph_wrunlock(NULL);
bdrv_graph_wrunlock();
s->log_file = NULL;
}
fail:
Expand All @@ -265,10 +265,10 @@ static void blk_log_writes_close(BlockDriverState *bs)
{
BDRVBlkLogWritesState *s = bs->opaque;

bdrv_graph_wrlock(NULL);
bdrv_graph_wrlock();
bdrv_unref_child(bs, s->log_file);
s->log_file = NULL;
bdrv_graph_wrunlock(NULL);
bdrv_graph_wrunlock();
}

static int64_t coroutine_fn GRAPH_RDLOCK
Expand Down
4 changes: 2 additions & 2 deletions block/blkverify.c
Expand Up @@ -151,10 +151,10 @@ static void blkverify_close(BlockDriverState *bs)
{
BDRVBlkverifyState *s = bs->opaque;

bdrv_graph_wrlock(NULL);
bdrv_graph_wrlock();
bdrv_unref_child(bs, s->test_file);
s->test_file = NULL;
bdrv_graph_wrunlock(NULL);
bdrv_graph_wrunlock();
}

static int64_t coroutine_fn GRAPH_RDLOCK
Expand Down
33 changes: 4 additions & 29 deletions block/block-backend.c
Expand Up @@ -390,8 +390,6 @@ 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 @@ -416,8 +414,6 @@ BlockBackend *blk_new_with_bs(BlockDriverState *bs, uint64_t perm,
* 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 @@ -429,7 +425,6 @@ 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 @@ -459,23 +454,18 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
shared = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
}

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

/* 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);
Expand Down Expand Up @@ -577,13 +567,9 @@ void blk_remove_all_bs(void)
GLOBAL_STATE_CODE();

while ((blk = blk_all_next(blk)) != NULL) {
AioContext *ctx = blk_get_aio_context(blk);

aio_context_acquire(ctx);
if (blk->root) {
blk_remove_bs(blk);
}
aio_context_release(ctx);
}
}

Expand Down Expand Up @@ -882,14 +868,11 @@ BlockBackend *blk_by_public(BlockBackendPublic *public)

/*
* Disassociates the currently associated BlockDriverState from @blk.
*
* The caller must hold the AioContext lock for the BlockBackend.
*/
void blk_remove_bs(BlockBackend *blk)
{
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
BdrvChild *root;
AioContext *ctx;

GLOBAL_STATE_CODE();

Expand Down Expand Up @@ -919,30 +902,26 @@ void blk_remove_bs(BlockBackend *blk)
root = blk->root;
blk->root = NULL;

ctx = bdrv_get_aio_context(root->bs);
bdrv_graph_wrlock(root->bs);
bdrv_graph_wrlock();
bdrv_root_unref_child(root);
bdrv_graph_wrunlock_ctx(ctx);
bdrv_graph_wrunlock();
}

/*
* Associates a new BlockDriverState with @blk.
*
* Callers must hold the AioContext lock of @bs.
*/
int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
{
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
AioContext *ctx = bdrv_get_aio_context(bs);

GLOBAL_STATE_CODE();
bdrv_ref(bs);
bdrv_graph_wrlock(bs);
bdrv_graph_wrlock();
blk->root = bdrv_root_attach_child(bs, "root", &child_root,
BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
blk->perm, blk->shared_perm,
blk, errp);
bdrv_graph_wrunlock_ctx(ctx);
bdrv_graph_wrunlock();
if (blk->root == NULL) {
return -EPERM;
}
Expand Down Expand Up @@ -2739,20 +2718,16 @@ int blk_commit_all(void)
GRAPH_RDLOCK_GUARD_MAINLOOP();

while ((blk = blk_all_next(blk)) != NULL) {
AioContext *aio_context = blk_get_aio_context(blk);
BlockDriverState *unfiltered_bs = bdrv_skip_filters(blk_bs(blk));

aio_context_acquire(aio_context);
if (blk_is_inserted(blk) && bdrv_cow_child(unfiltered_bs)) {
int ret;

ret = bdrv_commit(unfiltered_bs);
if (ret < 0) {
aio_context_release(aio_context);
return ret;
}
}
aio_context_release(aio_context);
}
return 0;
}
Expand Down
16 changes: 8 additions & 8 deletions block/commit.c
Expand Up @@ -100,9 +100,9 @@ static void commit_abort(Job *job)
bdrv_graph_rdunlock_main_loop();

bdrv_drained_begin(commit_top_backing_bs);
bdrv_graph_wrlock(commit_top_backing_bs);
bdrv_graph_wrlock();
bdrv_replace_node(s->commit_top_bs, commit_top_backing_bs, &error_abort);
bdrv_graph_wrunlock(commit_top_backing_bs);
bdrv_graph_wrunlock();
bdrv_drained_end(commit_top_backing_bs);

bdrv_unref(s->commit_top_bs);
Expand Down Expand Up @@ -339,7 +339,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
* this is the responsibility of the interface (i.e. whoever calls
* commit_start()).
*/
bdrv_graph_wrlock(top);
bdrv_graph_wrlock();
s->base_overlay = bdrv_find_overlay(top, base);
assert(s->base_overlay);

Expand Down Expand Up @@ -370,19 +370,19 @@ void commit_start(const char *job_id, BlockDriverState *bs,
ret = block_job_add_bdrv(&s->common, "intermediate node", iter, 0,
iter_shared_perms, errp);
if (ret < 0) {
bdrv_graph_wrunlock(top);
bdrv_graph_wrunlock();
goto fail;
}
}

if (bdrv_freeze_backing_chain(commit_top_bs, base, errp) < 0) {
bdrv_graph_wrunlock(top);
bdrv_graph_wrunlock();
goto fail;
}
s->chain_frozen = true;

ret = block_job_add_bdrv(&s->common, "base", base, 0, BLK_PERM_ALL, errp);
bdrv_graph_wrunlock(top);
bdrv_graph_wrunlock();

if (ret < 0) {
goto fail;
Expand Down Expand Up @@ -434,9 +434,9 @@ void commit_start(const char *job_id, BlockDriverState *bs,
* otherwise this would fail because of lack of permissions. */
if (commit_top_bs) {
bdrv_drained_begin(top);
bdrv_graph_wrlock(top);
bdrv_graph_wrlock();
bdrv_replace_node(commit_top_bs, top, &error_abort);
bdrv_graph_wrunlock(top);
bdrv_graph_wrunlock();
bdrv_drained_end(top);
}
}
Expand Down
22 changes: 5 additions & 17 deletions block/copy-before-write.c
Expand Up @@ -412,7 +412,6 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
int64_t cluster_size;
g_autoptr(BlockdevOptions) full_opts = NULL;
BlockdevOptionsCbw *opts;
AioContext *ctx;
int ret;

full_opts = cbw_parse_options(options, errp);
Expand All @@ -435,15 +434,11 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,

GRAPH_RDLOCK_GUARD_MAINLOOP();

ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);

if (opts->bitmap) {
bitmap = block_dirty_bitmap_lookup(opts->bitmap->node,
opts->bitmap->name, NULL, errp);
if (!bitmap) {
ret = -EINVAL;
goto out;
return -EINVAL;
}
}
s->on_cbw_error = opts->has_on_cbw_error ? opts->on_cbw_error :
Expand All @@ -461,24 +456,21 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
s->bcs = block_copy_state_new(bs->file, s->target, bitmap, errp);
if (!s->bcs) {
error_prepend(errp, "Cannot create block-copy-state: ");
ret = -EINVAL;
goto out;
return -EINVAL;
}

cluster_size = block_copy_cluster_size(s->bcs);

s->done_bitmap = bdrv_create_dirty_bitmap(bs, cluster_size, NULL, errp);
if (!s->done_bitmap) {
ret = -EINVAL;
goto out;
return -EINVAL;
}
bdrv_disable_dirty_bitmap(s->done_bitmap);

/* s->access_bitmap starts equal to bcs bitmap */
s->access_bitmap = bdrv_create_dirty_bitmap(bs, cluster_size, NULL, errp);
if (!s->access_bitmap) {
ret = -EINVAL;
goto out;
return -EINVAL;
}
bdrv_disable_dirty_bitmap(s->access_bitmap);
bdrv_dirty_bitmap_merge_internal(s->access_bitmap,
Expand All @@ -487,11 +479,7 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,

qemu_co_mutex_init(&s->lock);
QLIST_INIT(&s->frozen_read_reqs);

ret = 0;
out:
aio_context_release(ctx);
return ret;
return 0;
}

static void cbw_close(BlockDriverState *bs)
Expand Down
22 changes: 1 addition & 21 deletions block/export/export.c
Expand Up @@ -114,7 +114,6 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
}

ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);

if (export->iothread) {
IOThread *iothread;
Expand All @@ -133,8 +132,6 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
set_context_errp = fixed_iothread ? errp : NULL;
ret = bdrv_try_change_aio_context(bs, new_ctx, NULL, set_context_errp);
if (ret == 0) {
aio_context_release(ctx);
aio_context_acquire(new_ctx);
ctx = new_ctx;
} else if (fixed_iothread) {
goto fail;
Expand Down Expand Up @@ -191,16 +188,13 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
assert(exp->blk != NULL);

QLIST_INSERT_HEAD(&block_exports, exp, next);

aio_context_release(ctx);
return exp;

fail:
if (blk) {
blk_set_dev_ops(blk, NULL, NULL);
blk_unref(blk);
}
aio_context_release(ctx);
if (exp) {
g_free(exp->id);
g_free(exp);
Expand All @@ -218,9 +212,6 @@ void blk_exp_ref(BlockExport *exp)
static void blk_exp_delete_bh(void *opaque)
{
BlockExport *exp = opaque;
AioContext *aio_context = exp->ctx;

aio_context_acquire(aio_context);

assert(exp->refcount == 0);
QLIST_REMOVE(exp, next);
Expand All @@ -230,8 +221,6 @@ static void blk_exp_delete_bh(void *opaque)
qapi_event_send_block_export_deleted(exp->id);
g_free(exp->id);
g_free(exp);

aio_context_release(aio_context);
}

void blk_exp_unref(BlockExport *exp)
Expand All @@ -249,32 +238,23 @@ void blk_exp_unref(BlockExport *exp)
* connections and other internally held references start to shut down. When
* the function returns, there may still be active references while the export
* is in the process of shutting down.
*
* Acquires exp->ctx internally. Callers must *not* hold the lock.
*/
void blk_exp_request_shutdown(BlockExport *exp)
{
AioContext *aio_context = exp->ctx;

aio_context_acquire(aio_context);

/*
* If the user doesn't own the export any more, it is already shutting
* down. We must not call .request_shutdown and decrease the refcount a
* second time.
*/
if (!exp->user_owned) {
goto out;
return;
}

exp->drv->request_shutdown(exp);

assert(exp->user_owned);
exp->user_owned = false;
blk_exp_unref(exp);

out:
aio_context_release(aio_context);
}

/*
Expand Down

0 comments on commit 38717bc

Please sign in to comment.