[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/7] file-posix: Update open_flags in raw_set_pe
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 1/7] file-posix: Update open_flags in raw_set_perm() |
Date: |
Wed, 8 May 2019 15:06:11 +0200 |
User-agent: |
Mutt/1.11.3 (2019-02-01) |
Am 06.05.2019 um 21:47 hat Max Reitz geschrieben:
> raw_check_perm() + raw_set_perm() can change the flags associated with
> the current FD. If so, we have to update BDRVRawState.open_flags
> accordingly. Otherwise, we may keep reopening the FD even though the
> current one already has the correct flags.
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
> block/file-posix.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 1cf4ee49eb..66b46ec0eb 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -145,6 +145,7 @@ typedef struct BDRVRawState {
> uint64_t locked_shared_perm;
>
> int perm_change_fd;
> + int perm_change_flags;
> BDRVReopenState *reopen_state;
>
> #ifdef CONFIG_XFS
> @@ -2762,6 +2763,7 @@ static int raw_check_perm(BlockDriverState *bs,
> uint64_t perm, uint64_t shared,
Adding some context before this hunk:
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;
} 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;
> }
> }
s->perm_change_flags is set in the else branch. According to the comment
in raw_set_perm(), not setting it in the then branch is actually correct
because .bdrv_reopen_commit will run first, so s->perm_change_flags
isn't accessed, but wouldn't it be nicer to have a valid value in it
anyway? Who knows where we'll add accesses later.
Kevin
> @@ -2800,6 +2802,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;
>
> --
> 2.20.1
>
- [Qemu-devel] [PATCH 0/7] block: Ignore loosening perm restrictions failures, Max Reitz, 2019/05/06
- [Qemu-devel] [PATCH 1/7] file-posix: Update open_flags in raw_set_perm(), Max Reitz, 2019/05/06
- Re: [Qemu-devel] [PATCH 1/7] file-posix: Update open_flags in raw_set_perm(),
Kevin Wolf <=
- [Qemu-devel] [PATCH 2/7] block: Add bdrv_child_refresh_perms(), Max Reitz, 2019/05/06
- [Qemu-devel] [PATCH 3/7] block/mirror: Fix child permissions, Max Reitz, 2019/05/06
- [Qemu-devel] [PATCH 4/7] block/commit: Drop bdrv_child_try_set_perm(), Max Reitz, 2019/05/06
- [Qemu-devel] [PATCH 5/7] block: Fix order in bdrv_replace_child(), Max Reitz, 2019/05/06
- [Qemu-devel] [PATCH 6/7] block: Add *loosen_restrictions to *check*_perm(), Max Reitz, 2019/05/06
- [Qemu-devel] [PATCH 7/7] block: Ignore loosening perm restrictions failures, Max Reitz, 2019/05/06
- Re: [Qemu-devel] [PATCH 0/7] block: Ignore loosening perm restrictions failures, Kevin Wolf, 2019/05/08