Re: [libvirt] [PATCH 1/6] Add new API virDomainStreamDisk[Info] to header and drivers

On Fri, 2011-04-08 at 11:13 -0600, Eric Blake wrote: > On 04/08/2011 07:31 AM, Daniel P. Berrange wrote: > > On Thu, Apr 07, 2011 at 04:31:59PM -0500, Adam Litke wrote: > >> /* > >> + * Disk Streaming > >> + */ > >> +typedef enum { > >> + VIR_STREAM_DISK_ONE = 1, /* Stream a single disk unit */ > >> + VIR_STREAM_DISK_START = 2, /* Stream the entire disk */ > >> + VIR_STREAM_DISK_STOP = 4, /* Stop streaming a disk */ > >> +} virDomainStreamDiskFlags; > > > > Using flags to combine two separate tasks into one single API > > is rather unpleasant. As raised in the previous patch, the API > > should also be taking a offset+length in bytes, then there is > > no need for a special case transfer of an individual sector. > > > Taking all the previous points together, I think the API needs to > > look like this: > > > > typedef enum { > > /* If set, virDomainBlockAllocate() will return immediately > > * allowing polling for operation completion & status > > */ > > VIR_DOMAIN_DISK_ALLOCATE_NONBLOCK, > > } virDomainBlockAllocateFlags; > > It seems like adding one more flag would also make this API useful for > supporting the converse operation: if we have a disk that is currently > allocated, but we either know that a block is all 0s (or don't care > about the data in that block if it was not all 0s), it would be nice to > request punching a hole (for file-backed images residing on a file > system and kernels new enough to do that) and/or truncate back to a > smaller (thinly-provisioned) allocated size (which should work for both > file-backed and lvm-backed disk images). I agree that this could be a good future extension of the API and further justifies the use of offset _and_ length parameters. However, I'd prefer not to consider this part at the moment since I am not aware of a hypervisor that plans to implement it.

> Meanwhile, I know that GNU coreutils has been working on an API for > efficiently getting FIEMAP data from files; part of this effort needs to > involve migrating that code into gnulib so that libvirt can indeed > provide an API that enumerates sections of a disk image that are > allocated vs. holes. Why not just ask the hypervisor? Qemu's image format code is probably the most efficient place from which to gather this information.