On Wed, Jul 11, 2012 at 6:18 PM, Zeeshan Ali (Khattak) <zeeshanak@xxxxxxxxx> wrote:
Hi,
Forgot review this one. Mostly its the same things I noticed here
as the last one.
Same as in previous patch: no additional signal needed.
On Wed, Jul 11, 2012 at 8:28 AM, Jovanka Gulicoska
<jovanka.gulicoska@xxxxxxxxx> wrote:
> ---
> libvirt-gobject/libvirt-gobject-connection.c | 136 ++++++++++++++++++++++++++
> libvirt-gobject/libvirt-gobject-connection.h | 19 ++++
> libvirt-gobject/libvirt-gobject.sym | 3 +
> 3 files changed, 158 insertions(+)
>
> diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c
> index 3a99034..2302328 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.c
> +++ b/libvirt-gobject/libvirt-gobject-connection.c
> @@ -57,6 +57,7 @@ enum {
> VIR_CONNECTION_CLOSED,
> VIR_DOMAIN_ADDED,
> VIR_DOMAIN_REMOVED,
> + VIR_DOMAIN_RESTORED,
Now that I think of it, I think 'custom_conf' would be a better name
> LAST_SIGNAL
> };
>
> @@ -228,6 +229,15 @@ static void gvir_connection_class_init(GVirConnectionClass *klass)
> 1,
> GVIR_TYPE_DOMAIN);
>
> + signals[VIR_DOMAIN_RESTORED] = g_signal_new("domain-restored",
> + G_OBJECT_CLASS_TYPE(object_class),
> + G_SIGNAL_RUN_FIRST,
> + G_STRUCT_OFFSET(GVirConnectionClass, domain_restored),
> + NULL, NULL,
> + g_cclosure_marshal_VOID__OBJECT,
> + G_TYPE_NONE,
> + 0);
> +
> g_type_class_add_private(klass, sizeof(GVirConnectionPrivate));
> }
>
> @@ -1605,3 +1615,129 @@ gvir_connection_get_capabilities_finish(GVirConnection *conn,
>
> return g_object_ref(caps);
> }
> +
> +/*
> + * gvir_connection_domain_restore
> + */
> +gboolean gvir_connection_domain_restore(GVirConnection *conn,
> + gchar *filename,
> + GVirConfigDomain *conf,
for this parameter. Same for the previous patch.
Same here:
> + guint flags,
> + GError **err)
> +{
> + GVirConnectionPrivate *priv;
> + gchar *custom_xml;
> + int ret;
> +
> + g_return_val_if_fail(GVIR_IS_CONNECTION(conn), FALSE);
> + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN (conf), FALSE);
> + g_return_val_if_fail((err == NULL) || (*err == NULL), FALSE);
> +
> + priv = conn->priv;
> + custom_xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
> +
> + g_return_val_if_fail(custom_xml != NULL, FALSE);
> +
> + if(flags || custom_xml) {
if(flags || custom_xml != NULL)
Oh and now I realize that you are checking for the string extracted
from the parameter for being NULL. You want to check 'conf' itself to
be NULL here and then extract the XML string before using it. You'll
wan to remove one of the g_return_val_if_fail above to allow user to
pass NULL as conf.
Same goes for the previous patch too.
Since we are using essentially the same data as for
> + ret = virDomainRestoreFlags(priv->conn, filename, custom_xml, flags);
> + g_free (custom_xml);
> + }
> + else {
> + ret = virDomainRestore(priv->conn, filename);
> + g_free (custom_xml);
> + }
> +
> + if(ret < 0) {
> + gvir_set_error_literal(err, GVIR_CONNECTION_ERROR,
> + 0,
> + "Unable to restore domain");
> +
> + return FALSE;
> + }
> +
> + return TRUE;
> +}
> +
> +typedef struct {
> + gchar *filename;
> + gchar *custom_xml;
> + guint flags;
> +} DomainRestoreFromFileData;
gvir_connection_domain_save, I think we can simply re-use that:
#define DomainRestoreFromFileData DomainSaveFromFileData
Same goes for this function.
> +static void domain_restore_from_file_data_free(DomainRestoreFromFileData *data)
> +{
Possible coding-style issue here as well.
> + g_slice_free(DomainRestoreFromFileData, data);
> +}
> +
> +static void
> +gvir_connection_domain_restore_helper(GSimpleAsyncResult *res,
> + GObject *object,
> + GCancellable *cancellable G_GNUC_UNUSED)
> +{
> + GVirConnection *conn = GVIR_CONNECTION(object);
> + DomainRestoreFromFileData *data;
> + GVirConfigDomain *conf;
> + GError *err = NULL;
> +
> + data = ""> > + conf = gvir_config_domain_new_from_xml(data->custom_xml, &err);
> +
> + if(!gvir_connection_domain_restore(conn, data->filename, conf, data->flags, &err))
> + g_simple_async_result_take_error(res, err);
> +}
> +
> +/*
> + * Async function of gvir_connection_domain_restore
> + */
> +void gvir_connection_domain_restore_async(GVirConnection *conn,
> + gchar *filename,
> + GVirConfigDomain *conf,
> + guint flags,
> + GCancellable *cancellable,
> + GAsyncReadyCallback callback,
> + gpointer user_data)
Same warning about tabs.
> +{
> + GSimpleAsyncResult *res;
> + DomainRestoreFromFileData *data;
> + gchar *custom_xml;
> +
> + g_return_if_fail(GVIR_IS_CONNECTION(conn));
> + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN (conf));
> + g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
> +
> + custom_xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
> +
> + data = ""> > + data->filename = filename;
> + data->custom_xml = custom_xml;
> + data->flags = flags;
> +
> + res = g_simple_async_result_new(G_OBJECT(conn),
> + callback,
> + user_data,
> + gvir_connection_domain_restore_async);
> + g_simple_async_result_set_op_res_gpointer(res, data,
> + (GDestroyNotify)domain_restore_from_file_data_free);
> +
> + g_simple_async_result_run_in_thread(res,
> + gvir_connection_domain_restore_helper,
> + G_PRIORITY_DEFAULT,
> + cancellable);
> +
> + g_object_unref(res);
> +}
> +
> +gboolean gvir_connection_domain_restore_finish(GVirConnection *conn,
> + GAsyncResult *result,
> + GError **err)
> +{
> + g_return_val_if_fail(GVIR_IS_CONNECTION(conn), FALSE);
> + g_return_val_if_fail(g_simple_async_result_is_valid(result, G_OBJECT(conn),
> + gvir_connection_domain_restore_async),
> + FALSE);
> +
> + if (g_simple_async_result_propagate_error(G_SIMPLE_ASYNC_RESULT(result), err))
> + return FALSE;
> +
> + return TRUE;
> +}
> diff --git a/libvirt-gobject/libvirt-gobject-connection.h b/libvirt-gobject/libvirt-gobject-connection.h
> index c80eecf..01d5d0b 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.h
> +++ b/libvirt-gobject/libvirt-gobject-connection.h
> @@ -75,6 +75,8 @@ struct _GVirConnectionClass
> void (*domain_added)(GVirConnection *conn, GVirDomain *dom);
> void (*domain_removed)(GVirConnection *conn, GVirDomain *dom);
>
> + void (*domain_restored)(GVirConnection *conn);
> +
> GVirStream *(*stream_new)(GVirConnection *conn, gpointer handle);
>
> gpointer padding[20];
> @@ -202,6 +204,23 @@ gvir_connection_get_capabilities_finish(GVirConnection *conn,
> GAsyncResult *result,
> GError **err);
>
> +gboolean gvir_connection_domain_restore(GVirConnection *conn,
> + gchar *filename,
> + GVirConfigDomain *conf,
> + guint flags,
> + GError **err);
> +
> +void gvir_connection_domain_restore_async(GVirConnection *conn,
> + gchar *filename,
> + GVirConfigDomain *conf,
> + guint flags,
> + GCancellable *cancellable,
> + GAsyncReadyCallback callback,
> + gpointer user_data);
> +
> +gboolean gvir_connection_domain_restore_finish(GVirConnection *conn,
> + GAsyncResult *result,
> + GError **err);
> G_END_DECLS
>
> #endif /* __LIBVIRT_GOBJECT_CONNECTION_H__ */
> diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym
> index 8ff9e24..0aaefa2 100644
> --- a/libvirt-gobject/libvirt-gobject.sym
> +++ b/libvirt-gobject/libvirt-gobject.sym
> @@ -31,6 +31,9 @@ LIBVIRT_GOBJECT_0.0.8 {
> gvir_connection_create_storage_pool;
> gvir_connection_start_domain;
> gvir_connection_get_node_info;
> + gvir_connection_domain_restore;
> + gvir_connection_domain_restore_async;
> + gvir_connection_domain_restore_finish;
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list