On 29/03/2019 19:24, Vladimir Sementsov-Ogievskiy wrote: > 29.03.2019 16:29, Andrey Shinkevich wrote: >> A caller of the function bdrv_is_allocated_above() may want to >> include the base node in the search. It is useful when we have >> parallel commit/stream jobs on the same backing image chain. The >> base node may be a top one of a parallel job at the same time >> and go away before the first job completed. Instead of base, pass >> the node that has the base as its backing one to the new function >> bdrv_is_allocated_above_inclusive() to manage the issue. > > I think it would be enough to say that it be used in following patch > to drop dependence on base from block-stream job. > >> >> Suggested-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> Signed-off-by: Andrey Shinkevich <andrey.shinkev...@virtuozzo.com> >> --- >> block/io.c | 39 ++++++++++++++++++++++++++++++++++----- >> include/block/block.h | 5 ++++- >> 2 files changed, 38 insertions(+), 6 deletions(-) >> >> diff --git a/block/io.c b/block/io.c >> index dfc153b..8b273e5 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,21 @@ 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; >> + bool exit_loop = top == base ? true : false; >> + /* Sanity check */ >> + base_included = base ? base_included : false; >> + bool include_node = top == base ? base_included : true; >> >> intermediate = top; >> - while (intermediate && intermediate != base) { >> + while (intermediate && include_node) { >> int64_t pnum_inter; >> int64_t size_inter; >> >> @@ -2361,12 +2367,35 @@ int bdrv_is_allocated_above(BlockDriverState *top, >> } >> >> intermediate = backing_bs(intermediate); >> + include_node = intermediate != base; >> + if (exit_loop) { >> + include_node = false; >> + } else if (!include_node && base_included) { >> + /* Iterate once more */ >> + include_node = true; >> + exit_loop = true; >> + } > > how about this, without additional variables:
Unfortunately, 9 test cases do not pass in the file 030. > > -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; > > intermediate = top; > - while (intermediate && intermediate != base) { > + while (base_included || intermediate != base) { > int64_t pnum_inter; > int64_t size_inter; > > @@ -2360,6 +2361,11 @@ int bdrv_is_allocated_above(BlockDriverState *top, > n = pnum_inter; > } > > + if (intermediate == base) { > + assert(!base_included); > + break; > + } > + > intermediate = backing_bs(intermediate); > } > > > >> } >> >> *pnum = n; >> 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); >> bool bdrv_is_read_only(BlockDriverState *bs); >> int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, >> bool ignore_allow_rdw, Error **errp); >> > > -- With the best regards, Andrey Shinkevich