[libvirt] [PATCH 4/4] Support for probing qed image metadata

[Stefan Hajnoczi]

On Fri, Nov 19, 2010 at 11:54 PM, Eric Blake <eblake at redhat.com> wrote:
> On 11/19/2010 09:18 AM, Adam Litke wrote:
>> Implement getBackingStore() for QED images.  The header format is defined in
>> the QED spec: http://wiki.qemu.org/Features/QED .
>>
>
>> +    if (offset + size > buf_size || offset + size < offset)
>> +        return BACKING_STORE_INVALID;
>
> As currently coded, buf_size is at most STORAGE_MAX_HEAD (20*512).
> QED does not appear to have any maximum header size (other than the fact
> that header size is a multiple of cluster size), and permits a cluster
> size of 2**26.
>
> I don't see anything on the QED file format that requires the
> backing_filename to occur within the header clusters (that is, shouldn't
> QED add a file format restriction that
> backing_filename_offset+backing_filename_size must be less than the
> start of the first regular cluster?).
>
> More worrying, I don't see anything in QED that requires the filename to
> occur within the first 10K bytes.  Do we need to add another enum value
> to libvirt's backing store callback routine, to be used when the header
> requests data that lies beyond buf_size but is still feasibly valid, for
> the case where QED designates a backing store location that is beyond
> 10k but still before the start of the first cluster, rather than the
> current approach of just treating it as BACKING_STORE_INVALID?

QED doesn't explicitly restrict
backing_filename_offset/backing_filename_size to just the header
cluster(s).  I think it makes sense to say backing_filename_offset +
backing_filename_size <= header_size * cluster_size and will add it to
the QED spec.

But that doesn't guarantee backing_filename_offset < 10 KB.  The space
after struct QEDHeader is explicitly unstructured so extensions can
put arbitrary data there in a backwards compatible way.

Stefan