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

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(a)us.ibm.com&gt; 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 ?