On 3/25/20 7:42 AM, Max Reitz wrote:
On 24.03.20 18:42, Eric Blake wrote:
As the feature name table can be quite large (over 9k if all 64 bits
of all three feature fields have names; a mere 8 features leaves only
8 bytes for a backing file name in a 512-byte cluster), it is unwise
to emit this optional header in images with small cluster sizes.

Update iotest 036 to skip running on small cluster sizes; meanwhile,
note that iotest 061 never passed on alternative cluster sizes
(however, I limited this patch to tests with output affected by adding
feature names, rather than auditing for other tests that are not
robust to alternative cluster sizes).

-    /* Feature table */
-    if (s->qcow_version >= 3) {
+    /*
+     * Feature table.  A mere 8 feature names occupies 392 bytes, and
+     * when coupled with the v3 minimum header of 104 bytes plus the
+     * 8-byte end-of-extension marker, that would leave only 8 bytes
+     * for a backing file name in an image with 512-byte clusters.
+     * Thus, we choose to omit this header for cluster sizes 4k and
+     * smaller.

Can’t we do this decision more dynamically?  Like, always omit it when
cluster_size - sizeof(features) < 2k/3k/...?


+     */
+    if (s->qcow_version >= 3 && s->cluster_size > 4096) {

Yes. But when you consider that sizeof(features) is a compile-time constant, it isn't really dynamic for a given qemu release, but rather a different way to spell things; about the only thing it would buy us is that our cutoff window for what cluster size no longer gets the header may automatically shift from 2k to 4k to 8k as future qemu versions add more qcow2 features. If we want to write it like that, which size limit do you propose? Or asked differently, how much space should we reserve for other extension headers + backing file name?

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Reply via email to