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

[Daniel P. Berrange]

On Thu, Apr 07, 2011 at 04:31:59PM -0500, Adam Litke wrote:
>  /*
> + * Disk Streaming
> + */
> +typedef enum {
> +    VIR_STREAM_DISK_ONE   = 1,  /* Stream a single disk unit */
> +    VIR_STREAM_DISK_START = 2,  /* Stream the entire disk */
> +    VIR_STREAM_DISK_STOP  = 4,  /* Stop streaming a disk */
> +} virDomainStreamDiskFlags;

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.

> +
> +#define VIR_STREAM_PATH_BUFLEN 1024
> +#define VIR_STREAM_DISK_MAX_STREAMS 10
> +
> +typedef struct _virStreamDiskState virStreamDiskState;
> +struct _virStreamDiskState {
> +    char path[VIR_STREAM_PATH_BUFLEN];
> +    /*
> +     * The unit of measure for size and offset is unspecified.  These fields
> +     * are meant to indicate the progress of a continuous streaming operation.
> +     */
> +    unsigned long long offset; /* Current offset of active streaming */
> +    unsigned long long size;   /* Disk size */
> +};
> +typedef virStreamDiskState *virStreamDiskStatePtr;
> +
> +unsigned long long       virDomainStreamDisk(virDomainPtr dom,
> +                                             const char *path,
> +                                             unsigned long long offset,
> +                                             unsigned int flags);
> +
> +int                      virDomainStreamDiskInfo(virDomainPtr dom,
> +                                                 virStreamDiskStatePtr states,
> +                                                 unsigned int nr_states,
> +                                                 unsigned int flags);

I would have liked it if we could use the existing JobInfo APIs for
getting progress information, but if we need to allow concurrent
usage for multiple disks per-VM, we can't. I think we should still
use a similar style of API though.

There also doesn't appear to be a precise way to determine if the
copying of an entire disk failed part way through, and if so, how
much was actually copied.

Taking all the previous points together, I think the API needs to
look like this:

    typedef enum {
        /* If set, virDomainBlockAllocate() will return immediately
         * allowing polling for operation completion & status
         */
        VIR_DOMAIN_DISK_ALLOCATE_NONBLOCK,
    } virDomainBlockAllocateFlags;

    /*
     * @path: fully qualified filename of the virtual disk
     * @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 @path is fully allocated
     * between @offset and @offset+ at length. If a backing
     * store is present, data will be filled from the
     * backing store, otherwise data will be fileld with
     * zeros
     *
     * If @flags contains VIR_DOMAIN_DISK_ALLOCATE_NONBLOCK,
     * this API will return immediately after initiating the
     * copy, otherwise it will block until copying is complete
     *
     */
    int virDomainBlockAllocate(virDomainPtr dom,
                               const char *path,
                               unsigned long long offset,
                               unsigned long long length,
                               unsigned int flags);


    /*
     * @path: fully qualified filename of the virtual disk
     * @info: allocated struct to return progress info
     *
     * Query the progress of a disk allocation job. This
     * API must be used when virDomainBlockAllocate() was
     * invoked with the VIR_DOMAIN_DISK_ALLOCATE_NONBLOCK
     * flag set.
     *
     * The @info.type field will indicate whether the job
     * was completed successfully, or failed part way
     * through.
     *
     * The @info data progress fields will contain current
     * progress information.
     *
     * The hypervisor driver may optionally chose to also
     * fillin a time estimate for completion.
     */
    int virDomainBlockGetJobInfo(virDomainPtr dom,
                                 const char *path,
                                 virDomainJobInfoPtr info);

    /*
     * @path: fully qualified filename of the virtual disk
     *
     * Request that a disk allocation job be aborted at
     * the soonest opportunity. This API can only be used
     * when virDomainBlockAllocate() was invoked with the
     * VIR_DOMAIN_DISK_ALLOCATE_NONBLOCK flag set.
     */
    int virDomainBlockAbortJob(virDomainPtr dom,
                               const char *path);


    typedef struct _virDomainBlockRegion virDomainBlockRegion;
    typedef virDomainBlockRegion *virDomainBlockRegionPtr;
    struct _virDomainBlockRegion {
        /* The logical offset within the file of the allocated region */
        unsigned long long offset;
        /* The length of the allocated region */
        unsigned long long length;
    };

    /*
     * @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);

NB, I've used 'Block' rather than 'Disk' in the API names,
since all the other APIs we have dealing with disks in a
guest, use  'Block' in their name. An unfortunate naming
convention, but now we have it, we should stick with it
consistently.

This takes care of things for running guests. It would be
desirable to have the same functionality available when a
guest is not running, via the virStorageVol APIs. Indeed,
this would allow access to the allocation functionality
for disks not explicitly associated with any VM yet.

Basically the API design would be identical, but instead of
using

  virDomainBlockXXXX(virDomainPtr dom,
                     const char *path,
                     ....);

we'd have

  virStorageVolXXXX(virStorageVolPtr vol,
                    ....);


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|