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
