diff options
-rw-r--r-- | block.c | 155 | ||||
-rw-r--r-- | block/block-backend.c | 4 | ||||
-rw-r--r-- | block/commit.c | 2 | ||||
-rw-r--r-- | block/file-posix.c | 4 | ||||
-rw-r--r-- | block/mirror.c | 70 | ||||
-rw-r--r-- | block/replication.c | 21 | ||||
-rw-r--r-- | block/trace-events | 2 | ||||
-rw-r--r-- | blockdev.c | 19 | ||||
-rw-r--r-- | blockjob.c | 22 | ||||
-rw-r--r-- | include/block/block_int.h | 30 | ||||
-rw-r--r-- | include/block/blockjob.h | 9 | ||||
-rw-r--r-- | monitor/qmp-cmds.c | 5 | ||||
-rw-r--r-- | qapi/block-core.json | 4 | ||||
-rwxr-xr-x | tests/qemu-iotests/182 | 21 | ||||
-rw-r--r-- | tests/qemu-iotests/182.out | 6 | ||||
-rwxr-xr-x | tests/qemu-iotests/check | 1 | ||||
-rw-r--r-- | tests/test-blockjob.c | 5 |
17 files changed, 283 insertions, 97 deletions
@@ -1706,7 +1706,8 @@ static int bdrv_fill_options(QDict **options, const char *filename, static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q, uint64_t perm, uint64_t shared, - GSList *ignore_children, Error **errp); + GSList *ignore_children, + bool *tighten_restrictions, Error **errp); static void bdrv_child_abort_perm_update(BdrvChild *c); static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared); static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm, @@ -1781,18 +1782,43 @@ static void bdrv_child_perm(BlockDriverState *bs, BlockDriverState *child_bs, * permissions of all its parents. This involves checking whether all necessary * permission changes to child nodes can be performed. * + * Will set *tighten_restrictions to true if and only if new permissions have to + * be taken or currently shared permissions are to be unshared. Otherwise, + * errors are not fatal as long as the caller accepts that the restrictions + * remain tighter than they need to be. The caller still has to abort the + * transaction. + * @tighten_restrictions cannot be used together with @q: When reopening, we may + * encounter fatal errors even though no restrictions are to be tightened. For + * example, changing a node from RW to RO will fail if the WRITE permission is + * to be kept. + * * A call to this function must always be followed by a call to bdrv_set_perm() * or bdrv_abort_perm_update(). */ static int bdrv_check_perm(BlockDriverState *bs, BlockReopenQueue *q, uint64_t cumulative_perms, uint64_t cumulative_shared_perms, - GSList *ignore_children, Error **errp) + GSList *ignore_children, + bool *tighten_restrictions, Error **errp) { BlockDriver *drv = bs->drv; BdrvChild *c; int ret; + assert(!q || !tighten_restrictions); + + if (tighten_restrictions) { + uint64_t current_perms, current_shared; + uint64_t added_perms, removed_shared_perms; + + bdrv_get_cumulative_perm(bs, ¤t_perms, ¤t_shared); + + added_perms = cumulative_perms & ~current_perms; + removed_shared_perms = current_shared & ~cumulative_shared_perms; + + *tighten_restrictions = added_perms || removed_shared_perms; + } + /* Write permissions never work with read-only images */ if ((cumulative_perms & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) && !bdrv_is_writable_after_reopen(bs, q)) @@ -1833,11 +1859,18 @@ static int bdrv_check_perm(BlockDriverState *bs, BlockReopenQueue *q, /* Check all children */ QLIST_FOREACH(c, &bs->children, next) { uint64_t cur_perm, cur_shared; + bool child_tighten_restr; + bdrv_child_perm(bs, c->bs, c, c->role, q, cumulative_perms, cumulative_shared_perms, &cur_perm, &cur_shared); - ret = bdrv_child_check_perm(c, q, cur_perm, cur_shared, - ignore_children, errp); + ret = bdrv_child_check_perm(c, q, cur_perm, cur_shared, ignore_children, + tighten_restrictions ? &child_tighten_restr + : NULL, + errp); + if (tighten_restrictions) { + *tighten_restrictions |= child_tighten_restr; + } if (ret < 0) { return ret; } @@ -1961,17 +1994,23 @@ char *bdrv_perm_names(uint64_t perm) * set, the BdrvChild objects in this list are ignored in the calculations; * this allows checking permission updates for an existing reference. * + * See bdrv_check_perm() for the semantics of @tighten_restrictions. + * * Needs to be followed by a call to either bdrv_set_perm() or * bdrv_abort_perm_update(). */ static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q, uint64_t new_used_perm, uint64_t new_shared_perm, - GSList *ignore_children, Error **errp) + GSList *ignore_children, + bool *tighten_restrictions, + Error **errp) { BdrvChild *c; uint64_t cumulative_perms = new_used_perm; uint64_t cumulative_shared_perms = new_shared_perm; + assert(!q || !tighten_restrictions); + /* There is no reason why anyone couldn't tolerate write_unchanged */ assert(new_shared_perm & BLK_PERM_WRITE_UNCHANGED); @@ -1983,6 +2022,11 @@ static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q, if ((new_used_perm & c->shared_perm) != new_used_perm) { char *user = bdrv_child_user_desc(c); char *perm_names = bdrv_perm_names(new_used_perm & ~c->shared_perm); + + if (tighten_restrictions) { + *tighten_restrictions = true; + } + error_setg(errp, "Conflicts with use by %s as '%s', which does not " "allow '%s' on %s", user, c->name, perm_names, bdrv_get_node_name(c->bs)); @@ -1994,6 +2038,11 @@ static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q, if ((c->perm & new_shared_perm) != c->perm) { char *user = bdrv_child_user_desc(c); char *perm_names = bdrv_perm_names(c->perm & ~new_shared_perm); + + if (tighten_restrictions) { + *tighten_restrictions = true; + } + error_setg(errp, "Conflicts with use by %s as '%s', which uses " "'%s' on %s", user, c->name, perm_names, bdrv_get_node_name(c->bs)); @@ -2007,19 +2056,21 @@ static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q, } return bdrv_check_perm(bs, q, cumulative_perms, cumulative_shared_perms, - ignore_children, errp); + ignore_children, tighten_restrictions, errp); } /* Needs to be followed by a call to either bdrv_child_set_perm() or * bdrv_child_abort_perm_update(). */ static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q, uint64_t perm, uint64_t shared, - GSList *ignore_children, Error **errp) + GSList *ignore_children, + bool *tighten_restrictions, Error **errp) { int ret; ignore_children = g_slist_prepend(g_slist_copy(ignore_children), c); - ret = bdrv_check_update_perm(c->bs, q, perm, shared, ignore_children, errp); + ret = bdrv_check_update_perm(c->bs, q, perm, shared, ignore_children, + tighten_restrictions, errp); g_slist_free(ignore_children); if (ret < 0) { @@ -2070,11 +2121,26 @@ static void bdrv_child_abort_perm_update(BdrvChild *c) int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared, Error **errp) { + Error *local_err = NULL; int ret; + bool tighten_restrictions; - ret = bdrv_child_check_perm(c, NULL, perm, shared, NULL, errp); + ret = bdrv_child_check_perm(c, NULL, perm, shared, NULL, + &tighten_restrictions, &local_err); if (ret < 0) { bdrv_child_abort_perm_update(c); + if (tighten_restrictions) { + error_propagate(errp, local_err); + } else { + /* + * Our caller may intend to only loosen restrictions and + * does not expect this function to fail. Errors are not + * fatal in such a case, so we can just hide them from our + * caller. + */ + error_free(local_err); + ret = 0; + } return ret; } @@ -2083,6 +2149,18 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared, return 0; } +int bdrv_child_refresh_perms(BlockDriverState *bs, BdrvChild *c, Error **errp) +{ + uint64_t parent_perms, parent_shared; + uint64_t perms, shared; + + bdrv_get_cumulative_perm(bs, &parent_perms, &parent_shared); + bdrv_child_perm(bs, c->bs, c, c->role, NULL, parent_perms, parent_shared, + &perms, &shared); + + return bdrv_child_try_set_perm(c, perms, shared, errp); +} + void bdrv_filter_default_perms(BlockDriverState *bs, BdrvChild *c, const BdrvChildRole *role, BlockReopenQueue *reopen_queue, @@ -2228,23 +2306,41 @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs) bdrv_replace_child_noperm(child, new_bs); + /* + * Start with the new node's permissions. If @new_bs is a (direct + * or indirect) child of @old_bs, we must complete the permission + * update on @new_bs before we loosen the restrictions on @old_bs. + * Otherwise, bdrv_check_perm() on @old_bs would re-initiate + * updating the permissions of @new_bs, and thus not purely loosen + * restrictions. + */ + if (new_bs) { + bdrv_get_cumulative_perm(new_bs, &perm, &shared_perm); + bdrv_set_perm(new_bs, perm, shared_perm); + } + if (old_bs) { /* Update permissions for old node. This is guaranteed to succeed * because we're just taking a parent away, so we're loosening * restrictions. */ + bool tighten_restrictions; + int ret; + bdrv_get_cumulative_perm(old_bs, &perm, &shared_perm); - bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL, &error_abort); - bdrv_set_perm(old_bs, perm, shared_perm); + ret = bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL, + &tighten_restrictions, NULL); + assert(tighten_restrictions == false); + if (ret < 0) { + /* We only tried to loosen restrictions, so errors are not fatal */ + bdrv_abort_perm_update(old_bs); + } else { + bdrv_set_perm(old_bs, perm, shared_perm); + } /* When the parent requiring a non-default AioContext is removed, the * node moves back to the main AioContext */ bdrv_try_set_aio_context(old_bs, qemu_get_aio_context(), NULL); } - - if (new_bs) { - bdrv_get_cumulative_perm(new_bs, &perm, &shared_perm); - bdrv_set_perm(new_bs, perm, shared_perm); - } } /* @@ -2268,7 +2364,8 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, Error *local_err = NULL; int ret; - ret = bdrv_check_update_perm(child_bs, NULL, perm, shared_perm, NULL, errp); + ret = bdrv_check_update_perm(child_bs, NULL, perm, shared_perm, NULL, NULL, + errp); if (ret < 0) { bdrv_abort_perm_update(child_bs); bdrv_unref(child_bs); @@ -3349,7 +3446,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) { BDRVReopenState *state = &bs_entry->state; ret = bdrv_check_perm(state->bs, bs_queue, state->perm, - state->shared_perm, NULL, errp); + state->shared_perm, NULL, NULL, errp); if (ret < 0) { goto cleanup_perm; } @@ -3361,7 +3458,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) state->perm, state->shared_perm, &nperm, &nshared); ret = bdrv_check_update_perm(state->new_backing_bs, NULL, - nperm, nshared, NULL, errp); + nperm, nshared, NULL, NULL, errp); if (ret < 0) { goto cleanup_perm; } @@ -3905,7 +4002,6 @@ static void bdrv_close(BlockDriverState *bs) BdrvAioNotifier *ban, *ban_next; BdrvChild *child, *next; - assert(!bs->job); assert(!bs->refcnt); bdrv_drained_begin(bs); /* complete I/O */ @@ -4078,7 +4174,7 @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to, /* Check whether the required permissions can be granted on @to, ignoring * all BdrvChild in @list so that they can't block themselves. */ - ret = bdrv_check_update_perm(to, NULL, perm, shared, list, errp); + ret = bdrv_check_update_perm(to, NULL, perm, shared, list, NULL, errp); if (ret < 0) { bdrv_abort_perm_update(to); goto out; @@ -4146,7 +4242,6 @@ out: static void bdrv_delete(BlockDriverState *bs) { - assert(!bs->job); assert(bdrv_op_blocker_is_empty(bs)); assert(!bs->refcnt); @@ -4426,7 +4521,7 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base, /* Check whether we are allowed to switch c from top to base */ GSList *ignore_children = g_slist_prepend(NULL, c); ret = bdrv_check_update_perm(base, NULL, c->perm, c->shared_perm, - ignore_children, &local_err); + ignore_children, NULL, &local_err); g_slist_free(ignore_children); if (ret < 0) { error_report_err(local_err); @@ -5201,7 +5296,7 @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, */ bs->open_flags &= ~BDRV_O_INACTIVE; bdrv_get_cumulative_perm(bs, &perm, &shared_perm); - ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, &local_err); + ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, &local_err); if (ret < 0) { bs->open_flags |= BDRV_O_INACTIVE; error_propagate(errp, local_err); @@ -5315,6 +5410,7 @@ static bool bdrv_has_bds_parent(BlockDriverState *bs, bool only_active) static int bdrv_inactivate_recurse(BlockDriverState *bs) { BdrvChild *child, *parent; + bool tighten_restrictions; uint64_t perm, shared_perm; int ret; @@ -5351,8 +5447,15 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs) /* Update permissions, they may differ for inactive nodes */ bdrv_get_cumulative_perm(bs, &perm, &shared_perm); - bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, &error_abort); - bdrv_set_perm(bs, perm, shared_perm); + ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, + &tighten_restrictions, NULL); + assert(tighten_restrictions == false); + if (ret < 0) { + /* We only tried to loosen restrictions, so errors are not fatal */ + bdrv_abort_perm_update(bs); + } else { + bdrv_set_perm(bs, perm, shared_perm); + } /* Recursively inactivate children */ diff --git a/block/block-backend.c b/block/block-backend.c index f5d9407d20..a8d160fd5d 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1073,11 +1073,7 @@ void blk_iostatus_disable(BlockBackend *blk) void blk_iostatus_reset(BlockBackend *blk) { if (blk_iostatus_is_enabled(blk)) { - BlockDriverState *bs = blk_bs(blk); blk->iostatus = BLOCK_DEVICE_IO_STATUS_OK; - if (bs && bs->job) { - block_job_iostatus_reset(bs->job); - } } } diff --git a/block/commit.c b/block/commit.c index c815def89a..212c6f639e 100644 --- a/block/commit.c +++ b/block/commit.c @@ -110,8 +110,6 @@ static void commit_abort(Job *job) * XXX Can (or should) we somehow keep 'consistent read' blocked even * after the failed/cancelled commit job is gone? If we already wrote * something to base, the intermediate images aren't valid any more. */ - bdrv_child_try_set_perm(s->commit_top_bs->backing, 0, BLK_PERM_ALL, - &error_abort); bdrv_replace_node(s->commit_top_bs, backing_bs(s->commit_top_bs), &error_abort); diff --git a/block/file-posix.c b/block/file-posix.c index 83ab1b78ef..ab05b51a66 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -146,6 +146,7 @@ typedef struct BDRVRawState { uint64_t locked_shared_perm; int perm_change_fd; + int perm_change_flags; BDRVReopenState *reopen_state; #ifdef CONFIG_XFS @@ -2788,6 +2789,7 @@ static int raw_check_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared, assert(s->reopen_state->shared_perm == shared); rs = s->reopen_state->opaque; s->perm_change_fd = rs->fd; + s->perm_change_flags = rs->open_flags; } else { /* We may need a new fd if auto-read-only switches the mode */ ret = raw_reconfigure_getfd(bs, bs->open_flags, &open_flags, perm, @@ -2796,6 +2798,7 @@ static int raw_check_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared, return ret; } else if (ret != s->fd) { s->perm_change_fd = ret; + s->perm_change_flags = open_flags; } } @@ -2834,6 +2837,7 @@ static void raw_set_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared) if (s->perm_change_fd && s->fd != s->perm_change_fd) { qemu_close(s->fd); s->fd = s->perm_change_fd; + s->open_flags = s->perm_change_flags; } s->perm_change_fd = 0; diff --git a/block/mirror.c b/block/mirror.c index f8bdb5b21b..d17be4cdbc 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -85,6 +85,7 @@ typedef struct MirrorBlockJob { typedef struct MirrorBDSOpaque { MirrorBlockJob *job; + bool stop; } MirrorBDSOpaque; struct MirrorOp { @@ -656,8 +657,9 @@ static int mirror_exit_common(Job *job) /* We don't access the source any more. Dropping any WRITE/RESIZE is * required before it could become a backing file of target_bs. */ - bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, - &error_abort); + bs_opaque->stop = true; + bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing, + &error_abort); if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) { BlockDriverState *backing = s->is_none_mode ? src : s->base; if (backing_bs(target_bs) != backing) { @@ -704,13 +706,12 @@ static int mirror_exit_common(Job *job) g_free(s->replaces); bdrv_unref(target_bs); - /* Remove the mirror filter driver from the graph. Before this, get rid of + /* + * Remove the mirror filter driver from the graph. Before this, get rid of * the blockers on the intermediate nodes so that the resulting state is - * valid. Also give up permissions on mirror_top_bs->backing, which might - * block the removal. */ + * valid. + */ block_job_remove_all_bdrv(bjob); - bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, - &error_abort); bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort); /* We just changed the BDS the job BB refers to (with either or both of the @@ -1459,6 +1460,18 @@ static void bdrv_mirror_top_child_perm(BlockDriverState *bs, BdrvChild *c, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared) { + MirrorBDSOpaque *s = bs->opaque; + + if (s->stop) { + /* + * If the job is to be stopped, we do not need to forward + * anything to the real image. + */ + *nperm = 0; + *nshared = BLK_PERM_ALL; + return; + } + /* Must be able to forward guest writes to the real image */ *nperm = 0; if (perm & BLK_PERM_WRITE) { @@ -1482,7 +1495,8 @@ static BlockDriver bdrv_mirror_top = { .bdrv_child_perm = bdrv_mirror_top_child_perm, }; -static void mirror_start_job(const char *job_id, BlockDriverState *bs, +static BlockJob *mirror_start_job( + const char *job_id, BlockDriverState *bs, int creation_flags, BlockDriverState *target, const char *replaces, int64_t speed, uint32_t granularity, int64_t buf_size, @@ -1514,7 +1528,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, if (buf_size < 0) { error_setg(errp, "Invalid parameter 'buf-size'"); - return; + return NULL; } if (buf_size == 0) { @@ -1523,7 +1537,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, if (bs == target) { error_setg(errp, "Can't mirror node into itself"); - return; + return NULL; } /* In the case of active commit, add dummy driver to provide consistent @@ -1532,7 +1546,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, mirror_top_bs = bdrv_new_open_driver(&bdrv_mirror_top, filter_node_name, BDRV_O_RDWR, errp); if (mirror_top_bs == NULL) { - return; + return NULL; } if (!filter_node_name) { mirror_top_bs->implicit = true; @@ -1554,7 +1568,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, if (local_err) { bdrv_unref(mirror_top_bs); error_propagate(errp, local_err); - return; + return NULL; } /* Make sure that the source is not resized while the job is running */ @@ -1662,7 +1676,8 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, trace_mirror_start(bs, s, opaque); job_start(&s->common.job); - return; + + return &s->common; fail: if (s) { @@ -1679,11 +1694,14 @@ fail: job_early_fail(&s->common.job); } - bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, - &error_abort); + bs_opaque->stop = true; + bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing, + &error_abort); bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort); bdrv_unref(mirror_top_bs); + + return NULL; } void mirror_start(const char *job_id, BlockDriverState *bs, @@ -1712,25 +1730,27 @@ void mirror_start(const char *job_id, BlockDriverState *bs, filter_node_name, true, copy_mode, errp); } -void commit_active_start(const char *job_id, BlockDriverState *bs, - BlockDriverState *base, int creation_flags, - int64_t speed, BlockdevOnError on_error, - const char *filter_node_name, - BlockCompletionFunc *cb, void *opaque, - bool auto_complete, Error **errp) +BlockJob *commit_active_start(const char *job_id, BlockDriverState *bs, + BlockDriverState *base, int creation_flags, + int64_t speed, BlockdevOnError on_error, + const char *filter_node_name, + BlockCompletionFunc *cb, void *opaque, + bool auto_complete, Error **errp) { bool base_read_only; Error *local_err = NULL; + BlockJob *ret; base_read_only = bdrv_is_read_only(base); if (base_read_only) { if (bdrv_reopen_set_read_only(base, false, errp) < 0) { - return; + return NULL; } } - mirror_start_job(job_id, bs, creation_flags, base, NULL, speed, 0, 0, + ret = mirror_start_job( + job_id, bs, creation_flags, base, NULL, speed, 0, 0, MIRROR_LEAVE_BACKING_CHAIN, on_error, on_error, true, cb, opaque, &commit_active_job_driver, false, base, auto_complete, @@ -1741,7 +1761,7 @@ void commit_active_start(const char *job_id, BlockDriverState *bs, goto error_restore_flags; } - return; + return ret; error_restore_flags: /* ignore error and errp for bdrv_reopen, because we want to propagate @@ -1749,5 +1769,5 @@ error_restore_flags: if (base_read_only) { bdrv_reopen_set_read_only(base, true, NULL); } - return; + return NULL; } diff --git a/block/replication.c b/block/replication.c index 5cffb790b3..b41bc507c0 100644 --- a/block/replication.c +++ b/block/replication.c @@ -36,8 +36,10 @@ typedef struct BDRVReplicationState { ReplicationMode mode; ReplicationStage stage; BdrvChild *active_disk; + BlockJob *commit_job; BdrvChild *hidden_disk; BdrvChild *secondary_disk; + BlockJob *backup_job; char *top_id; ReplicationState *rs; Error *blocker; @@ -147,7 +149,7 @@ static void replication_close(BlockDriverState *bs) replication_stop(s->rs, false, NULL); } if (s->stage == BLOCK_REPLICATION_FAILOVER) { - job_cancel_sync(&s->active_disk->bs->job->job); + job_cancel_sync(&s->commit_job->job); } if (s->mode == REPLICATION_MODE_SECONDARY) { @@ -315,12 +317,12 @@ static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp) Error *local_err = NULL; int ret; - if (!s->secondary_disk->bs->job) { + if (!s->backup_job) { error_setg(errp, "Backup job was cancelled unexpectedly"); return; } - backup_do_checkpoint(s->secondary_disk->bs->job, &local_err); + backup_do_checkpoint(s->backup_job, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -449,7 +451,6 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, int64_t active_length, hidden_length, disk_length; AioContext *aio_context; Error *local_err = NULL; - BlockJob *job; aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); @@ -540,7 +541,8 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, bdrv_op_block_all(top_bs, s->blocker); bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker); - job = backup_job_create(NULL, s->secondary_disk->bs, s->hidden_disk->bs, + s->backup_job = backup_job_create( + NULL, s->secondary_disk->bs, s->hidden_disk->bs, 0, MIRROR_SYNC_MODE_NONE, NULL, false, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL, @@ -551,7 +553,7 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, aio_context_release(aio_context); return; } - job_start(&job->job); + job_start(&s->backup_job->job); break; default: aio_context_release(aio_context); @@ -653,8 +655,8 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp) * before the BDS is closed, because we will access hidden * disk, secondary disk in backup_job_completed(). */ - if (s->secondary_disk->bs->job) { - job_cancel_sync(&s->secondary_disk->bs->job->job); + if (s->backup_job) { + job_cancel_sync(&s->backup_job->job); } if (!failover) { @@ -665,7 +667,8 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp) } s->stage = BLOCK_REPLICATION_FAILOVER; - commit_active_start(NULL, s->active_disk->bs, s->secondary_disk->bs, + s->commit_job = commit_active_start( + NULL, s->active_disk->bs, s->secondary_disk->bs, JOB_INTERNAL, 0, BLOCKDEV_ON_ERROR_REPORT, NULL, replication_done, bs, true, errp); break; diff --git a/block/trace-events b/block/trace-events index f6e43ee023..9ccea755da 100644 --- a/block/trace-events +++ b/block/trace-events @@ -53,7 +53,7 @@ qmp_block_job_resume(void *job) "job %p" qmp_block_job_complete(void *job) "job %p" qmp_block_job_finalize(void *job) "job %p" qmp_block_job_dismiss(void *job) "job %p" -qmp_block_stream(void *bs, void *job) "bs %p job %p" +qmp_block_stream(void *bs) "bs %p" # file-posix.c # file-win32.c diff --git a/blockdev.c b/blockdev.c index b5c0fd3c49..5d6a13dea9 100644 --- a/blockdev.c +++ b/blockdev.c @@ -140,22 +140,21 @@ void override_max_devs(BlockInterfaceType type, int max_devs) void blockdev_mark_auto_del(BlockBackend *blk) { DriveInfo *dinfo = blk_legacy_dinfo(blk); - BlockDriverState *bs = blk_bs(blk); - AioContext *aio_context; + BlockJob *job; if (!dinfo) { return; } - if (bs) { - aio_context = bdrv_get_aio_context(bs); - aio_context_acquire(aio_context); + for (job = block_job_next(NULL); job; job = block_job_next(job)) { + if (block_job_has_bdrv(job, blk_bs(blk))) { + AioContext *aio_context = job->job.aio_context; + aio_context_acquire(aio_context); - if (bs->job) { - job_cancel(&bs->job->job, false); - } + job_cancel(&job->job, false); - aio_context_release(aio_context); + aio_context_release(aio_context); + } } dinfo->auto_del = 1; @@ -3261,7 +3260,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, goto out; } - trace_qmp_block_stream(bs, bs->job); + trace_qmp_block_stream(bs); out: aio_context_release(aio_context); diff --git a/blockjob.c b/blockjob.c index 1fac6bb8a7..458ae76f51 100644 --- a/blockjob.c +++ b/blockjob.c @@ -83,9 +83,7 @@ BlockJob *block_job_get(const char *id) void block_job_free(Job *job) { BlockJob *bjob = container_of(job, BlockJob, job); - BlockDriverState *bs = blk_bs(bjob->blk); - bs->job = NULL; block_job_remove_all_bdrv(bjob); blk_unref(bjob->blk); error_free(bjob->blocker); @@ -198,6 +196,20 @@ void block_job_remove_all_bdrv(BlockJob *job) job->nodes = NULL; } +bool block_job_has_bdrv(BlockJob *job, BlockDriverState *bs) +{ + GSList *el; + + for (el = job->nodes; el; el = el->next) { + BdrvChild *c = el->data; + if (c->bs == bs) { + return true; + } + } + + return false; +} + int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs, uint64_t perm, uint64_t shared_perm, Error **errp) { @@ -388,11 +400,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, BlockJob *job; int ret; - if (bs->job) { - error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); - return NULL; - } - if (job_id == NULL && !(flags & JOB_INTERNAL)) { job_id = bdrv_get_device_name(bs); } @@ -435,7 +442,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, error_setg(&job->blocker, "block device is in use by block job: %s", job_type_str(&job->job)); block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort); - bs->job = job; bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); diff --git a/include/block/block_int.h b/include/block/block_int.h index 06df2bda1b..d6415b53c1 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -812,9 +812,6 @@ struct BlockDriverState { /* operation blockers */ QLIST_HEAD(, BdrvOpBlocker) op_blockers[BLOCK_OP_TYPE_MAX]; - /* long-running background operation */ - BlockJob *job; - /* The node that this node inherited default options from (and a reopen on * which can affect this node by changing these defaults). This is always a * parent node of this node. */ @@ -1082,12 +1079,12 @@ void commit_start(const char *job_id, BlockDriverState *bs, * @errp: Error object. * */ -void commit_active_start(const char *job_id, BlockDriverState *bs, - BlockDriverState *base, int creation_flags, - int64_t speed, BlockdevOnError on_error, - const char *filter_node_name, - BlockCompletionFunc *cb, void *opaque, - bool auto_complete, Error **errp); +BlockJob *commit_active_start(const char *job_id, BlockDriverState *bs, + BlockDriverState *base, int creation_flags, + int64_t speed, BlockdevOnError on_error, + const char *filter_node_name, + BlockCompletionFunc *cb, void *opaque, + bool auto_complete, Error **errp); /* * mirror_start: * @job_id: The id of the newly-created job, or %NULL to use the @@ -1168,9 +1165,24 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, void *opaque, Error **errp); void bdrv_root_unref_child(BdrvChild *child); +/** + * Sets a BdrvChild's permissions. Avoid if the parent is a BDS; use + * bdrv_child_refresh_perms() instead and make the parent's + * .bdrv_child_perm() implementation return the correct values. + */ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared, Error **errp); +/** + * Calls bs->drv->bdrv_child_perm() and updates the child's permission + * masks with the result. + * Drivers should invoke this function whenever an event occurs that + * makes their .bdrv_child_perm() implementation return different + * values than before, but which will not result in the block layer + * automatically refreshing the permissions. + */ +int bdrv_child_refresh_perms(BlockDriverState *bs, BdrvChild *c, Error **errp); + /* Default implementation for BlockDriver.bdrv_child_perm() that can be used by * block filters: Forward CONSISTENT_READ, WRITE, WRITE_UNCHANGED and RESIZE to * all children */ diff --git a/include/block/blockjob.h b/include/block/blockjob.h index ede0bd8dcb..35faa3aa26 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -122,6 +122,15 @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs, void block_job_remove_all_bdrv(BlockJob *job); /** + * block_job_has_bdrv: + * @job: The block job + * + * Searches for @bs in the list of nodes that are involved in the + * job. + */ +bool block_job_has_bdrv(BlockJob *job, BlockDriverState *bs); + +/** * block_job_set_speed: * @job: The job to set the speed for. * @speed: The new value diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index f1b1e4f08b..01ce77e129 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -143,6 +143,7 @@ void qmp_x_exit_preconfig(Error **errp) void qmp_cont(Error **errp) { BlockBackend *blk; + BlockJob *job; Error *local_err = NULL; /* if there is a dump in background, we should wait until the dump @@ -166,6 +167,10 @@ void qmp_cont(Error **errp) blk_iostatus_reset(blk); } + for (job = block_job_next(NULL); job; job = block_job_next(job)) { + block_job_iostatus_reset(job); + } + /* Continuing after completed migration. Images have been inactivated to * allow the destination to take control. Need to get control back now. * diff --git a/qapi/block-core.json b/qapi/block-core.json index 61124431d8..0d43d4f37c 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2890,11 +2890,13 @@ # @latency-ns: emulated latency (in nanoseconds) in processing # requests. Default to zero which completes requests immediately. # (Since 2.4) +# @read-zeroes: if true, reads from the device produce zeroes; if false, the +# buffer is left unchanged. (default: false; since: 4.1) # # Since: 2.9 ## { 'struct': 'BlockdevOptionsNull', - 'data': { '*size': 'int', '*latency-ns': 'uint64' } } + 'data': { '*size': 'int', '*latency-ns': 'uint64', '*read-zeroes': 'bool' } } ## # @BlockdevOptionsNVMe: diff --git a/tests/qemu-iotests/182 b/tests/qemu-iotests/182 index 3a90ebfbfd..7f494eb9bb 100755 --- a/tests/qemu-iotests/182 +++ b/tests/qemu-iotests/182 @@ -152,6 +152,27 @@ success_or_failure=y _send_qemu_cmd $QEMU_HANDLE \ _cleanup_qemu +echo +echo '=== Testing failure to loosen restrictions ===' +echo + +_launch_qemu -drive file=$TEST_IMG,if=none,file.locking=on + +_send_qemu_cmd $QEMU_HANDLE \ + "{'execute': 'qmp_capabilities'}" \ + 'return' + +_cleanup_test_img + +# When quitting qemu, it will try to drop its locks on the test image. +# Because that file no longer exists, it will be unable to do so. +# However, that is not fatal, so it should just move on. +_send_qemu_cmd $QEMU_HANDLE \ + "{'execute': 'quit'}" \ + 'return' + +wait=1 _cleanup_qemu + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/182.out b/tests/qemu-iotests/182.out index 33d41eea91..ffef23e32b 100644 --- a/tests/qemu-iotests/182.out +++ b/tests/qemu-iotests/182.out @@ -15,4 +15,10 @@ Formatting 'TEST_DIR/t.qcow2.overlay', fmt=qcow2 size=197120 backing_file=TEST_D {"return": {}} {"return": {}} {"return": {}} + +=== Testing failure to loosen restrictions === + +{"return": {}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}} *** done diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 44ebf24080..f925606cc5 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -773,6 +773,7 @@ do printdiff=false # show diff to reference output? status="" # test result summary results="" # test result details + thistime="" # time the test took if [ -n "$TESTS_REMAINING_LOG" ] ; then sed -e "s/$seq//" -e 's/ / /' -e 's/^ *//' $TESTS_REMAINING_LOG > $TESTS_REMAINING_LOG.tmp diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index 8c91980c70..b33f899873 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -122,8 +122,9 @@ static void test_job_ids(void) /* This one is valid */ job[0] = do_test_id(blk[0], "id0", true); - /* We cannot have two jobs in the same BDS */ - do_test_id(blk[0], "id1", false); + /* We can have two jobs in the same BDS */ + job[1] = do_test_id(blk[0], "id1", true); + job_early_fail(&job[1]->job); /* Duplicate job IDs are not allowed */ job[1] = do_test_id(blk[1], "id0", false); |