[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/6] Add new API virDomainStreamDisk[Info] to header and drivers



On 05/02/2011 03:29 PM, Adam Litke wrote:
> After several long distractions, I am back to working on disk streaming.
> Before I hit the list with a new series of patches, I was hoping to
> reach some middle ground on the proposed streaming API.
> 
>> Using flags to combine two separate tasks into one single API
>> is rather unpleasant. As raised in the previous patch, the API
>> should also be taking a offset+length in bytes, then there is
>> no need for a special case transfer of an individual sector.
> 
> This is a fair point.  I will work with Stefan to support an
> offset/length qemu API.  Since there doesn't seem to be a great way to
> query device sizes, I think we do need a convenient way to request that
> the entire disk be streamed.  We could either do this with a flag or by
> overriding (offset==0 && length==0) to mean stream the entire device.
> Do you have a preference?

I'm personally okay with length=0 as a special case (after all, we
special-case length=0 for 'virsh vol-upload' as meaning the rest of the
file.

>>     /*
>>      * @path: fully qualified filename of the virtual disk
>>      * @nregions: filled in the number of @region structs
>>      * @regions: filled with a list of allocated regions
>>      *
>>      * Query the extents of allocated regions within the
>>      * virtual disk file. The offsets in the list of regions
>>      * are not guarenteed to be sorted in any explicit order.
>>      */
>>     int virDomainBlockGetAllocationMap(virDomainPtr dom,
>>                                        const char *path,
>>                                        unsigned int *nregions,
>>                                        virDomainBlockRegionPtr *regions);
> 
> I am not convinced that we really need the block allocation map stuff.

Maybe you aren't, but our solution should not preclude adding that at a
later point if someone else can find a good use for it.  And I think
that for offline volume cloning, it could very well be useful to know
which blocks are allocated.

> It's a fun toy, but the layout of data within a device is far too
> low-level of a concept to be exposing to users.

Never underestimate your users :)

>     typedef enum {
>          /* If set, virDomainBlockAllocate() will return immediately
>           * allowing polling for operation completion & status
>           */
>          VIR_DOMAIN_DISK_STREAM_NONBLOCK,
>      } virDomainBlockStreamFlags;
> 
>      /*
>       * @device: alias of the target block device
>       * @offset: logical position in bytes, within the virtual disk
>       * @length: amount of data to copy, in bytes starting from @offset
>       * @flags: One of virDomainBlockAllocateFlags, or zero
>       *
>       * Ensure the virtual disk @device is fully allocated
>       * between @offset and @offset+ length  If a backing
>       * store is present, data will be filled from the
>       * backing store, otherwise data will be fileld with

s/fileld/filled/

>       * zeros.
>       *
>       * If @flags contains VIR_DOMAIN_DISK_STREAM_NONBLOCK,
>       * this API will return immediately after initiating the
>       * copy, otherwise it will block until copying is complete
>       *
>       */
>      int virDomainBlockStream(virDomainPtr dom,
>                               const char *device,
>                               unsigned long long offset,
>                               unsigned long long length,
>                               unsigned int flags);
> 
>      /*
>       * @device: alias of the target block device
>       *
>       * Request that a disk streaming job be aborted at
>       * the soonest opportunity. This API can only be used
>       * when virDomainBlockStream() was invoked with the
>       * VIR_DOMAIN_DISK_STREAM_NONBLOCK flag set.
>       */
>      int virDomainBlockStreamAbort(virDomainPtr dom,
>                                    const char *device);
> 
>      typedef enum {
>          VIR_BLOCK_STREAM_STATE_COMPLETED = 0,
>          VIR_BLOCK_STREAM_STATE_ACTIVE = 1,
>          VIR_BLOCK_STREAM_STATE_FAILED = 2,
>      } virBlockStreamStatus;
> 
>      typedef struct _virBlockStreamInfo virBlockStreamInfo;
>      struct _virBlockStreamInfo {
>          virBlockStreamStatus status;
>          unsigned long long remaining; /* number of bytes remaining */ 
>      };
>      typedef virBlockStreamInfo *virBlockStreamInfoPtr;

Looks useful.  It's up to the user to remember the offset they started
at and the length they requested, to be able to definitively state what
offset is currently being worked on according to the information in the
info->remaining field.

> 
>      /*
>       * @device: alias of the target block device
>       * @info: allocated struct to return progress info
>       *
>       * Query the progress of a disk streaming job. This
>       * API must be used when virDomainBlockStream() was
>       * invoked with the VIR_DOMAIN_DISK_STREAM_NONBLOCK
>       * flag set.
>       *
>       */
>      int virDomainBlockStreamInfo(virDomainPtr dom,
>                                   const char *device,
>                                   virBlockStreamInfoPtr info);
> 

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]