qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v3 3/3] block/stream: introduce a bottom node


From: Andrey Shinkevich
Subject: Re: [Qemu-block] [PATCH v3 3/3] block/stream: introduce a bottom node
Date: Mon, 8 Apr 2019 18:17:37 +0000


On 08/04/2019 18:39, Alberto Garcia wrote:
> On Fri 05 Apr 2019 06:56:19 PM CEST, Andrey Shinkevich wrote:
>> @@ -232,8 +232,13 @@ void stream_start(const char *job_id, BlockDriverState 
>> *bs,
>>       StreamBlockJob *s;
>>       BlockDriverState *iter;
>>       bool bs_read_only;
>> +    BlockDriverState *bottom = NULL;
>> +    int basic_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
>> +
>> +    /* Find the bottom node that has the base as its backing image */
>> +    bottom = bdrv_find_overlay(bs, base);
> 
> What happens if bs == base here ??
> 

Actually, that condition is checked in the qmp_block_stream().
I used to have the 'assert(bottom)' after the call to the
bdrv_find_overlay(bs, base) in the qmp_block_stream() of the v2.

>> +    /*
>> +     * Block all intermediate nodes between bs and bottom (inclusive), 
>> because
>> +     * they will disappear from the chain after this operation. The 
>> streaming
>> +     * job reads every block only once, assuming that it doesn't change, so
>> +     * forbid writes and resizes.
>> +     */
>> +    for (iter = bs; iter != bottom; iter = backing_bs(iter)) {
>> +        block_job_add_bdrv(&s->common, "intermediate node", 
>> backing_bs(iter),
>> +                           0, basic_flags, &error_abort);
>>       }
> 
> This stops when iter == bottom, so bottom is not actually blocked.

The bottom is actually blocked because
backing_bs(iter) == bottom is passed to the block_job_add_bdrv()
with the last iteration of the loop.

> 
>> diff --git a/block/trace-events b/block/trace-events
>> index 7335a42..5366d45 100644
>> --- a/block/trace-events
>> +++ b/block/trace-events
>> @@ -20,7 +20,7 @@ bdrv_co_copy_range_to(void *src, uint64_t src_offset, void 
>> *dst, uint64_t dst_of
>>   
>>   # stream.c
>>   stream_one_iteration(void *s, int64_t offset, uint64_t bytes, int 
>> is_allocated) "s %p offset %" PRId64 " bytes %" PRIu64 " is_allocated %d"
>> -stream_start(void *bs, void *base, void *s) "bs %p base %p s %p"
>> +stream_start(void *bs, void *bottom, void *s) "bs %p bottom %p s %p"
> 
> Is this change still correct? We don't pass bottom anymore.
> 
> Berto
> 

-- 
With the best regards,
Andrey Shinkevich

reply via email to

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