Re: [Qemu-block] [PATCH v2 1/3] block: Make bdrv_is_writable() public

[Jeff Cody]

On Wed, Jun 06, 2018 at 09:37:00PM +0200, Max Reitz wrote:
> This is a useful function for the whole block layer, so make it public.
> At the same time, users outside of block.c probably do not need to make
> use of the reopen functionality, so rename the current function to
> bdrv_is_writable_after_reopen() create a new bdrv_is_writable() function
> that just passes NULL to it for the reopen queue.
> 
> Cc: address@hidden
> Signed-off-by: Max Reitz <address@hidden>

Thanks!

Reviewed-by: Jeff Cody <address@hidden>

> ---
>  include/block/block.h |  1 +
>  block.c               | 17 ++++++++++++++---
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/include/block/block.h b/include/block/block.h
> index 4dd4f1eab2..e677080c4e 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -408,6 +408,7 @@ bool bdrv_is_read_only(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);
> +bool bdrv_is_writable(BlockDriverState *bs);
>  bool bdrv_is_sg(BlockDriverState *bs);
>  bool bdrv_is_inserted(BlockDriverState *bs);
>  void bdrv_lock_medium(BlockDriverState *bs, bool locked);
> diff --git a/block.c b/block.c
> index 9d577f65bb..50887087f3 100644
> --- a/block.c
> +++ b/block.c
> @@ -1620,13 +1620,24 @@ 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)
> +static bool bdrv_is_writable_after_reopen(BlockDriverState *bs,
> +                                          BlockReopenQueue *q)
>  {
>      int flags = bdrv_reopen_get_flags(q, bs);
>  
>      return (flags & (BDRV_O_RDWR | BDRV_O_INACTIVE)) == BDRV_O_RDWR;
>  }
>  
> +/*
> + * Return whether the BDS can be written to.  This is not necessarily
> + * the same as !bdrv_is_read_only(bs), as inactivated images may not
> + * be written to but do not count as read-only images.
> + */
> +bool bdrv_is_writable(BlockDriverState *bs)
> +{
> +    return bdrv_is_writable_after_reopen(bs, NULL);
> +}
> +
>  static void bdrv_child_perm(BlockDriverState *bs, BlockDriverState *child_bs,
>                              BdrvChild *c, const BdrvChildRole *role,
>                              BlockReopenQueue *reopen_queue,
> @@ -1664,7 +1675,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, q))
> +        !bdrv_is_writable_after_reopen(bs, q))
>      {
>          error_setg(errp, "Block node is read-only");
>          return -EPERM;
> @@ -1956,7 +1967,7 @@ void bdrv_format_default_perms(BlockDriverState *bs, 
> BdrvChild *c,
>                                    &perm, &shared);
>  
>          /* Format drivers may touch metadata even if the guest doesn't write 
> */
> -        if (bdrv_is_writable(bs, reopen_queue)) {
> +        if (bdrv_is_writable_after_reopen(bs, reopen_queue)) {
>              perm |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
>          }
>  
> -- 
> 2.17.0
>