[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/3] block: include base when checking image
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/3] block: include base when checking image chain for block allocation |
Date: |
Mon, 1 Apr 2019 12:29:53 +0000 |
01.04.2019 15:06, Andrey Shinkevich wrote:
> This patch is used in the 'block/stream: introduce a bottom node'
> that is following. Instead of the base node, the caller may pass
> the node that has the base as its backing image to the new function
> bdrv_is_allocated_above_inclusive() and get rid of the dependency
> on the base that may change during commit/stream parallel jobs.
> If the specified base is not found in the backing image chain, the
> function fails.
More honest: s/fail/now crashes/
And I think it's correct. But it would be good to check all callers.. Do we
freeze backing chain in all cases before calling this function?
>
> Suggested-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> Signed-off-by: Andrey Shinkevich <address@hidden>
> ---
> block/io.c | 32 +++++++++++++++++++++++++++-----
> include/block/block.h | 5 ++++-
> 2 files changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index dfc153b..7edeeb2 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2317,7 +2317,8 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState
> *bs, int64_t offset,
> * Given an image chain: ... -> [BASE] -> [INTER1] -> [INTER2] -> [TOP]
> *
> * Return true if (a prefix of) the given range is allocated in any image
> - * between BASE and TOP (inclusive). BASE can be NULL to check if the given
> + * between BASE and TOP (TOP included). To check the BASE image, set the
> + * 'base_included' to 'true'. The BASE can be NULL to check if the given
> * offset is allocated in any image of the chain. Return false otherwise,
> * or negative errno on failure.
> *
> @@ -2329,16 +2330,18 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState
> *bs, int64_t offset,
> * but 'pnum' will only be 0 when end of file is reached.
> *
> */
> -int bdrv_is_allocated_above(BlockDriverState *top,
> - BlockDriverState *base,
> - int64_t offset, int64_t bytes, int64_t *pnum)
> +static int bdrv_do_is_allocated_above(BlockDriverState *top,
> + BlockDriverState *base,
> + bool base_included, int64_t offset,
> + int64_t bytes, int64_t *pnum)
> {
> BlockDriverState *intermediate;
> int ret;
> int64_t n = bytes;
empty line after variable definitions
> + assert(base || !base_included);
>
> intermediate = top;
> - while (intermediate && intermediate != base) {
> + while (base_included || intermediate != base) {
> int64_t pnum_inter;
> int64_t size_inter;
>
> @@ -2360,6 +2363,10 @@ int bdrv_is_allocated_above(BlockDriverState *top,
> n = pnum_inter;
> }
>
> + if (intermediate == base) {
> + break;
> + }
> +
> intermediate = backing_bs(intermediate);
> }
>
> @@ -2367,6 +2374,21 @@ int bdrv_is_allocated_above(BlockDriverState *top,
> return 0;
> }
>
> +int bdrv_is_allocated_above(BlockDriverState *top,
> + BlockDriverState *base,
> + int64_t offset, int64_t bytes, int64_t *pnum)
> +{
> + return bdrv_do_is_allocated_above(top, base, false, offset, bytes, pnum);
> +}
> +
> +int bdrv_is_allocated_above_inclusive(BlockDriverState *top,
> + BlockDriverState *base,
> + int64_t offset, int64_t bytes,
> + int64_t *pnum)
> +{
> + return bdrv_do_is_allocated_above(top, base, true, offset, bytes, pnum);
> +}
> +
> typedef struct BdrvVmstateCo {
> BlockDriverState *bs;
> QEMUIOVector *qiov;
> diff --git a/include/block/block.h b/include/block/block.h
> index c7a2619..a7846d9 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -448,7 +448,10 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t
> offset, int64_t bytes,
> int64_t *pnum);
> int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
> int64_t offset, int64_t bytes, int64_t *pnum);
> -
> +int bdrv_is_allocated_above_inclusive(BlockDriverState *top,
> + BlockDriverState *base,
> + int64_t offset, int64_t bytes,
> + int64_t *pnum);
dropped empty line better stay here.
> bool bdrv_is_read_only(BlockDriverState *bs);
> int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only,
> bool ignore_allow_rdw, Error **errp);
>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
--
Best regards,
Vladimir