Re: [Qemu-devel] [PATCH 4/6] block: Base permissions on rw state after reopen

[Eric Blake]

On 09/15/2017 05:10 AM, Kevin Wolf wrote:
> 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 <address@hidden>
> ---
>  include/block/block.h |  1 -
>  block.c               | 52 
> ++++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 37 insertions(+), 16 deletions(-)
> 

> + * 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)

> +/* 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)

Is it worth having both functions with arguments in the same order?

> +{
> +    int flags = bdrv_reopen_get_flags(q, bs);
> +

No real semantic impact to leave it as is, but it would avoid the odd
swap of arguments here.  So either way,

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature