[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] specs/qcow2: Fix documentation of the compre
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2] specs/qcow2: Fix documentation of the compressed cluster descriptor |
Date: |
Tue, 20 Feb 2018 16:13:39 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
On 02/20/2018 04:03 PM, Eric Blake wrote:
boundary. Technically, it might be possible, but qemu does NOT do that
(again, looking at qcow2_alloc_bytes() - we loop if free_in_cluster <
size) - so we may want to be explicit about this point to prevent
OTHER implementations from creating a compressed cluster that crosses
host cluster boundaries (right now, I can't see
qcow2_decompress_cluster() validating it, though - YIKES).
Aha, I see where I went wrong.
That said, a simple patch to try this:
triggers failures in iotests 122:
--- /home/eblake/qemu/tests/qemu-iotests/122.out 2017-10-06
13:45:25.559279136 -0500
+++ /home/eblake/qemu/tests/qemu-iotests/122.out.bad 2018-02-20
15:54:29.890221575 -0600
@@ -117,8 +117,8 @@
convert -c -S 0:
read 3145728/3145728 bytes at offset 0
3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 63963136/63963136 bytes at offset 3145728
-61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qcow2: Marking image as corrupt: Compressed cluster at 0x5ffd2 crosses
host cluster boundary; further corruption events will be suppressed
+read failed: Input/output error
[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data":
true}]
so it looks like I'm reading qcow2_alloc_bytes() wrong and that we CAN
have a compressed cluster that crosses host cluster boundaries?
We DO allow crossing sector boundaries, IF the newly allocated cluster
is contiguous to the cluster that has the unused tail that we are
starting in. Good, because that's less wasteful of the image (suppose
every compressed cluster got 49% reduction in size - since each one
requires 51% of a cluster, not allowing cluster crossing would require a
full cluster each, rather than the expected ~49% reduction in overall
image size if we are good at contiguous allocations).
So if I may suggest:
x+1 - 61: Number of additional 512-byte sectors used for the
compressed data, beyond the sector containing the
offset in the previous field. These sectors must fit
within the same host cluster.
This sentence needs tweaking to match reality, given that my simple
patch to flag cross-sector hosts triggered (or I need to figure out what
was wrong with my patch).
So change that sentence to: As needed, these additional sectors may
reside in the next contiguous host cluster.
Note that the compressed
data does not necessarily occupy all of the bytes in
the final sector; rather, decompression stops when it
has produced a cluster of data. Another compressed
cluster may map to the tail of the final sector used
by this compressed cluster.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org