[libvirt] [glib PATCH] Add gvir_connection_domain_restore() and gvir_connection_domain_restore_async()
Jovanka Gulicoska
jovanka.gulicoska at gmail.com
Wed Jul 11 16:32:29 UTC 2012
I've sent version 2 of this PATCH. I've changed this one since I saw your
comments on gvir_save_to_file_domain.
On Wed, Jul 11, 2012 at 6:18 PM, Zeeshan Ali (Khattak)
<zeeshanak at gnome.org>wrote:
> Hi,
> Forgot review this one. Mostly its the same things I noticed here
> as the last one.
>
> On Wed, Jul 11, 2012 at 8:28 AM, Jovanka Gulicoska
> <jovanka.gulicoska at gmail.com> 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,
>
> Same as in previous patch: no additional signal needed.
>
> > 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,
>
> Now that I think of it, I think 'custom_conf' would be a better name
> for this parameter. Same for the previous patch.
>
> > + 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) {
>
> Same here:
>
> 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.
>
> > + 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;
>
> Since we are using essentially the same data as for
> gvir_connection_domain_save, I think we can simply re-use that:
>
> #define DomainRestoreFromFileData DomainSaveFromFileData
>
> > +static void
> domain_restore_from_file_data_free(DomainRestoreFromFileData *data)
> > +{
>
> Same goes for this function.
>
> > + 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 = g_simple_async_result_get_op_res_gpointer(res);
> > + 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)
>
> Possible coding-style issue here as well.
>
> > +{
> > + 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 = g_slice_new0(DomainRestoreFromFileData);
> > + 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;
>
> Same warning about tabs.
>
> --
> Regards,
>
> Zeeshan Ali (Khattak)
> FSF member#5124
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120711/426902c7/attachment-0001.htm>
More information about the libvir-list
mailing list