On Fri 21 Aug 2020 03:42:29 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
What are these ifs for?
/* The data (middle) region must be immediately after the
* start region */
if (l2meta_cow_start(m) + m->cow_start.nb_bytes != offset) {
continue;
}
/* The end region must be immediately after the data (middle)
* region */
if (m->offset + m->cow_end.offset != offset + bytes) {
continue;
}
How is it possible that data doesn't immediately follow start cow
region or end cow region doesn't immediately follow data region?
They are sanity checks. They maybe cannot happen in practice and in
that case I suppose they should be replaced with assertions but this
should be checked carefully. If I remember correctly I was wary of
overlooking a case where this could happen.
In particular, that function receives only one data region but a list
of QCowL2Meta objects. I think you can get more than one QCowL2Meta
if the same request involves a mix of copied and newly allocated
clusters, but that shouldn't be a problem either.
OK, thanks. So, intuitively it shouldn't happen, but there should be
some careful investigation to change them to assertions.
I was having a look at this and here's a simple example of how this can
happen:
qemu-img create -f qcow2 -o cluster_size=1k img.qcow2 1M
qemu-io -c 'write 0 3k' img.qcow2
qemu-io -c 'discard 0 1k' img.qcow2
qemu-io -c 'discard 2k 1k' img.qcow2
qemu-io -c 'write 512 2k' img.qcow2
The last write request can be performed with one single write operation
but it needs to allocate clusters #0 and #2.
This means that merge_cow() is called with offset=512, bytes=2k and two
QCowL2Meta structures:
- The first one with cow_start={0, 512} and cow_end={1k, 0}
- The second one with cow_start={2k, 0} and cow_end={2560, 512}
In theory it should be possible to combine both into one that has
cow_start={0, 512} and cow_end={2560, 512}, but I don't think this
situation happens very often so I wouldn't go that way.
In any case, the checks have to remain and they cannot be turned into
assertions.