Hi Christophe,

On Wed, Oct 28, 2015 at 10:15 AM, Christophe Fergeau
<cferg...@redhat.com> wrote:
> Hey,
>
> On Tue, Oct 27, 2015 at 06:17:58PM +0000, Zeeshan Ali (Khattak) wrote:
>> ---
>>  libvirt-gobject/libvirt-gobject-domain.c | 126 
>> +++++++++++++++++++++++++++++++
>>  libvirt-gobject/libvirt-gobject-domain.h |  25 ++++++
>>  libvirt-gobject/libvirt-gobject.sym      |   9 +++
>>  3 files changed, 160 insertions(+)
>>
>> diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
>> b/libvirt-gobject/libvirt-gobject-domain.c
>> index 34eb7ca..26d0cba 100644
>> --- a/libvirt-gobject/libvirt-gobject-domain.c
>> +++ b/libvirt-gobject/libvirt-gobject-domain.c
>> @@ -1886,3 +1886,129 @@ gboolean 
>> gvir_domain_get_has_current_snapshot(GVirDomain *dom,
>>
>>      return TRUE;
>>  }
>> +
>> +/**
>> + * gvir_domain_set_time:
>> + * @dom: the domain
>> + * @seconds: The seconds since Epoch in UTC.
>> + * @nseconds: The microseconds.
>> + * @flags: bitwise-OR of #GVirDomainSetTimeFlags.
>> + *
>> + * This function tries to set guest time to the given value. The time to set
>> + * (@seconds and @nseconds) should be in seconds relative to the Epoch of
>> + * 1970-01-01 00:00:00 in UTC.
>> + *
>> + * If you pass #GVIR_DOMAIN_TIME_SYNC as @flags, the time is reset using
>> + * domain's RTC and @seconds and @nseconds arguments are ignored.
>
> I'd add some more articles, but not sure it's needed/better: « using
> the domain's RTC and the @seconds and @nseconds arguments are ignored. »
>
>> + *
>> + * Please note that some hypervisors may require guest agent to be 
>> configured
>> + * and running in order for this function to work.
>
> and here « a guest agent ». Just ignore these comments if you are
> confident your version reads better to native speakers :)

I'm no native speaker so I wouldn't know. :) However, "a guest agent"
would sound to me that it could be any guest agent but then again,
specifying "Qemu guest agent" would be wrong, keeping in mind that
libvirt wants to remain hypervisor-agnostic. I had just copy&pasted
that part from underlying libvirt docs.

>> + */
>> +gboolean gvir_domain_set_time(GVirDomain *dom,
>> +                              gint64 seconds,
>> +                              guint nseconds,
>
> A GDateTime would be more convenient than these seconds/nseconds fields.

Ah, indeed.

>> +                              guint flags,
>> +                              GError **err)
>> +{
>> +    GVirDomainPrivate *priv;
>> +    int ret;
>> +
>> +    g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
>> +    g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
>> +    g_return_val_if_fail(flags == 0 || flags & GVIR_DOMAIN_TIME_SYNC, 
>> FALSE);
>
> (flags & GVIR_DOMAIN_TIME_SYNC) != 0 to be consistent with the 'flags ==
> 0' before. However, shouldn't it be flags & GVIR_DOMAIN_TIME_SYNC ==
> GVIR_DOMAIN_TIME_SYNC to be sure we were not passed unknown flags?

Yeah, that sounds better.

> Looks good otherwise. Out of curiosity have you observed
> virDomainSetTime blocking, or is the async version there because it will
> be doing expensive stuff if it has to talk to a guest agent?

The latter. Most libvirt calls do I/O so it's best to have async
variants for them but in this case I thought it's especially needed
since there not only communication with libvirtd involved but also
guest agent.

-- 
Regards,

Zeeshan Ali (Khattak)
________________________________________
Befriend GNOME: http://www.gnome.org/friends/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to