Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

[Vladimir Sementsov-Ogievskiy]

24.04.2020 20:56, Alberto Garcia wrote:
> On Fri 24 Apr 2020 07:44:33 PM CEST, Eric Blake <address@hidden> wrote:
> > > > at the same time, I can see where you're coming from in stating that
> > > > if it makes management of extended L2 easier to allow zero subclusters
> > > > on top of a compressed cluster, then there's no reason to forbid it.
> > >
> > > I'm not sure if it makes it easier. Some operations are definitely going
> > > to be easier but maybe we have to add and handle _ZERO_COMPRESSED in
> > > addition to _ZERO_PLAIN and _ZERO_ALLOC (the same for unallocated
> > > subclusters). Or maybe replace QCow2SubclusterType with something
> > > else. I need to evaluate that.

Reviewing your series it already came in my mind, that we are doing too much with the conversion from l2e 
flags to "type". Does it worth it? All these ZERO_PLAIN and UNALLOCATED_ALLOC, and "case 
<TYPE>:" lines combined by three-four into one case, do they help, or is it an extra work? We 
just have to maintain two views of one model.. But I don't suggest to refactor it in these series :)

> > Reading the entire cluster will be interesting - you'll have to
> > decompress the entire memory, then overwrite the zeroed portions.
>
> I don't think so, qcow2_get_host_offset() would detect the number of
> contiguous subclusters of the same type at the given offset. In this
> case they would be _ZERO subclusters so there's no need to decompress
> anything, or even read it (it works the same with uncompressed
> clusters).

But if at least one of subclusters to read is not _ZERO, you'll have to 
decompress the whole cluster, and after decompression rewrite zero-subclusters 
by zeroes, as Eric says.. Or I lost the thread:)


--
Best regards,
Vladimir