[libvirt] API Proposal: virDomainBlockPull() (Block device streaming) V4

[Daniel P. Berrange]

On Tue, May 24, 2011 at 01:00:04PM +0100, Stefan Hajnoczi wrote:
> On Mon, May 23, 2011 at 5:56 PM, Adam Litke <agl at us.ibm.com> wrote:
> > /**
> >  * virDomainBlockPull:
> >  * @dom: pointer to domain object
> >  * @path: Fully-qualified filename of disk
> >  * @cursor: pointer to a virDomainBlockPullCursor, or NULL
> >  * @flags: One of virDomainBlockPullFlags, or zero
> >  *
> >  * Populate a disk image with data from its backing image.  Once all data from
> >  * its backing image has been pulled, the disk no longer depends on a backing
> >  * image.
> >  *
> >  * If VIR_DOMAIN_BLOCK_PULL_CONTINUOUS is specified, the entire device will be
> >  * streamed in the background.  Otherwise, the value stored in @cursor will be
> >  * used to stream an increment.
> >  *
> >  * Returns -1 in case of failure, 0 when successful.  On success and when @flags
> >  * does not contain VIR_DOMAIN_BLOCK_PULL_CONTINUOUS, the iterator @cursor will
> >  * be updated to the proper value for use in a subsequent call.
> >  */
> > int                 virDomainBlockPull(virDomainPtr dom,
> >                                       const char *path,
> >                                       virDomainBlockPullCursor *cursor,
> >                                       unsigned int flags);
> 
> If this function is used without VIR_DOMAIN_BLOCK_PULL_CONTINUOUS then
> the "end" value is unknown.  Therefore it is not possible to calculate
> streaming progress.  Perhaps instead of cursor we need a
> virDomainBlockStreamInfoPtr info argument?

It almost feels like we should just not overload semantics using
flags and have a separate APIs:

Incremental, just iterate on:

 int virDomainBlockPull(virDomainPtr dom,
                        const char *path,
                        virDomainBlockPullInfoPtr *pos,
                        unsigned int flags);

Continuous, invoke once:

 int virDomainBlockPullAll(virDomainPtr dom,
                           const char *path,
                           unsigned int flags);

 ...and wait for the async event notification for completion, or
optionally poll on virDomainGetBlockPullInfo, or use BlockPullAbort()


> 
> > NOTE: Qemu will emit an asynchronous event (VIR_DOMAIN_BLOCK_PULL_COMPLETED)
> > after any operation that eliminates the dependency on the backing file.  This
> > could be a virDomainBlockPull() without VIR_DOMAIN_BLOCK_PULL_CONTINUOUS and
> > will allow libvirt to manage backing file security regardless of the pull mode
> > used.
> 
> Currently QEMU does not change the backing file when incremental
> streaming is used (instead of continuous).  This is because it is hard
> to know whether or not the entire file has finished streaming - it's
> an operation that requires traversing all block allocation metadata to
> check that the backing file is no longer necessary.

Having different semantics for what happens at the end of streaming
for incremental vs continuous is a really bad idea. I assume this
limitation will be fixed in QEMU before streaming is finally merged ?

Regards
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 :|