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. Berto