Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 5/9] block: introduce bdrv_try_set_read_only()

[John Snow]

On 04/05/2017 02:28 PM, Jeff Cody wrote:
> Introduce try function for setting read_only flags.  Will return < 0 on
> error, with appropriate Error value set.  Does not alter any flags.
> 
> Signed-off-by: Jeff Cody <address@hidden>
> ---
>  block.c               | 14 +++++++++++++-
>  include/block/block.h |  1 +
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 8bfe7f4..ad958b9 100644
> --- a/block.c
> +++ b/block.c
> @@ -197,7 +197,7 @@ bool bdrv_is_read_only(BlockDriverState *bs)
>      return bs->read_only;
>  }
>  
> -int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
> +int bdrv_try_set_read_only(BlockDriverState *bs, bool read_only, Error 
> **errp)
>  {
>      /* Do not set read_only if copy_on_read is enabled */
>      if (bs->copy_on_read && read_only) {
> @@ -213,6 +213,18 @@ int bdrv_set_read_only(BlockDriverState *bs, bool 
> read_only, Error **errp)
>          return -EPERM;
>      }
>  
> +    return 0;
> +}
> +
> +int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
> +{
> +    int ret = 0;
> +
> +    ret = bdrv_try_set_read_only(bs, read_only, errp);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
>      bs->read_only = read_only;
>      return 0;
>  }
> diff --git a/include/block/block.h b/include/block/block.h
> index beb563a..0049b57 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -426,6 +426,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, 
> BlockDriverState *base,
>                              int64_t sector_num, int nb_sectors, int *pnum);
>  
>  bool bdrv_is_read_only(BlockDriverState *bs);
> +int bdrv_try_set_read_only(BlockDriverState *bs, bool read_only, Error 
> **errp);
>  int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
>  bool bdrv_is_sg(BlockDriverState *bs);
>  bool bdrv_is_inserted(BlockDriverState *bs);
> 

Maybe I'm just green, but I find it weird that the function is named
try_x when we aren't actually trying to do anything, we're checking to
see if we can.