|
From: | Eric Blake |
Subject: | Re: [PATCH v2 3/4] qcow2: Avoid feature name extension on small cluster size |
Date: | Wed, 25 Mar 2020 09:01:42 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 |
On 3/25/20 8:52 AM, Max Reitz wrote:
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?Well, that was the “2k/3k/...” list. :) The backing file name is limited to 1k, so I suppose that plus 1k for a potential external data filename, plus 1k for the rest, so 3k in total? Now that I look into the spec, I see that it actually doesn’t say that the backing filename has to be part of the header cluster. But, well.
qemu enforces that the header is one cluster. But you're right, that does not appear to directly be a limitation mandated by the spec, and we could relax qemu to allow the header to be several consecutive clusters. The tricky part, however, is that the backing file name is NOT described by a header extension, but rather is just whatever bytes occur after the final header extension. There's no clear indication anywhere on when to stop reading those bytes, other than by an implicit limit such as insisting those bytes fall within the first cluster.
Had we been smarter when designing v3, we would have made the backing file name a header extension (in fact, it would have been possible to design the additional fields of v3 to look like an unknown header extension when parsed by a v2 binary) - but hindsight is 20/20.
It also only says that the image header must be part of the first cluster, which in my opinion doesn’t necessarily include its extensions. So header extensions (and the backing filename) could actually be in consecutive clusters. But that of course would be much more difficult to implement.
We'd still want a sane limit even for small-cluster images (maybe "no more than 2M of header information, regardless of cluster size); or maybe even introduce a NEW header field and/or extension to make it obvious how many clusters are being used for the purpose of the metadata header in this particular image (with sane fallbacks for when that extension is not present). But you're right - it comes with complexity. This patch as written is safe for 5.0-rc1, but this discussion about teaching qemu to permit headers larger than 1 cluster is squarely in the 5.1 category, if at all.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[Prev in Thread] | Current Thread | [Next in Thread] |