[libvirt] [glib PATCH V2] Add bindings for virStorageDownload() and virStorageUpload()
Zeeshan Ali (Khattak)
zeeshanak at gnome.org
Sat Jul 14 02:46:18 UTC 2012
Hi,
Looks pretty stright-forward and correct. Some comments below. I'm
already pushing it with these issues fixed.
On Fri, Jul 13, 2012 at 10:37 AM, Jovanka Gulicoska
<jovanka.gulicoska at gmail.com> wrote:
> ---
> libvirt-gobject/libvirt-gobject-storage-vol.c | 85 +++++++++++++++++++++++++
> libvirt-gobject/libvirt-gobject-storage-vol.h | 14 ++++
> libvirt-gobject/libvirt-gobject.h | 1 +
> libvirt-gobject/libvirt-gobject.sym | 2 +
> 4 files changed, 102 insertions(+)
>
> diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c
> index 6f60fcd..462a99e 100644
> --- a/libvirt-gobject/libvirt-gobject-storage-vol.c
> +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c
> @@ -349,3 +349,88 @@ gboolean gvir_storage_vol_resize(GVirStorageVol *vol,
>
> return TRUE;
> }
> +
> +/**
> + * gvir_storage_vol_download:
> + * @vol: the storage volume to download from
> + * @stream: stream to use as output
> + * @offset: position in @vol to start reading from
> + * @length: limit on amount of data to download
You want to document here that "If @length is zero, then the remaining
contents of the volume after @offset will be downloaded" (c&p from
libvirt docs).
> + * @flags: extra flags, not used yet, pass 0
> + *
> + * Returns: #TRUE of success, #FALSE otherwise
> + */
> +gboolean gvir_storage_vol_download(GVirStorageVol *vol,
> + GVirStream *stream,
> + unsigned long long offset,
> + unsigned long long length,
You want to use guint64 for both these arguments here. GIR actually
ignores both these new functions and we don't get them into the vala
API because of this.
> + guint flags,
> + GError **err)
> +{
> + virStreamPtr st = NULL;
Better name it 'stream_handle'.
> + gboolean ret = FALSE;
> +
> + g_object_get(stream, "handle", &st, NULL);
> +
> + g_return_val_if_fail(GVIR_IS_STORAGE_VOL(vol), FALSE);
> + g_return_val_if_fail(GVIR_IS_STREAM(stream), FALSE);
> + g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
> +
> + if (virStorageVolDownload(vol->priv->handle, st, offset, length, 0) < 0) {
This needs breaking to fit within 80 columns.
> + gvir_set_error_literal(err,
> + GVIR_STORAGE_VOL_ERROR,
> + 0,
> + "Unable to downlaod volume storage");
> +
> + goto cleanup;
> + }
> +
> + ret = TRUE;
> +cleanup:
> + if (st != NULL)
> + virStreamFree(st);
> + return ret;
> +}
> +
> +/**
> + * gvir_storage_vol_upload:
> + * @vol: the storage volume to upload
> + * @stream: stream to use as input
> + * @offset: position in @vol to start to write to
> + * @length: limit on amount of data to upload
Same documentation needed here as well.
> + * @flags: the flags, not set yet, pass 0
> + *
> + * Returns: #TRUE of success, #FALSE otherwise
> + */
> +gboolean gvir_storage_vol_upload(GVirStorageVol *vol,
> + GVirStream *stream,
> + unsigned long long offset,
> + unsigned long long length,
guint64 for these too.
> + guint flags,
> + GError **err)
> +{
> + virStreamPtr st = NULL;
You want to rename this too.
> + gboolean ret = FALSE;
> +
> + g_object_get(stream, "handle", &st, NULL);
> +
> + g_return_val_if_fail(GVIR_IS_STORAGE_VOL(vol), FALSE);
> + g_return_val_if_fail(GVIR_IS_STREAM(stream), FALSE);
> + g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
> +
> + if (virStorageVolUpload(vol->priv->handle, st, offset, length, 0) < 0) {
This one also needs breaking.
> + gvir_set_error_literal(err,
> + GVIR_STORAGE_VOL_ERROR,
> + 0,
> + "Unable to upload to stream");
> +
> + goto cleanup;
> + }
> +
> + ret = TRUE;
> +cleanup:
> + if (st != NULL)
> + virStreamFree(st);
> + return ret;
> +}
> +
Git doesn't like empty newlines at the end of file and issues warnings
so better just avoid them. :)
> diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.h b/libvirt-gobject/libvirt-gobject-storage-vol.h
> index b425f0a..e156792 100644
> --- a/libvirt-gobject/libvirt-gobject-storage-vol.h
> +++ b/libvirt-gobject/libvirt-gobject-storage-vol.h
> @@ -110,6 +110,20 @@ gboolean gvir_storage_vol_resize(GVirStorageVol *vol,
> guint flags,
> GError **err);
>
> +gboolean gvir_storage_vol_download(GVirStorageVol *vol,
> + GVirStream *stream,
> + unsigned long long offset,
> + unsigned long long length,
> + guint flags,
> + GError **err);
> +
> +gboolean gvir_storage_vol_upload(GVirStorageVol *vol,
> + GVirStream *stream,
> + unsigned long long offset,
> + unsigned long long length,
> + guint flags,
> + GError **err);
And we use guint64 there as well.
> G_END_DECLS
>
> #endif /* __LIBVIRT_GOBJECT_STORAGE_VOL_H__ */
> diff --git a/libvirt-gobject/libvirt-gobject.h b/libvirt-gobject/libvirt-gobject.h
> index f52cc00..b1158f7 100644
> --- a/libvirt-gobject/libvirt-gobject.h
> +++ b/libvirt-gobject/libvirt-gobject.h
> @@ -44,5 +44,6 @@
> #include <libvirt-gobject/libvirt-gobject-storage-pool.h>
> #include <libvirt-gobject/libvirt-gobject-connection.h>
> #include <libvirt-gobject/libvirt-gobject-manager.h>
> +#include <libvirt-gobject/libvirt-gobject-stream.h>
>
> #endif /* __LIBVIRT_GOBJECT_H__ */
> diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym
> index db32c7f..478881b 100644
> --- a/libvirt-gobject/libvirt-gobject.sym
> +++ b/libvirt-gobject/libvirt-gobject.sym
> @@ -143,6 +143,8 @@ LIBVIRT_GOBJECT_0.0.8 {
> gvir_storage_vol_get_info;
> gvir_storage_vol_delete;
> gvir_storage_vol_resize;
> + gvir_storage_vol_download;
> + gvir_storage_vol_upload;
>
> gvir_connection_handle_get_type;
>
> --
> 1.7.10.4
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124
More information about the libvir-list
mailing list