Re: [PATCH v4 06/30] qcow2: Add get_l2_entry() and set_l2_entry()

[Vladimir Sementsov-Ogievskiy]

17.03.2020 21:16, Alberto Garcia wrote:
> The size of an L2 entry is 64 bits, but if we want to have subclusters
> we need extended L2 entries. This means that we have to access L2
> tables and slices differently depending on whether an image has
> extended L2 entries or not.
>
> This patch replaces all l2_slice[] accesses with calls to
> get_l2_entry() and set_l2_entry().

and it replaces some l2_table[] as well.

I found one not-updated case, in qcow2-refcount.c:

       ret = bdrv_pwrite_sync(bs->file, l2e_offset,
                              &l2_table[i], sizeof(uint64_t));

But on the other hand, if l2_table will be enhanced somehow, this should
be updated other way, as we don't get l2_entry, but write it...

Also, I don't quite like the naming: you'll update in further patch the 
interface

to be [gs]et_l2_entry and [gs]et_l2_bitmap..

But get_l2_entry, don't return the whole entry, only one half of it, same for 
set_l2_entry...

May be, good to make a comment above [gs]et_l2_entry definitions.

anyway,
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>


--
Best regards,
Vladimir