Re: [libvirt] [PATCH v1 1/4] Introduce virDomain{Get,Set}Time APIs
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