mirrors/qemu
0
0
Fork 0
mirror of https://github.com/qemu/qemu.git synced 2025-01-21 04:53:26 +08:00
Code Issues Packages Projects Releases Wiki Activity

block: bdrv_reopen_multiple: refresh permissions on updated graph

Browse Source 
Move bdrv_reopen_multiple to new paradigm of permission update:
first update graph relations, then do refresh the permissions.

We have to modify reopen process in file-posix driver: with new scheme
we don't have prepared permissions in raw_reopen_prepare(), so we
should reconfigure fd in raw_check_perm(). Still this seems more native
and simple anyway.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210428151804.439460-31-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Vladimir Sementsov-Ogievskiy 2021-04-28 18:17:58 +03:00 committed by Kevin Wolf
parent a2aabf8895
commit 72373e40fb
3 changed files with 88 additions and 201 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

193
block.c
View File

@ -95,8 +95,9 @@ static int bdrv_attach_child_noperm(BlockDriverState *parent_bs,
static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs,
Transaction *tran);
static int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue
*queue, Error **errp);
static int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue,
Transaction *set_backings_tran, Error **errp);
static void bdrv_reopen_commit(BDRVReopenState *reopen_state);
static void bdrv_reopen_abort(BDRVReopenState *reopen_state);
@ -2468,6 +2469,7 @@ static void bdrv_list_abort_perm_update(GSList *list)
}
}
__attribute__((unused))
static void bdrv_abort_perm_update(BlockDriverState *bs)
{
g_autoptr(GSList) list = bdrv_topological_dfs(NULL, NULL, bs);
@ -2563,6 +2565,7 @@ char *bdrv_perm_names(uint64_t perm)
*
* Needs to be followed by a call to either bdrv_set_perm() or
* bdrv_abort_perm_update(). */
__attribute__((unused))
static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q,
uint64_t new_used_perm,
uint64_t new_shared_perm,
@ -4183,10 +4186,6 @@ 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;
/*
* If keep_old_opts is false then it means that unspecified
* options must be reset to their original value. We don't allow
@ -4271,6 +4270,9 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
{
int ret = -1;
BlockReopenQueueEntry *bs_entry, *next;
Transaction *tran = tran_new();
g_autoptr(GHashTable) found = NULL;
g_autoptr(GSList) refresh_list = NULL;
assert(bs_queue != NULL);
@ -4284,33 +4286,33 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
QTAILQ_FOREACH(bs_entry, bs_queue, entry) {
assert(bs_entry->state.bs->quiesce_counter > 0);
if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, errp)) {
goto cleanup;
ret = bdrv_reopen_prepare(&bs_entry->state, bs_queue, tran, errp);
if (ret < 0) {
goto abort;
}
bs_entry->prepared = true;
}
found = g_hash_table_new(NULL, NULL);
QTAILQ_FOREACH(bs_entry, bs_queue, entry) {
BDRVReopenState *state = &bs_entry->state;
ret = bdrv_check_perm(state->bs, bs_queue, state->perm,
state->shared_perm, errp);
if (ret < 0) {
goto cleanup_perm;
refresh_list = bdrv_topological_dfs(refresh_list, found, state->bs);
if (state->old_backing_bs) {
refresh_list = bdrv_topological_dfs(refresh_list, found,
state->old_backing_bs);
}
/* Check if new_backing_bs would accept the new permissions */
if (state->replace_backing_bs && state->new_backing_bs) {
uint64_t nperm, nshared;
bdrv_child_perm(state->bs, state->new_backing_bs,
NULL, bdrv_backing_role(state->bs),
bs_queue, state->perm, state->shared_perm,
&nperm, &nshared);
ret = bdrv_check_update_perm(state->new_backing_bs, NULL,
nperm, nshared, errp);
if (ret < 0) {
goto cleanup_perm;
}
}
bs_entry->perms_checked = true;
}
/*
* Note that file-posix driver rely on permission update done during reopen
* (even if no permission changed), because it wants "new" permissions for
* reconfiguring the fd and that's why it does it in raw_check_perm(), not
* in raw_reopen_prepare() which is called with "old" permissions.
*/
ret = bdrv_list_refresh_perms(refresh_list, bs_queue, tran, errp);
if (ret < 0) {
goto abort;
}
/*
@ -4326,51 +4328,31 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
bdrv_reopen_commit(&bs_entry->state);
}
tran_commit(tran);
QTAILQ_FOREACH_REVERSE(bs_entry, bs_queue, entry) {
BlockDriverState *bs = bs_entry->state.bs;
if (bs->drv->bdrv_reopen_commit_post) {
bs->drv->bdrv_reopen_commit_post(&bs_entry->state);
}
}
ret = 0;
cleanup_perm:
goto cleanup;
abort:
tran_abort(tran);
QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
BDRVReopenState *state = &bs_entry->state;
if (!bs_entry->perms_checked) {
continue;
}
if (ret == 0) {
uint64_t perm, shared;
bdrv_get_cumulative_perm(state->bs, &perm, &shared);
assert(perm == state->perm);
assert(shared == state->shared_perm);
bdrv_set_perm(state->bs);
} else {
bdrv_abort_perm_update(state->bs);
if (state->replace_backing_bs && state->new_backing_bs) {
bdrv_abort_perm_update(state->new_backing_bs);
}
if (bs_entry->prepared) {
bdrv_reopen_abort(&bs_entry->state);
}
qobject_unref(bs_entry->state.explicit_options);
qobject_unref(bs_entry->state.options);
}
if (ret == 0) {
QTAILQ_FOREACH_REVERSE(bs_entry, bs_queue, entry) {
BlockDriverState *bs = bs_entry->state.bs;
if (bs->drv->bdrv_reopen_commit_post)
bs->drv->bdrv_reopen_commit_post(&bs_entry->state);
}
}
cleanup:
QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
if (ret) {
if (bs_entry->prepared) {
bdrv_reopen_abort(&bs_entry->state);
}
qobject_unref(bs_entry->state.explicit_options);
qobject_unref(bs_entry->state.options);
}
if (bs_entry->state.new_backing_bs) {
bdrv_unref(bs_entry->state.new_backing_bs);
}
g_free(bs_entry);
}
g_free(bs_queue);
@ -4395,53 +4377,6 @@ int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only,
return ret;
}
static BlockReopenQueueEntry *find_parent_in_reopen_queue(BlockReopenQueue *q,
BdrvChild *c)
{
BlockReopenQueueEntry *entry;
QTAILQ_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;
}
static bool bdrv_reopen_can_attach(BlockDriverState *parent,
BdrvChild *child,
BlockDriverState *new_child,
@ -4483,6 +4418,7 @@ static bool bdrv_reopen_can_attach(BlockDriverState *parent,
* Return 0 on success, otherwise return < 0 and set @errp.
*/
static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
Transaction *set_backings_tran,
Error **errp)
{
BlockDriverState *bs = reopen_state->bs;
@ -4559,6 +4495,8 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
/* If we want to replace the backing file we need some extra checks */
if (new_backing_bs != bdrv_filter_or_cow_bs(overlay_bs)) {
int ret;
/* Check for implicit nodes between bs and its backing file */
if (bs != overlay_bs) {
error_setg(errp, "Cannot change backing link if '%s' has "
@ -4579,9 +4517,11 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
return -EPERM;
}
reopen_state->replace_backing_bs = true;
if (new_backing_bs) {
bdrv_ref(new_backing_bs);
reopen_state->new_backing_bs = new_backing_bs;
reopen_state->old_backing_bs = bs->backing ? bs->backing->bs : NULL;
ret = bdrv_set_backing_noperm(bs, new_backing_bs, set_backings_tran,
errp);
if (ret < 0) {
return ret;
}
}
@ -4606,7 +4546,8 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
*
*/
static int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue, Error **errp)
BlockReopenQueue *queue,
Transaction *set_backings_tran, Error **errp)
{
int ret = -1;
int old_flags;
@ -4673,10 +4614,6 @@ static int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
goto error;
}
/* Calculate required permissions after reopening */
bdrv_reopen_perm(queue, reopen_state->bs,
&reopen_state->perm, &reopen_state->shared_perm);
if (drv->bdrv_reopen_prepare) {
/*
* If a driver-specific option is missing, it means that we
@ -4730,7 +4667,7 @@ static int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
* either a reference to an existing node (using its node name)
* or NULL to simply detach the current backing file.
*/
ret = bdrv_reopen_parse_backing(reopen_state, errp);
ret = bdrv_reopen_parse_backing(reopen_state, set_backings_tran, errp);
if (ret < 0) {
goto error;
}
@ -4852,22 +4789,6 @@ static void bdrv_reopen_commit(BDRVReopenState *reopen_state)
qdict_del(bs->explicit_options, child->name);
qdict_del(bs->options, child->name);
}
/*
* Change the backing file if a new one was specified. We do this
* after updating bs->options, so bdrv_refresh_filename() (called
* from bdrv_set_backing_hd()) has the new values.
*/
if (reopen_state->replace_backing_bs) {
BlockDriverState *old_backing_bs = child_bs(bs->backing);
assert(!old_backing_bs || !old_backing_bs->implicit);
/* Abort the permission update on the backing bs we're detaching */
if (old_backing_bs) {
bdrv_abort_perm_update(old_backing_bs);
}
bdrv_set_backing_hd(bs, reopen_state->new_backing_bs, &error_abort);
}
bdrv_refresh_limits(bs, NULL, NULL);
}

93
block/file-posix.c
View File

@ -175,7 +175,6 @@ typedef struct BDRVRawState {
} BDRVRawState;
typedef struct BDRVRawReopenState {
int fd;
int open_flags;
bool drop_cache;
bool check_cache_dropped;
@ -1075,7 +1074,6 @@ static int raw_reopen_prepare(BDRVReopenState *state,
BDRVRawReopenState *rs;
QemuOpts *opts;
int ret;
Error *local_err = NULL;
assert(state != NULL);
assert(state->bs != NULL);
@ -1101,32 +1099,18 @@ static int raw_reopen_prepare(BDRVReopenState *state,
* bdrv_reopen_prepare() will detect changes and complain. */
qemu_opts_to_qdict(opts, state->options);
rs->fd = raw_reconfigure_getfd(state->bs, state->flags, &rs->open_flags,
state->perm, true, &local_err);
if (local_err) {
error_propagate(errp, local_err);
ret = -1;
goto out;
}
/* Fail already reopen_prepare() if we can't get a working O_DIRECT
* alignment with the new fd. */
if (rs->fd != -1) {
raw_probe_alignment(state->bs, rs->fd, &local_err);
if (local_err) {
error_propagate(errp, local_err);
ret = -EINVAL;
goto out_fd;
}
}
/*
* As part of reopen prepare we also want to create new fd by
* raw_reconfigure_getfd(). But it wants updated "perm", when in
* bdrv_reopen_multiple() .bdrv_reopen_prepare() callback called prior to
* permission update. Happily, permission update is always a part (a seprate
* stage) of bdrv_reopen_multiple() so we can rely on this fact and
* reconfigure fd in raw_check_perm().
*/
s->reopen_state = state;
ret = 0;
out_fd:
if (ret < 0) {
qemu_close(rs->fd);
rs->fd = -1;
}
out:
qemu_opts_del(opts);
return ret;
@ -1140,10 +1124,6 @@ static void raw_reopen_commit(BDRVReopenState *state)
s->drop_cache = rs->drop_cache;
s->check_cache_dropped = rs->check_cache_dropped;
s->open_flags = rs->open_flags;
qemu_close(s->fd);
s->fd = rs->fd;
g_free(state->opaque);
state->opaque = NULL;
@ -1162,10 +1142,6 @@ static void raw_reopen_abort(BDRVReopenState *state)
return;
}
if (rs->fd >= 0) {
qemu_close(rs->fd);
rs->fd = -1;
}
g_free(state->opaque);
state->opaque = NULL;
@ -3073,39 +3049,30 @@ static int raw_check_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared,
Error **errp)
{
BDRVRawState *s = bs->opaque;
BDRVRawReopenState *rs = NULL;
int input_flags = s->reopen_state ? s->reopen_state->flags : bs->open_flags;
int open_flags;
int ret;
if (s->perm_change_fd) {
/*
* In the context of reopen, this function may be called several times
* (directly and recursively while change permissions of the parent).
* This is even true for children that don't inherit from the original
* reopen node, so s->reopen_state is not set.
*
* Ignore all but the first call.
*/
return 0;
}
/* We may need a new fd if auto-read-only switches the mode */
ret = raw_reconfigure_getfd(bs, input_flags, &open_flags, perm,
false, errp);
if (ret < 0) {
return ret;
} else if (ret != s->fd) {
Error *local_err = NULL;
if (s->reopen_state) {
/* We already have a new file descriptor to set permissions for */
assert(s->reopen_state->perm == perm);
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,
false, errp);
if (ret < 0) {
return ret;
} else if (ret != s->fd) {
s->perm_change_fd = ret;
s->perm_change_flags = open_flags;
/*
* Fail already check_perm() if we can't get a working O_DIRECT
* alignment with the new fd.
*/
raw_probe_alignment(bs, ret, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return -EINVAL;
}
s->perm_change_fd = ret;
s->perm_change_flags = open_flags;
}
/* Prepare permissions on old fd to avoid conflicts between old and new,
@ -3127,7 +3094,7 @@ static int raw_check_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared,
return 0;
fail:
if (s->perm_change_fd && !s->reopen_state) {
if (s->perm_change_fd) {
qemu_close(s->perm_change_fd);
}
s->perm_change_fd = 0;
@ -3158,7 +3125,7 @@ static void raw_abort_perm_update(BlockDriverState *bs)
/* For reopen, .bdrv_reopen_abort is called afterwards and will close
* the file descriptor. */
if (s->perm_change_fd && !s->reopen_state) {
if (s->perm_change_fd) {
qemu_close(s->perm_change_fd);
}
s->perm_change_fd = 0;

3
include/block/block.h
View File

@ -208,8 +208,7 @@ typedef struct BDRVReopenState {
BlockdevDetectZeroesOptions detect_zeroes;
bool backing_missing;
bool replace_backing_bs; /* new_backing_bs is ignored if this is false */
BlockDriverState *new_backing_bs; /* If NULL then detach the current bs */
uint64_t perm, shared_perm;
BlockDriverState *old_backing_bs; /* keep pointer for permissions update */
QDict *options;
QDict *explicit_options;
void *opaque;