[libvirt] [PATCH 3/3] esx: Add volume upload and download to the storage driver

[Eric Blake]

On 07/02/2012 03:44 PM, Matthias Bolte wrote:
> The new METADATA and CONTENT flags allow a caller to tell the ESX
> storage driver which part of a volume should be up- or downloaded
> in case of a two-file VMDK. In case of a one-file VMDK both or no
> flags must be given. In case of non-VMDK volumes such as ISO or
> floppy images no flags must be specified.

These flags would also be useful for other drivers; I've complained in
the past that qcow2 images under the qemu driver should be able to
differentiate between metadata (let download give me the qcow2 headers)
and content (return the data as seen by the guest, effectively
converting from qcow2 to raw in the download).

> ---
>  include/libvirt/libvirt.h.in |   14 +

Before we can accept the new flags, we need to also document their
effect on the functions in src/libvirt.c.  I'd suggest splitting this
commit into two parts - add the new flags and expose them in virsh in
part 1, then implement the flags for esx in part 2.  This matters in
case someone else implements the flags for qemu, then wants to backport
the new flags and the qemu implementation but not the esx implementation.

>  src/Makefile.am              |    1 +
>  src/esx/esx_storage_driver.c |  268 ++++++++++++++++++
>  src/esx/esx_stream.c         |  610 ++++++++++++++++++++++++++++++++++++++++++
>  src/esx/esx_stream.h         |   33 +++
>  src/esx/esx_util.c           |   82 ++++++
>  src/esx/esx_util.h           |    2 +
>  src/esx/esx_vi.c             |   29 ++
>  src/esx/esx_vi.h             |    2 +
>  tools/virsh.c                |   22 ++-

Also, needs documentation in tools/virsh.pod for the new options in virsh.c.


> +++ b/include/libvirt/libvirt.h.in
> @@ -2562,11 +2562,25 @@ virStorageVolPtr        virStorageVolCreateXMLFrom      (virStoragePoolPtr pool,
>                                                           const char *xmldesc,
>                                                           virStorageVolPtr clonevol,
>                                                           unsigned int flags);
> +
> +typedef enum {
> +    VIR_STORAGE_VOL_DOWNLOAD_DEFAULT  = 0,      /* default behavior */

I'm not sure whether it makes sense to define a name for '0'; in some
cases we have done it, in others we have not.  I'm not opposed to the
idea, but we aren't very consistent.

I do, however, like the idea of using the flags argument to support the
needs of ESX in representing things through two files.

I have not closely reviewed this patch for content, so much as offering
this as my first impressions high-level review.

> +
> +        if (STREQLEN(magic, "KDMV", 4)) {
> +            /* It's a one-file VMDK with metadata and content */
> +            if (flags != VIR_STORAGE_VOL_DOWNLOAD_DEFAULT &&
> +                flags != (VIR_STORAGE_VOL_DOWNLOAD_METADATA |
> +                          VIR_STORAGE_VOL_DOWNLOAD_CONTENT)) {
> +                ESX_ERROR(VIR_ERR_INVALID_ARG, "%s",
> +                          _("Non or both of metadata and content flag is required for this volume"));

s/Non/None/

> @@ -1668,6 +1934,8 @@ static virStorageDriver esxStorageDriver = {
>      .volLookupByPath = esxStorageVolumeLookupByPath, /* 0.8.4 */
>      .volCreateXML = esxStorageVolumeCreateXML, /* 0.8.4 */
>      .volCreateXMLFrom = esxStorageVolumeCreateXMLFrom, /* 0.8.7 */
> +    .volDownload = esxStorageVolumeDownload, /* 0.9.14 */
> +    .volUpload = esxStorageVolumeUpload, /* 0.9.14 */

I think consensus was that the next version will be numberd 0.10.0,
thanks to the addition of the parallels driver.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120702/31b17a7c/attachment-0001.sig>