On Fri, Nov 19, 2010 at 11:54 PM, Eric Blake <eblake@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