Re: [PATCH v2 3/4] qcow2: Avoid feature name extension on small cluster size

[Eric Blake <eblake@redhat.com>]

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