On Fri, Apr 08, 2011 at 12:44:26PM -0500, Adam Litke wrote:
> 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.
That's fine, its just useful to know the design can cope with the
concept in the future.
> > 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.
In the case of the virDomainBlock* APIs, we should always just ask
QEMU since we know it has the info, and it doesn't make sense to try
to duplicate it in libvirt (and it isn't even safe todo so for non-raw
files anyway).
If we did the equivalent virStorageVolGetAllocation APIs, then we'd
likely want to have an impl for raw files natively in libvirt, for
use from non-QEMU drivers (LXC, Xen, etc). I wouldn't ever wnat to
support non-raw formats though, so for those we could just shell
out to qemu-img eventually.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list