[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 1/3] block: include base when checking image cha
From: |
Andrey Shinkevich |
Subject: |
Re: [Qemu-block] [PATCH 1/3] block: include base when checking image chain for block allocation |
Date: |
Mon, 1 Apr 2019 09:26:43 +0000 |
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 <address@hidden>
>> Signed-off-by: Andrey Shinkevich <address@hidden>
>> ---
>> 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-block] [PATCH 1/3] block: include base when checking image chain for block allocation,
Andrey Shinkevich <=