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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list