[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]