On Fri, Nov 19, 2010 at 11:54 PM, Eric Blake <eblake@xxxxxxxxxx> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list