From e0995dc3da0894d0a8260bddaa200a4cd7809ba4 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 14 Sep 2017 12:47:11 +0200 Subject: block: Add reopen_queue to bdrv_child_perm() In the context of bdrv_reopen(), we'll have to look at the state of the graph as it will be after the reopen. This interface addition is in preparation for the change. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- include/block/block_int.h | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'include') diff --git a/include/block/block_int.h b/include/block/block_int.h index ba4c383393..99abe2ce74 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -411,9 +411,14 @@ struct BlockDriver { * * If @c is NULL, return the permissions for attaching a new child for the * given @role. + * + * If @reopen_queue is non-NULL, don't return the currently needed + * permissions, but those that will be needed after applying the + * @reopen_queue. */ void (*bdrv_child_perm)(BlockDriverState *bs, BdrvChild *c, const BdrvChildRole *role, + BlockReopenQueue *reopen_queue, uint64_t parent_perm, uint64_t parent_shared, uint64_t *nperm, uint64_t *nshared); @@ -983,6 +988,7 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared, * all children */ void bdrv_filter_default_perms(BlockDriverState *bs, BdrvChild *c, const BdrvChildRole *role, + BlockReopenQueue *reopen_queue, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared); @@ -992,6 +998,7 @@ void bdrv_filter_default_perms(BlockDriverState *bs, BdrvChild *c, * CONSISTENT_READ and doesn't share WRITE. */ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c, const BdrvChildRole *role, + BlockReopenQueue *reopen_queue, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared); -- cgit v1.2.3-55-g7522 From 148eb13c84cccd0eedd6e59f90e0151bd7bac9fa Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 14 Sep 2017 14:32:04 +0200 Subject: block: Base permissions on rw state after reopen When new permissions are calculated during bdrv_reopen(), they need to be based on the state of the graph as it will be after the reopen has completed, not on the current state of the involved nodes. This patch makes bdrv_is_writable() optionally accept a BlockReopenQueue from which the new flags are taken. This is then used for determining the new bs->file permissions of format drivers as soon as we add the code to actually pass a non-NULL reopen queue to the .bdrv_child_perm callbacks. While moving bdrv_is_writable(), make it static. It isn't used outside block.c. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 52 ++++++++++++++++++++++++++++++++++++--------------- include/block/block.h | 1 - 2 files changed, 37 insertions(+), 16 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index 0b499fda4c..ed8d51dd42 100644 --- a/block.c +++ b/block.c @@ -239,12 +239,6 @@ bool bdrv_is_read_only(BlockDriverState *bs) return bs->read_only; } -/* Returns whether the image file can be written to right now */ -bool bdrv_is_writable(BlockDriverState *bs) -{ - return !bdrv_is_read_only(bs) && !(bs->open_flags & BDRV_O_INACTIVE); -} - int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, bool ignore_allow_rdw, Error **errp) { @@ -1537,6 +1531,41 @@ static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q, static void bdrv_child_abort_perm_update(BdrvChild *c); static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared); +typedef struct BlockReopenQueueEntry { + bool prepared; + BDRVReopenState state; + QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry; +} BlockReopenQueueEntry; + +/* + * Return the flags that @bs will have after the reopens in @q have + * successfully completed. If @q is NULL (or @bs is not contained in @q), + * return the current flags. + */ +static int bdrv_reopen_get_flags(BlockReopenQueue *q, BlockDriverState *bs) +{ + BlockReopenQueueEntry *entry; + + if (q != NULL) { + QSIMPLEQ_FOREACH(entry, q, entry) { + if (entry->state.bs == bs) { + return entry->state.flags; + } + } + } + + return bs->open_flags; +} + +/* Returns whether the image file can be written to after the reopen queue @q + * has been successfully applied, or right now if @q is NULL. */ +static bool bdrv_is_writable(BlockDriverState *bs, BlockReopenQueue *q) +{ + int flags = bdrv_reopen_get_flags(q, bs); + + return (flags & (BDRV_O_RDWR | BDRV_O_INACTIVE)) == BDRV_O_RDWR; +} + static void bdrv_child_perm(BlockDriverState *bs, BlockDriverState *child_bs, BdrvChild *c, const BdrvChildRole *role, BlockReopenQueue *reopen_queue, @@ -1574,7 +1603,7 @@ static int bdrv_check_perm(BlockDriverState *bs, BlockReopenQueue *q, /* Write permissions never work with read-only images */ if ((cumulative_perms & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) && - !bdrv_is_writable(bs)) + !bdrv_is_writable(bs, q)) { error_setg(errp, "Block node is read-only"); return -EPERM; @@ -1864,8 +1893,7 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c, &perm, &shared); /* Format drivers may touch metadata even if the guest doesn't write */ - /* TODO Take flags from reopen_queue */ - if (bdrv_is_writable(bs)) { + if (bdrv_is_writable(bs, reopen_queue)) { perm |= BLK_PERM_WRITE | BLK_PERM_RESIZE; } @@ -2642,12 +2670,6 @@ BlockDriverState *bdrv_open(const char *filename, const char *reference, NULL, errp); } -typedef struct BlockReopenQueueEntry { - bool prepared; - BDRVReopenState state; - QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry; -} BlockReopenQueueEntry; - /* * Adds a BlockDriverState to a simple queue for an atomic, transactional * reopen of multiple devices. diff --git a/include/block/block.h b/include/block/block.h index 2ad18775af..082eb2cd9c 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -435,7 +435,6 @@ int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base, int64_t offset, int64_t bytes, int64_t *pnum); bool bdrv_is_read_only(BlockDriverState *bs); -bool bdrv_is_writable(BlockDriverState *bs); int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, bool ignore_allow_rdw, Error **errp); int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp); -- cgit v1.2.3-55-g7522 From 3045025991ebeec77ce89c8ec56e83858950bbb3 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 3 Jul 2017 17:07:35 +0200 Subject: block: Fix permissions after bdrv_reopen() If we switch between read-only and read-write, the permissions that image format drivers need on bs->file change, too. Make sure to update the permissions during bdrv_reopen(). Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/block/block.h | 1 + 2 files changed, 65 insertions(+) (limited to 'include') diff --git a/block.c b/block.c index 204cbb46c7..5c65fac672 100644 --- a/block.c +++ b/block.c @@ -2781,6 +2781,10 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, bs_entry->state.explicit_options = explicit_options; bs_entry->state.flags = flags; + /* This needs to be overwritten in bdrv_reopen_prepare() */ + bs_entry->state.perm = UINT64_MAX; + bs_entry->state.shared_perm = 0; + QLIST_FOREACH(child, &bs->children, next) { QDict *new_child_options; char *child_key_dot; @@ -2887,6 +2891,52 @@ int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp) return ret; } +static BlockReopenQueueEntry *find_parent_in_reopen_queue(BlockReopenQueue *q, + BdrvChild *c) +{ + BlockReopenQueueEntry *entry; + + QSIMPLEQ_FOREACH(entry, q, entry) { + BlockDriverState *bs = entry->state.bs; + BdrvChild *child; + + QLIST_FOREACH(child, &bs->children, next) { + if (child == c) { + return entry; + } + } + } + + return NULL; +} + +static void bdrv_reopen_perm(BlockReopenQueue *q, BlockDriverState *bs, + uint64_t *perm, uint64_t *shared) +{ + BdrvChild *c; + BlockReopenQueueEntry *parent; + uint64_t cumulative_perms = 0; + uint64_t cumulative_shared_perms = BLK_PERM_ALL; + + QLIST_FOREACH(c, &bs->parents, next_parent) { + parent = find_parent_in_reopen_queue(q, c); + if (!parent) { + cumulative_perms |= c->perm; + cumulative_shared_perms &= c->shared_perm; + } else { + uint64_t nperm, nshared; + + bdrv_child_perm(parent->state.bs, bs, c, c->role, q, + parent->state.perm, parent->state.shared_perm, + &nperm, &nshared); + + cumulative_perms |= nperm; + cumulative_shared_perms &= nshared; + } + } + *perm = cumulative_perms; + *shared = cumulative_shared_perms; +} /* * Prepares a BlockDriverState for reopen. All changes are staged in the @@ -2952,6 +3002,9 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, goto error; } + /* Calculate required permissions after reopening */ + bdrv_reopen_perm(queue, reopen_state->bs, + &reopen_state->perm, &reopen_state->shared_perm); ret = bdrv_flush(reopen_state->bs); if (ret) { @@ -3007,6 +3060,12 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, } while ((entry = qdict_next(reopen_state->options, entry))); } + ret = bdrv_check_perm(reopen_state->bs, queue, reopen_state->perm, + reopen_state->shared_perm, NULL, errp); + if (ret < 0) { + goto error; + } + ret = 0; error: @@ -3047,6 +3106,9 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) bdrv_refresh_limits(bs, NULL); + bdrv_set_perm(reopen_state->bs, reopen_state->perm, + reopen_state->shared_perm); + new_can_write = !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE); if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) { @@ -3080,6 +3142,8 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state) } QDECREF(reopen_state->explicit_options); + + bdrv_abort_perm_update(reopen_state->bs); } diff --git a/include/block/block.h b/include/block/block.h index 082eb2cd9c..3c3af462e4 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -166,6 +166,7 @@ typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue; typedef struct BDRVReopenState { BlockDriverState *bs; int flags; + uint64_t perm, shared_perm; QDict *options; QDict *explicit_options; void *opaque; -- cgit v1.2.3-55-g7522