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

On 05/24/2011 07:19 AM, Daniel P. Berrange wrote: > On Mon, May 23, 2011 at 11:56:03AM -0500, Adam Litke wrote: >> Here is version 4 of the BlockStream API (now called BlockPull). Hopefully this >> version addresses all remaining concerns and I can begin to work on the code. >> Does everyone approve of the new name 'virDomainBlockPull'? >> >> >> Changes since V3: >> - Renamed the API to 'Block Pull' to emphasize the effect of the API over its >> method of operation. >> - Changed virDomainGetBlockPullInfo() to accept a path argument and to return >> information about that path alone. >> - Added a new virDomainEvent to report final status when using CONTINUOUS mode. >> >> /* >> * Block Pull API >> */ >> typedef enum { >> /* >> * If set, virDomainBlockPull() will operate on the entire device in the >> * background. The status can be checked and the operation aborted by >> * using the functions virDomainBlockPullInfo() and >> * virDomainBlockPullAbort(). >> */ >> VIR_DOMAIN_BLOCK_PULL_CONTINUOUS = 1, >> } virDomainBlockPullFlags; >> >> /* An iterator for initiating and monitoring block pull operations */ >> typedef unsigned long long virDomainBlockPullCursor; >> >> typedef struct _virDomainBlockPullInfo virDomainBlockPullInfo; >> struct _virDomainBlockPullInfo { >> /* >> * The following fields provide an indication of block pull progress. @cur >> * indicates the current position and will be between 0 and @end. @end is >> * the final cursor position for this operation and represents completion. >> * To approximate progress, divide @cur by @end. >> */ >> virDomainBlockPullCursor cur; >> virDomainBlockPullCursor end; >> }; >> typedef virDomainBlockPullInfo *virDomainBlockPullInfoPtr; >> >> /** >> * 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); >> >> /** >> * virDomainBlockPullAbort: >> * @dom: pointer to domain object >> * @path: fully-qualified filename of disk >> * @flags: currently unused, for future extension >> * >> * Cancel a pull operation that has been previously started by a call to >> * virDomainBlockPull() with the VIR_DOMAIN_BLOCK_PULL_CONTINUOUS flag. >> * >> * Returns -1 in case of failure, 0 when successful. >> */ >> int virDomainBlockPullAbort(virDomainPtr dom, >> const char *path, >> unsigned int flags); >> >> /** >> * virDomainGetBlockPullInfo: >> * @dom: pointer to domain object >> * @path: fully-qualified filename of disk >> * @info: pointer to a virDomainBlockPullInfo structure >> * @flags: currently unused, for future extension >> * >> * Request progress information on a block pull operation that has been started >> * with the VIR_DOMAIN_BLOCK_PULL_CONTINUOUS flag set. If an operation is >> * active for the given parameters, @info will be updated with the current >> * progress. >> * >> * Returns -1 in case of failure, 0 when successful. >> */ >> int virDomainGetBlockPullInfo(virDomainPtr dom, >> const char *path, >> virDomainBlockStreamInfoPtr info, >> unsigned int flags); >> >> >> The following new asynchronous event will be made available for subscription: >> >> VIR_DOMAIN_EVENT_ID_BLOCK_PULL = 7, >> >> typedef enum { >> VIR_DOMAIN_BLOCK_PULL_COMPLETED, >> VIR_DOMAIN_BLOCK_PULL_FAILED, >> } virConnectDomainEventBlockPullStatus; >> >> typedef void (*virConnectDomainEventBlockPullCallback(virConnectPtr conn, >> virDomainPtr dom, >> const char *path, >> int status); >> >> Upon receipt of this event and when the status field indicates success, libvirt >> will revoke access to the backing file which is no longer needed by the domain. >> >> 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. > > I assume it will also emit VIR_DOMAIN_BLOCK_PULL_FAILED when a continuous > streaming operation aborts, because in that case you obviously still have > the backing file present so PULL_COMPLETED wouldn't be issued ? I don't think abort is the same as failed in this case. An abort is triggered by the API user whereas a failure is not. If a user issues an abort, they should know to stop waiting around for PULL_COMPLETED. A failure is unexpected and thus absolutely needs an event to be raised.