I need to clarify this a bit... On 05/24/2011 08:06 AM, Adam Litke wrote: > > > On 05/24/2011 07:16 AM, Daniel P. Berrange 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, > > We don't even need 'pos' anymore. See below. We still will want 'pos', but it changes from an in/out parameter to out-only. Qemu tracks the position internally so this is just for progress reporting. > >> 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() > > Yeah, despite adding four functions to the API this seems like a natural > way to segment it out. > >>>> 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 ? > > After talking to Stefan I think we have resolved this issue. We have > decided to drop the 'pos' argument to virDomainBlockPull() completely. > If pos is a sequential, opaque cursor then there is only ever one valid > value that can be passed to qemu at any given time. If qemu has to > store the valid value to check against, then we might as well just store > the position internally to qemu and save the API user the trouble of > managing the iterator. An added benefit is that we now know it is safe > to know that the entire disk has been pulled after the last cursor > position has finished (regardless of which mode was used). As stated above, 'pos' remains in the API but becomes an out parameter. > I will follow-up with V5 of the API... -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list