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
