Re: [Qemu-devel] [PATCH 07/22] qcow2-bitmap: introduce auto-loading bitmaps

[Max Reitz]

On 20.10.2016 14:22, Vladimir Sementsov-Ogievskiy wrote:
> On 01.10.2016 19:26, Max Reitz wrote:
>> On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:
>>> Auto loading bitmaps are bitmaps in Qcow2, with AUTO flag set. They are
> 
> [...]
> 
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 08c4ef9..02ec224 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -213,6 +213,11 @@ static int qcow2_read_extensions(BlockDriverState
>> *bs, uint64_t start_offset,
>>               s->bitmap_directory_size =
>>                       bitmaps_ext.bitmap_directory_size;
>>   +            ret = qcow2_read_bitmaps(bs, errp);
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
>> +
>> I think I'd put this directly into qcow2_open(), just like
>> qcow2_read_snapshots(); but that's an optional suggestion.
>>
>> Max
>>
>>
> 
> Snapshots are not header extension.. so it is not the case. Here
> qcow2_read_bitmaps looks like part of header extension loading, and
> header extension fields describe other parts of the extension.. I think
> this is a good point, isn't it?

I said it's optional, so it's optional. :-)

I personally feel like a header extension is just the bit of data in the
qcow2 header. The bitmaps itself are managed by that extension, but not
part of the extension itself. Therefore, I wouldn't load it here but in
the main function qcow2_open().

However, this is a personal opinion and thus it's an optional suggestion
(as I said), so if you disagree (which you apparently do) then don't let
it bother you. :-)

Max

signature.asc
Description: OpenPGP digital signature