Proxmox-Port/qemu
1
0
Fork 0
mirror of https://github.com/qemu/qemu.git synced 2025-08-07 23:16:19 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

block: Make permission changes in reopen less wrong

Browse Source 
The way that reopen interacts with permission changes has one big
problem: Both operations are recursive, and the permissions are changes
for each node in the reopen queue.

For a simple graph that consists just of parent and child,
.bdrv_check_perm will be called twice for the child, once recursively
when adjusting the permissions of parent, and once again when the child
itself is reopened.

Even worse, the first .bdrv_check_perm call happens before
.bdrv_reopen_prepare was called for the child and the second one is
called afterwards.

Making sure that .bdrv_check_perm (and the other permission callbacks)
are called only once is hard. We can cope with multiple calls right now,
but as soon as file-posix gets a dynamic auto-read-only that may need to
open a new file descriptor, we get the additional requirement that all
of them are after the .bdrv_reopen_prepare call.

So reorder things in bdrv_reopen_multiple() to first call
.bdrv_reopen_prepare for all involved nodes and only then adjust
permissions.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Kevin Wolf 2019-03-05 17:18:22 +01:00
parent a4615ab31c
commit 69b736e765
1 changed files with 24 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
block.c
View File

@ -1698,6 +1698,7 @@ static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
typedef struct BlockReopenQueueEntry { typedef struct BlockReopenQueueEntry {
bool prepared; bool prepared;
bool perms_checked;
BDRVReopenState state; BDRVReopenState state;
QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry; QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;
} BlockReopenQueueEntry; } BlockReopenQueueEntry;
@ -3166,6 +3167,16 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er
bs_entry->prepared = true; bs_entry->prepared = true;
} }
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);
if (ret < 0) {
goto cleanup_perm;
}
bs_entry->perms_checked = true;
}
/* If we reach this point, we have success and just need to apply the /* If we reach this point, we have success and just need to apply the
* changes * changes
*/ */
@ -3174,7 +3185,20 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er
} }
ret = 0; ret = 0;
cleanup_perm:
QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
BDRVReopenState *state = &bs_entry->state;
if (!bs_entry->perms_checked) {
continue;
}
if (ret == 0) {
bdrv_set_perm(state->bs, state->perm, state->shared_perm);
} else {
bdrv_abort_perm_update(state->bs);
}
}
cleanup: cleanup:
QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
if (ret) { if (ret) {
@ -3428,12 +3452,6 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
} while ((entry = qdict_next(reopen_state->options, entry))); } 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; ret = 0;
/* Restore the original reopen_state->options QDict */ /* Restore the original reopen_state->options QDict */
@ -3500,9 +3518,6 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
bdrv_refresh_limits(bs, NULL); bdrv_refresh_limits(bs, NULL);
bdrv_set_perm(reopen_state->bs, reopen_state->perm,
reopen_state->shared_perm);
new_can_write = new_can_write =
!bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE); !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);
if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) { if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) {
@ -3534,8 +3549,6 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
if (drv->bdrv_reopen_abort) { if (drv->bdrv_reopen_abort) {
drv->bdrv_reopen_abort(reopen_state); drv->bdrv_reopen_abort(reopen_state);
} }
bdrv_abort_perm_update(reopen_state->bs);
} }