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