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

[Stefan Hajnoczi]

On Tue, May 24, 2011 at 1:16 PM, Daniel P. Berrange <berrange at redhat.com> wrote:
> 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 ?

Agreed, there needs to be consistent semantics and I will solve it in QEMU.

Stefan