Hi Jovanka,
Starting to look really good already. Some comments below:
On Wed, Jul 11, 2012 at 8:26 AM, Jovanka Gulicoska
<jovanka.gulicoska(a)gmail.com> wrote:
---
libvirt-gobject/libvirt-gobject-domain.c | 138 ++++++++++++++++++++++++++++++
libvirt-gobject/libvirt-gobject-domain.h | 19 ++++
libvirt-gobject/libvirt-gobject.sym | 3 +
3 files changed, 160 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c
b/libvirt-gobject/libvirt-gobject-domain.c
index 088cd33..356b15b 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -55,6 +55,7 @@ enum {
VIR_RESUMED,
VIR_STOPPED,
VIR_UPDATED,
+ VIR_SAVED_TO_FILE,
There is no need for adding this signal. We already have appropriate
signals for the relevant libvirt events ("stopped::saved" in this
case). Besides you are not emitting this signal.
LAST_SIGNAL
};
@@ -225,6 +226,16 @@ static void gvir_domain_class_init(GVirDomainClass *klass)
G_TYPE_NONE,
0);
+ signals[VIR_SAVED_TO_FILE] = g_signal_new("saved_to_file",
+ G_OBJECT_CLASS_TYPE(object_class),
+ G_SIGNAL_RUN_LAST | G_SIGNAL_NO_RECURSE |
+ G_SIGNAL_NO_HOOKS | G_SIGNAL_DETAILED,
+ G_STRUCT_OFFSET(GVirDomainClass,
saved_to_file),
+ NULL, NULL,
+ g_cclosure_marshal_VOID__VOID,
+ G_TYPE_NONE,
+ 0);
+
g_type_class_add_private(klass, sizeof(GVirDomainPrivate));
}
@@ -556,6 +567,133 @@ gboolean gvir_domain_reboot(GVirDomain *dom,
return TRUE;
}
+gboolean gvir_domain_save_to_file_config(GVirDomain *dom,
No need for the additional '_config' suffix.
+ gchar *filename,
+ GVirConfigDomain *conf,
+ guint flags,
+ GError **err)
+{
+ GVirDomainPrivate *priv;
+ gchar *custom_xml;
+ int ret;
+
+ g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
+ g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN (conf), FALSE);
+ g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
+
+ priv = dom->priv;
+ custom_xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
+
+ if (flags || custom_xml) {
I'd prefer explicit NULL check (compiler can optimize for us):
if (flags || custom_xml != NULL) {
+ ret = virDomainSaveFlags(priv->handle, filename,
custom_xml, flags);
+ g_free (custom_xml);
+ }
+ else {
+ ret = virDomainSave(priv->handle, filename);
+ g_free (custom_xml);
+ }
+ if (ret < 0) {
+ gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
+ 0,
+ "Unable to save domain to file");
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+typedef struct {
+ gchar *filename;
+ gchar *custom_xml;
+ guint flags;
+} DomainSaveToFileData;
+
+static void domain_save_to_file_data_free(DomainSaveToFileData *data)
+{
You need to duplicate (g_strdup) the 'custom_xml' and 'filename'
strings when you put them in the data and then free them here.
+ g_slice_free(DomainSaveToFileData, data);
+}
+
+static void
+gvir_domain_save_to_file_helper(GSimpleAsyncResult *res,
+ GObject *object,
+ GCancellable *cancellable G_GNUC_UNUSED)
Its hard to be sure with this gmail's web interface but I think last 2
arguments aren't aligned. Please do read the HACKING file and read
through your code/patches to see if it follows the conventions.
+{
+ GVirDomain *dom = GVIR_DOMAIN(object);
+ DomainSaveToFileData *data;
+ GVirConfigDomain *conf;
+ GError *err = NULL;
+ // gchar *xml;
Some redundant commented-out code here and below. I guess you forgot to remove.
+ data = g_simple_async_result_get_op_res_gpointer(res);
+ // // xml = data->filename;
+ conf = gvir_domain_get_config(dom, data->flags, &err);
+
+ if (!gvir_domain_save_to_file_config(dom, data->filename, conf, data->flags,
&err))
+ g_simple_async_result_take_error(res, err);
+ }
+
+/**
+ * gvir_domain_save_to_file_async:
+ * @dom: the domain
+ * @flags: the flags
+ * @cancellable: cancallation object
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
+ *
+ * Asynchronous variant of #gvir_domain_save_to_file.
+ */
+
+void gvir_domain_save_to_file_async (GVirDomain *dom,
+ gchar *filename,
+ GVirConfigDomain *conf,
+ guint flags,
+ GCancellable *cancellable,
+ GAsyncReadyCallback callback,
+ gpointer user_data)
+{
+ GSimpleAsyncResult *res;
+ DomainSaveToFileData *data;
+ gchar *xml;
+
+ g_return_if_fail(GVIR_IS_DOMAIN(dom));
+ g_return_if_fail(GVIR_CONFIG_IS_DOMAIN (conf));
+ g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
+
+ xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
+
+ data = g_slice_new0(DomainSaveToFileData);
+ data->filename = filename;
+ data->custom_xml = xml;
+ data->flags = flags;
+
+ res = g_simple_async_result_new(G_OBJECT(dom),
+ callback,
+ user_data,
+ gvir_domain_save_to_file_async);
+ g_simple_async_result_set_op_res_gpointer(res, data,
(GDestroyNotify)domain_save_to_file_data_free);
+
+ g_simple_async_result_run_in_thread(res,
+ gvir_domain_save_to_file_helper,
+ G_PRIORITY_DEFAULT,
+ cancellable);
+
+ g_object_unref(res);
+}
+
+gboolean gvir_domain_save_to_file_finish(GVirDomain *dom,
+ GAsyncResult *result,
+ GError **err)
+{
+ g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
+ g_return_val_if_fail(g_simple_async_result_is_valid(result, G_OBJECT(dom),
gvir_domain_save_to_file_async), FALSE);
+ g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
+
+ if (g_simple_async_result_propagate_error(G_SIMPLE_ASYNC_RESULT(result), err))
+ return FALSE;
+
+ return TRUE;
+}
+
/**
* gvir_domain_get_config:
* @dom: the domain
diff --git a/libvirt-gobject/libvirt-gobject-domain.h
b/libvirt-gobject/libvirt-gobject-domain.h
index 87b94f4..1df54de 100644
--- a/libvirt-gobject/libvirt-gobject-domain.h
+++ b/libvirt-gobject/libvirt-gobject-domain.h
@@ -65,6 +65,7 @@ struct _GVirDomainClass
void (*resumed)(GVirDomain *dom);
void (*updated)(GVirDomain *dom);
void (*suspended)(GVirDomain *dom);
+ void (*saved_to_file) (GVirDomain *dom);
This will also go away with the signal.
gpointer padding[20];
};
@@ -148,6 +149,24 @@ gboolean gvir_domain_reboot(GVirDomain *dom,
guint flags,
GError **err);
+void gvir_domain_save_to_file_async (GVirDomain *dom,
+ gchar *filename,
+ GVirConfigDomain *conf,
+ guint flags,
+ GCancellable *cancellable,
+ GAsyncReadyCallback callback,
+ gpointer user_data);
+
+gboolean gvir_domain_save_to_file_finish(GVirDomain *dom,
+ GAsyncResult *result,
+ GError **err);
+
+gboolean gvir_domain_save_to_file_config(GVirDomain *dom,
+ gchar *filename,
+ GVirConfigDomain *conf,
+ guint flags,
+ GError **err);
+
GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
GError **err);
void gvir_domain_get_info_async(GVirDomain *dom,
diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym
index 94e441a..8ff9e24 100644
--- a/libvirt-gobject/libvirt-gobject.sym
+++ b/libvirt-gobject/libvirt-gobject.sym
@@ -75,6 +75,9 @@ LIBVIRT_GOBJECT_0.0.8 {
gvir_domain_get_persistent;
gvir_domain_get_saved;
gvir_domain_screenshot;
+ gvir_domain_save_to_file_config;
+ gvir_domain_save_to_file_async;
+ gvir_domain_save_to_file_finish;
Beware that this file (unlike the rest of the code) uses tabs rather
than spaces for indenting.
BTW, commit log summary could just be: Add bindings for virDomainSave*().
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124