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

Re: [libvirt] [PATCH 2/5] Add public APIs for storage volume upload/download



On 03/15/2011 06:30 AM, Daniel P. Berrange wrote:
> New APIs are added allowing streaming of content to/from
> storage volumes. A new API for creating volumes is also
> added allowing the content to be provided immediately at
> time of creation
> 
> * include/libvirt/libvirt.h.in: Add virStorageVolUpload and
>   virStorageVolDownload, virStorageVolCreateUpload APIs

I see virStorageVol{Up,Down}load, but not virStorageVolCreateUpload in
this patch.

> * src/driver.h, src/libvirt.c, src/libvirt_public.syms: Stub
>   code for new APIs
> * src/storage/storage_driver.c, src/esx/esx_storage_driver.c:
>   Add dummy entries in driver table for new APIs
> ---
>  include/libvirt/libvirt.h.in |   10 ++++
>  include/libvirt/virterror.h  |    1 +
>  src/driver.h                 |   14 +++++
>  src/esx/esx_storage_driver.c |    2 +
>  src/libvirt.c                |  120 ++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms      |    2 +
>  src/storage/storage_driver.c |    2 +
>  src/util/virterror.c         |    6 ++
>  8 files changed, 157 insertions(+), 0 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index eaeccd6..17ed09d 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1499,6 +1499,16 @@ virStorageVolPtr        virStorageVolCreateXMLFrom      (virStoragePoolPtr pool,
>                                                           const char *xmldesc,
>                                                           virStorageVolPtr clonevol,
>                                                           unsigned int flags);
> +int                     virStorageVolDownload           (virStorageVolPtr vol,
> +                                                         virStreamPtr stream,
> +                                                         unsigned long long offset,
> +                                                         unsigned long long length,
> +                                                         unsigned int flags);
> +int                     virStorageVolUpload             (virStorageVolPtr vol,
> +                                                         virStreamPtr stream,
> +                                                         unsigned long long offset,
> +                                                         unsigned long long length,
> +                                                         unsigned int flags);
>  int                     virStorageVolDelete             (virStorageVolPtr vol,

Are you missing a function declaration?

> +++ b/src/driver.h
> @@ -899,6 +899,18 @@ typedef virStorageVolPtr
>                                                const char *xmldesc,
>                                                virStorageVolPtr clone,
>                                                unsigned int flags);
> +typedef int
> +    (*virDrvStorageVolDownload) (virStorageVolPtr vol,
> +                                 virStreamPtr stream,
> +                                 unsigned long long offset,
> +                                 unsigned long long length,
> +                                 unsigned int flags);
> +typedef int
> +    (*virDrvStorageVolUpload) (virStorageVolPtr vol,
> +                               virStreamPtr stream,
> +                               unsigned long long offset,
> +                               unsigned long long length,
> +                               unsigned int flags);
>  

Makes sense that the drivers only need two callbacks, even if you add
three public functions, since the CreateUpload can piece together
multiple driver callbacks.

> +++ b/src/libvirt.c
> @@ -9059,6 +9059,126 @@ error:
>  
>  
>  /**
> + * virStorageVolDownload:
> + * @pool: pointer to volume to download
> + * @stream: stream to use as output
> + * @offset: position to start reading from
> + * @length: limit on amount of data to download

Does 0 (or UINT64_MAX) have a special meaning of read-to-end?  If so,
document that.

> +
> +    if (vol->conn->flags & VIR_CONNECT_RO ||
> +        stream->conn->flags & VIR_CONNECT_RO) {
> +        virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> +        goto error;
> +    }

Why is reading a volume prohibited on a RO connection?

> +/**
> + * virStorageVolUpload:
> + * @pool: pointer to volume to download
> + * @stream: stream to use as output
> + * @offset: position to start writing to
> + * @length: limit on amount of data to upload

Again, is there any special meaning of length 0, or a directive to
easily specify to end of input?

> + * @flags: flags for creation (unused, pass 0)

Do we need a flag to allow for expansion of the volume if the input
stream (and length) are larger than the current size of the volume, for
volumes that can be expanded in size?

> +
> +    if (vol->conn->flags & VIR_CONNECT_RO ||
> +        stream->conn->flags & VIR_CONNECT_RO) {
> +        virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> +        goto error;
> +    }

Agree here that RO connection should not modify volumes.

> +++ b/src/util/virterror.c
> @@ -1198,6 +1198,12 @@ virErrorMsg(virErrorNumber error, const char *info)
>              else
>                  errmsg = _("Domain snapshot not found: %s");
>              break;
> +        case VIR_ERR_INVALID_STREAM:
> +            if (info == NULL)
> +                errmsg = _("invalid stream pointer in");

s/ in//

-- 
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]