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
