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

[Michal Privoznik]

On 14.02.2014 16:39, Eric Blake wrote:
> On 02/14/2014 08:14 AM, Michal Privoznik wrote:
>> On 13.02.2014 19:51, 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.
>>>
>
>>> + *
>>> + * 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.
>>
>> One question though. qemu-ga currently takes nanoseconds in its 'guest-set-time' and returns nanoseconds in 'guest-get-time'. I know nanoseconds are out of scope for libvirt. But aren't seconds too gross? Maybe we want something more finer - miliseconds perhaps. In my measurements I was unable to get below 6-7 miliseconds:
>
> We definitely need subsecond resolution.  Maybe best is to mirror struct
> timespec, by providing 'long long seconds' and 'unsigned int nanos'.
> Even if nanos are too fine and precision is lost along the way, there's
> no need to artificially limit things for when performance gets faster in
> the future.
>
>>
>> for ((i=0; i<100; i++)) ; do virsh -t  qemu-agent-command rhel7 '{"execute":"guest-ping"}' | grep Time; done | sort -n -t ':' -k 2
>> (Time: 7,590 ms)
>> (Time: 7,601 ms)
>> (Time: 7,635 ms)
>> (Time: 7,682 ms)
>>
>> (although to be fair, this involves domain lookup API too)
>
> Not to mention that your approach was spawning an app and connection per
> request, rather than reusing a connection within a single app.

Reconnecting is not counted in the timing:

         if ((ctl->conn == NULL || disconnected) &&
             !(cmd->def->flags & VSH_CMD_FLAG_NOCONNECT))
             vshReconnect(ctl);

         if (enable_timing)
             GETTIMEOFDAY(&before);

         if ((cmd->def->flags & VSH_CMD_FLAG_NOCONNECT) ||
             vshConnectionUsability(ctl, ctl->conn)) {
             ret = cmd->def->handler(ctl, cmd);
         } else {
             /* connection is not usable, return error */
             ret = false;
         }

         if (enable_timing)
             GETTIMEOFDAY(&after);



>
> And while I'm thinking about it, I would like to make sure that at least
> the virsh command has a way to both set an explicit time, as well as to
> request a sync to the host time without having to specify a timestamp
> (that is, a common use case will be to sync the guest to the time that
> the host is using, without having to first figure out the host time and
> type that into the virsh command line).  But I'm not sure whether such
> convenience should be limited to virsh, or actually folded into the API
> via another flag.  Remember, if it is virsh that does it, then the time
> being chosen is local to the host running virsh; whereas if a flag is
> used, then the time chosen will be on the hypervisor (which is different
> than the host running virsh if you use remote connection)
>

The patches I'm proposing have the virsh part. With them you can do both:

virsh domtime $dom --sync
virsh domtime $dom --now
virsh domtime $dom 1234567890

Where --now is taken from the host running virsh, not where libvirtd is 
running.  For --host-now (to take the $now from libvirtd) - my patches 
don't implement this right now, but they certainly create environment 
for it - just a new flag needs to be added.

Michal