Re: [libvirt] [PATCH v1 1/4] Introduce virDomain{Get,Set}Time APIs

On 02/13/2014 11:51 AM, Michal Privoznik wrote: > These APIs allow users to get or set time in a domain, which may come > handy if the domain has been resumed just recently and NTP is not > configured or hasn't kicked in yet and the guest is running > something time critical. In addition, NTP may refuse to re-set the clock > if the skew is too big. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > +/** > + * virDomainGetTime: > + * @dom: a domain object > + * @time: where to store the domain's time > + * @flags: extra flags, not used yet, so callers should always pass 0 > + * > + * Get the information about guest time relative to the Epoch of > + * 1970-01-01 in UTC. The returned time is in seconds. Even though qga doesn't yet provide it, should we make this API flexible enough to also allow return the timezone offset of the guest for hypervisors that have a way of reporting that from the guest? That is, documenting that the reported time is always normalized to UTC is okay, but it would also be nice to have an int* parameter that can store the timezone offset, if known.

> + * > + * Please note that some hypoervisors may require guest agent to s/hypoervisors/hypervisors/ > + * be configured and running in order to be able to run this API. > + * > + * Returns 0 on success, -1 otherwise. > + */ > +int > +virDomainGetTime(virDomainPtr dom, > + long long *time, > + unsigned int flags) > +{ > + VIR_DOMAIN_DEBUG(dom, "time=%p, flags=%x", > + time, flags); > + > + virResetLastError(); > + > + virCheckDomainReturn(dom, -1); > + virCheckReadOnlyGoto(dom->conn->flags, error); Good - since this may involve guest agent interaction, it should not be allowed on read-only clients. > + > +/** > + * virDomainSetTime: > + * @dom: a domain object > + * @time: time to set in the domain > + * @timezone: timezone of @time, currently not used, always pass NULL > + * @flags: extra flags, not used yet, so callers should always pass 0 There is an upstream patch pending for qga that adds the ability to call set-time without a time specification, which then tells the guest to reread its (virtual) hardware clock and adjust its time from there. https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02333.html We need to support that option; I suggest that it might be worth defining our first @flags option.

> + * > + * When a domain is suspended or restored from a file the > + * domain's OS has no idea that there was a big gap in the time. > + * Depending on how long the gap was, NTP might not be able to > + * resynchronize the guest. > + * > + * This API tries to set guest time to the given value. The time > + * should be in seconds, relative to the Epoch of 1970-01-01 in UTC. > + * > + * Please note that some hypoervisors may require guest agent to s/hypoervisors/hypervisors/ > + * be configured and running in order to be able to run this API. > + * > + * Returns 0 on success, -1 otherwise. > + */ > +int > +virDomainSetTime(virDomainPtr dom, > + long long time, > + const char *timezone, If timezone is not NULL, how would it be interpreted? Would it be better to report a timezone as an int (minutes east or west from UTC) than a string?