[libvirt] [glib PATCH V2] Add bindings for virStorageDownload() and virStorageUpload()
--- 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 + * @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, + guint flags, + GError **err) +{ + virStreamPtr st = NULL; + 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) { + 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 + * @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, + guint flags, + GError **err) +{ + virStreamPtr st = NULL; + 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) { + 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; +} + 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); + 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
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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Fri, 13 Jul 2012 at 07:37 GMT, Jovanka Gulicoska <jovanka.gulicoska@gmail.com> wrote:
+gboolean gvir_storage_vol_download(GVirStorageVol *vol, + GVirStream *stream, + unsigned long long offset, + unsigned long long length, + guint flags, + GError **err) +{ + virStreamPtr st = NULL; + 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) { + 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; +}
That 'goto' is not necessary, you can refactor the code to eliminate it, something like below: if (.... < 0) gvir_set_error_literal(...); else ret = TRUE; if (st != NULL) virStreamFree(st); return ret;
On Wed, Jul 18, 2012 at 09:33:58AM +0000, Cong Wang wrote:
On Fri, 13 Jul 2012 at 07:37 GMT, Jovanka Gulicoska <jovanka.gulicoska@gmail.com> wrote:
+gboolean gvir_storage_vol_download(GVirStorageVol *vol, + GVirStream *stream, + unsigned long long offset, + unsigned long long length, + guint flags, + GError **err) +{ + virStreamPtr st = NULL; + 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) { + 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; +}
That 'goto' is not necessary, you can refactor the code to eliminate it, something like below:
if (.... < 0) gvir_set_error_literal(...); else ret = TRUE;
if (st != NULL) virStreamFree(st); return ret;
The style we have here is the standard libvirt format, since it becomes clearer to use the goto when you add further code in the method. 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 :|
participants (4)
-
Cong Wang -
Daniel P. Berrange -
Jovanka Gulicoska -
Zeeshan Ali (Khattak)