qemu-block
[Top][All Lists]
Advanced

[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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]