On Tue, May 24, 2011 at 1:16 PM, Daniel P. Berrange <berrange@xxxxxxxxxx> 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@xxxxxxxxxx> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list