[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [glib PATCH V3] Add bindings for virDomainRestore*()



Hi Jovanka,

As we figured out while chating about this, I had misunderstood that
VirDomainRestore operates on connection rather than domain so sorry
for mistakes in my previous review of this.

On Sat, Jul 28, 2012 at 9:23 PM, Jovanka Gulicoska
<jovanka gulicoska gmail com> wrote:
> ---
>  libvirt-gobject/libvirt-gobject-connection.c |  160 ++++++++++++++++++++++++++
>  libvirt-gobject/libvirt-gobject-connection.h |   20 ++++
>  libvirt-gobject/libvirt-gobject.sym          |    3 +
>  3 files changed, 183 insertions(+)
>
> diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c
> index 3a99034..54e4d2c 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.c
> +++ b/libvirt-gobject/libvirt-gobject-connection.c
> @@ -1605,3 +1605,163 @@ gvir_connection_get_capabilities_finish(GVirConnection *conn,
>
>      return g_object_ref(caps);
>  }
> +
> +/**
> + * gvir_connection_domain_restore_from_file:

-> gvir_connection_restore_domain_from_file . Same goes for other
functions/structs in this patch.

> + * @conn: a #GVirConnection
> + * @filename: path to input file
> + * @custom_conf: (allow-none): configuration for domain or NULL
> + * @flags: the flags

Since the saving method is in another object, it will not be intuitive
for app developer so you want to add some docs here telling that this
method restores the domain saved by gvir_domain_save_to_file.

> + *
> + * Returns: TRUE on success, FALSE otherwise
> + */

This function IMO should return the restored domain but unfortunately,
 virDomainRestore doesn't give you the domain pointer. :(

> +gboolean gvir_connection_domain_restore_from_file(GVirConnection *conn,
> +                                                  gchar *filename,
> +                                                  GVirConfigDomain *custom_conf,
> +                                                  guint flags,
> +                                                  GError **err)
> +{
> +    GVirConnectionPrivate *priv;
> +    int ret;
> +
> +    g_return_val_if_fail(GVIR_IS_CONNECTION(conn), FALSE);
> +    g_return_val_if_fail((filename != NULL), FALSE);
> +    g_return_val_if_fail((err == NULL) || (*err == NULL), FALSE);
> +
> +    priv = conn->priv;
> +
> +    if(flags || (custom_conf != NULL)) {

Space after 'if'. Same goes for other ifs.

> +       gchar *custom_xml = NULL;
> +
> +       if(custom_conf != NULL)
> +          custom_xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(custom_conf));
> +
> +       ret = virDomainRestoreFlags(priv->conn, filename, custom_xml, flags);
> +       g_free (custom_xml);
> +    }
> +    else {
> +       ret = virDomainRestore(priv->conn, filename);
> +    }
> +
> +    if(ret < 0) {
> +       gvir_set_error_literal(err, GVIR_CONNECTION_ERROR,
> +                              0,
> +                              "Unable to restore domain");
> +
> +       return FALSE;
> +    }
> +
> +    return TRUE;
> +}
> +
> +typedef struct {
> +    gchar *filename;
> +    GVirConfigDomain *custom_conf;
> +    guint flags;
> +} DomainRestoreFromFileData;
> +
> +static void domain_restore_from_file_data_free(DomainRestoreFromFileData *data)
> +{
> +    g_free(data->filename);
> +    g_object_unref(data->custom_conf);

You need a NULL check here, just like for g_object_ref() below.

> +    g_slice_free(DomainRestoreFromFileData, data);
> +}
> +
> +
> +static void
> +gvir_connection_domain_restore_from_file_helper
> +                               (GSimpleAsyncResult *res,
> +                                GObject *object,
> +                                GCancellable *cancellable G_GNUC_UNUSED)
> +{
> +    GVirConnection *conn = GVIR_CONNECTION(object);
> +    DomainRestoreFromFileData *data;
> +    GError *err = NULL;
> +
> +    data = g_simple_async_result_get_op_res_gpointer(res);
> +
> +    if(!gvir_connection_domain_restore_from_file(conn, data->filename,
> +                                                 data->custom_conf,
> +                                                 data->flags, &err))
> +       g_simple_async_result_take_error(res, err);
> +}
> +
> +/**
> + * gvir_connection_domain_restore_async:
> + * @conn: a #GVirConnection
> + * @filename: path to input file
> + * @custom_conf: (allow-none): configuration for domain
> + * @flags: the flags
> + * @cancellable: (allow-none) (transfer none):cancallation object
> + * @callback: (scope async): completion callback
> + * @user_data: (closure): opaque data for callback
> + *
> + * Asynchronous variant of #gvir_connection_domain_restore
> + */
> +void
> +gvir_connection_domain_restore_from_file_async(GVirConnection *conn,
> +                                               gchar *filename,
> +                                               GVirConfigDomain *custom_conf,
> +                                               guint flags,
> +                                               GCancellable *cancellable,
> +                                               GAsyncReadyCallback callback,
> +                                               gpointer user_data)
> +{
> +    GSimpleAsyncResult *res;
> +    DomainRestoreFromFileData *data;
> +
> +    g_return_if_fail(GVIR_IS_CONNECTION(conn));
> +    g_return_if_fail(filename != NULL);
> +    g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
> +
> +    data = g_slice_new0(DomainRestoreFromFileData);
> +    data->filename = g_strdup(filename);
> +    if(custom_conf != NULL)
> +       data->custom_conf = g_object_ref(custom_conf);
> +    data->flags = flags;
> +
> +    res = g_simple_async_result_new
> +                         (G_OBJECT(conn),
> +                          callback,
> +                          user_data,
> +                          gvir_connection_domain_restore_from_file_async);
> +    g_simple_async_result_set_op_res_gpointer
> +                          (res, data,

'data' on newline.

> +                           (GDestroyNotify)domain_restore_from_file_data_free);
> +
> +    g_simple_async_result_run_in_thread
> +                          (res,
> +                           gvir_connection_domain_restore_from_file_helper,
> +                           G_PRIORITY_DEFAULT,
> +                           cancellable);
> +
> +    g_object_unref(res);
> +}
> +
> +/**
> + * gvir_connection_domain_finish:
> + * @conn: a #GVirConnection
> + * @result: (transfer none): async method result
> + * @err: Place-holder for possible errors
> + *
> + * Finishes the operation started by #gvir_domain_restore_async.
> + *
> + * Returns: TRUE if domain was restored successfully, FALSE otherwise.
> + */
> +gboolean
> +gvir_connection_domain_restore_from_file_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_from_file_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..f475918 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.h
> +++ b/libvirt-gobject/libvirt-gobject-connection.h
> @@ -202,6 +202,26 @@ gvir_connection_get_capabilities_finish(GVirConnection *conn,
>                                          GAsyncResult *result,
>                                          GError **err);
>
> +gboolean
> +gvir_connection_domain_restore_from_file(GVirConnection *conn,
> +                                         gchar *filename,
> +                                         GVirConfigDomain *custom_conf,
> +                                         guint flags,
> +                                         GError **err);
> +
> +void
> +gvir_connection_domain_restore_from_file_async(GVirConnection *conn,
> +                                               gchar *filename,
> +                                               GVirConfigDomain *custom_conf,
> +                                               guint flags,
> +                                               GCancellable *cancellable,
> +                                               GAsyncReadyCallback callback,
> +                                               gpointer user_data);
> +
> +gboolean
> +gvir_connection_domain_restore_from_file_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 54a093a..a2b0428 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_from_file;
> +       gvir_connection_domain_restore_from_file_async;
> +       gvir_connection_domain_restore_from_file_finish;
>
>         gvir_domain_device_get_type;
>         gvir_domain_device_get_domain;


-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]