[libvirt] [PATCH v2] gobject: Add wrapper virDomainSetTime()

Christophe Fergeau cfergeau at redhat.com
Mon Nov 2 15:43:15 UTC 2015


Some minor comments below, ACK.

Christophe

On Thu, Oct 29, 2015 at 08:44:17PM +0000, Zeeshan Ali (Khattak) wrote:
> ---
> 
> This version:
> 
> * Replaces the seconds and nseconds arguments by a GDateTime.
> * Drops the use of flags argument, since caller can specify the only flag currently possible (VIR_DOMAIN_TIME_SYNC) by simply passing a NULL as the GDateTime argument.
> * Add some needed articles to doc comment.
> 
>  libvirt-gobject/libvirt-gobject-domain.c | 121 +++++++++++++++++++++++++++++++
>  libvirt-gobject/libvirt-gobject-domain.h |  15 +++-
>  libvirt-gobject/libvirt-gobject.sym      |   9 +++
>  3 files changed, 144 insertions(+), 1 deletion(-)
> 
> diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c
> index 34eb7ca..debae2d 100644
> --- a/libvirt-gobject/libvirt-gobject-domain.c
> +++ b/libvirt-gobject/libvirt-gobject-domain.c
> @@ -1886,3 +1886,124 @@ gboolean gvir_domain_get_has_current_snapshot(GVirDomain *dom,
>  
>      return TRUE;
>  }
> +
> +/**
> + * gvir_domain_set_time:
> + * @dom: the domain
> + * @date_time: (allow-none)(transfer none): the time to set as #GDateTime.
> + * @flags: Unused, Pass 0.

'pass 0' rather than 'Pass 0' ?
Missing "@err: (allow-none): Place-holder for error or NULL"?

> + *
> + * This function tries to set guest time to the given value. The passed
> + * time must in UTC.
> + *
> + * If @date_time is %NULL, the time is reset using the domain's RTC.
> + *
> + * Please note that some hypervisors may require guest agent to be configured
> + * and running in order for this function to work.
> + */
> +gboolean gvir_domain_set_time(GVirDomain *dom,
> +                              GDateTime *date_time,
> +                              guint flags G_GNUC_UNUSED,
> +                              GError **err)
> +{
> +    GVirDomainPrivate *priv;
> +    int ret;
> +    GTimeVal tv;
> +    gint64 seconds;
> +    guint nseconds;

They are going to store GTimeVal fields, glong could be used for
seconds, and glong or guint64 for nseconds. (but I think the types you
picked are going to fit tv_sec and tv_usec * 1000).

> +    guint settime_flags;
> +
> +    g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
> +    g_return_val_if_fail(err == NULL || *err == NULL, FALSE);

I'd keep a g_return_val_if_fail(flags == 0, FALSE);

> +
> +    if (date_time != NULL) {
> +        if (!g_date_time_to_timeval(date_time, &tv)) {
> +            gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                                   0,
> +                                   "Failed to parse given time argument");

g_set_error_literal, not gvir_set_error_literal.

> +            return FALSE;
> +        }
> +
> +        seconds = tv.tv_sec;
> +        nseconds = tv.tv_usec * 1000;
> +        settime_flags = 0;
> +    } else {
> +        seconds = 0;
> +        nseconds = 0;
> +        settime_flags = VIR_DOMAIN_TIME_SYNC;
> +    }
> +
> +    priv = dom->priv;
> +    ret = virDomainSetTime(priv->handle, seconds, nseconds, settime_flags);

Nit: I'd just go with dom->priv->handle rather than having a local
'priv' used only once.

> +    if (ret < 0) {
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to set domain time");
> +        return FALSE;
> +    }
> +
> +    return TRUE;
> +}
> +
> +static void
> +gvir_domain_set_time_helper(GTask *task,
> +                            gpointer object,
> +                            gpointer task_data,
> +                            GCancellable *cancellable G_GNUC_UNUSED)
> +{
> +    GVirDomain *dom = GVIR_DOMAIN(object);
> +    GDateTime *date_time = (GDateTime *) task_data;
> +    GError *err = NULL;
> +
> +    if (!gvir_domain_set_time(dom, date_time, 0, &err))
> +        g_task_return_error(task, err);
> +    else
> +        g_task_return_boolean(task, TRUE);
> +}
> +
> +/**
> + * gvir_domain_set_time_async:
> + * @dom: the domain
> + * @date_time: (allow-none)(transfer none): the time to set as #GDateTime.
> + * @flags: bitwise-OR of #GVirDomainSetTimeFlags.
> + * @cancellable: (allow-none)(transfer none): cancellation object
> + * @callback: (scope async): completion callback
> + * @user_data: (closure): opaque data for callback
> + *
> + * Asynchronous variant of #gvir_domain_set_time.
> + */
> +void gvir_domain_set_time_async(GVirDomain *dom,
> +                                GDateTime *date_time,
> +                                guint flags G_GNUC_UNUSED,
> +                                GCancellable *cancellable,
> +                                GAsyncReadyCallback callback,
> +                                gpointer user_data)
> +{
> +    GTask *task;
> +
> +    g_return_if_fail(GVIR_IS_DOMAIN(dom));
> +    g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));

Same comment about testing that flags is actually 0

> +
> +    task = g_task_new(G_OBJECT(dom),
> +                      cancellable,
> +                      callback,
> +                      user_data);
> +    if (date_time != NULL)
> +        g_task_set_task_data(task,
> +                             g_date_time_ref(date_time),
> +                             (GDestroyNotify)g_date_time_unref);
> +    g_task_run_in_thread(task, gvir_domain_set_time_helper);
> +
> +    g_object_unref(task);
> +}
> +

API doc for _finish would be nice.

> +gboolean gvir_domain_set_time_finish(GVirDomain *dom,
> +                                     GAsyncResult *result,
> +                                     GError **err)
> +{
> +    g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
> +    g_return_val_if_fail(g_task_is_valid(result, G_OBJECT(dom)), FALSE);
> +    g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
> +
> +    return g_task_propagate_boolean(G_TASK(result), err);
> +}
> diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h
> index 4fe381e..099cde3 100644
> --- a/libvirt-gobject/libvirt-gobject-domain.h
> +++ b/libvirt-gobject/libvirt-gobject-domain.h
> @@ -215,7 +215,6 @@ typedef enum {
>      GVIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL    = VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL
>  } GVirDomainSnapshotListFlags;
>  
> -
>  typedef struct _GVirDomainInfo GVirDomainInfo;
>  struct _GVirDomainInfo
>  {
> @@ -401,6 +400,20 @@ gboolean gvir_domain_get_has_current_snapshot(GVirDomain *dom,
>                                                gboolean *has_current_snapshot,
>                                                GError **error);
>  
> +gboolean gvir_domain_set_time(GVirDomain *dom,
> +                              GDateTime *date_time,
> +                              guint flags,
> +                              GError **err);
> +void gvir_domain_set_time_async(GVirDomain *dom,
> +                                GDateTime *date_time,
> +                                guint flags,
> +                                GCancellable *cancellable,
> +                                GAsyncReadyCallback callback,
> +                                gpointer user_data);
> +gboolean gvir_domain_set_time_finish(GVirDomain *dom,
> +                                     GAsyncResult *result,
> +                                     GError **err);
> +
>  G_END_DECLS
>  
>  #endif /* __LIBVIRT_GOBJECT_DOMAIN_H__ */
> diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym
> index ca89a45..cbfaa71 100644
> --- a/libvirt-gobject/libvirt-gobject.sym
> +++ b/libvirt-gobject/libvirt-gobject.sym
> @@ -304,4 +304,13 @@ LIBVIRT_GOBJECT_0.2.2 {
>  	gvir_network_get_dhcp_leases;
>  } LIBVIRT_GOBJECT_0.2.1;
>  
> +LIBVIRT_GOBJECT_0.2.3 {
> +  global:
> +	gvir_domain_set_time_flags_get_type;
> +
> +	gvir_domain_set_time;
> +	gvir_domain_set_time_async;
> +	gvir_domain_set_time_finish;
> +} LIBVIRT_GOBJECT_0.2.2;
> +
>  # .... define new API here using predicted next version number ....
> -- 
> 2.5.0
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151102/aaeef738/attachment-0001.sig>


More information about the libvir-list mailing list