[libvirt] [PATCH v1 0/4] Time setting and getting in qemu guests
This can be used all, but 'virsh domtime --sync $dom' is dependent on this qemu patch: http://lists.nongnu.org/archive/html/qemu-devel/2014-01/msg04293.html Anyway, once all the patches are merged, users can simply do: virsh resume $dom virsh domtime --sync $dom and their guest time won't be off after the resume. Michal Privoznik (4): Introduce virDomain{Get,Set}Time APIs remote: Implement remote{Get,Set}Time virsh: Expose virDomain{Get,Set}Time qemu: Implement virDomain{Get,Set}Time daemon/remote.c | 35 +++++++++++ include/libvirt/libvirt.h.in | 13 +++++ src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 6 ++ src/driver.h | 13 +++++ src/libvirt.c | 91 +++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/qemu/qemu_agent.c | 81 ++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 6 ++ src/qemu/qemu_driver.c | 135 +++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 32 ++++++++++ src/remote/remote_protocol.x | 31 +++++++++- src/remote_protocol-structs | 16 +++++ tools/virsh-domain-monitor.c | 126 ++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 16 +++++ 15 files changed, 607 insertions(+), 2 deletions(-) -- 1.8.5.3
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@redhat.com> --- include/libvirt/libvirt.h.in | 13 +++++++ src/driver.h | 13 +++++++ src/libvirt.c | 91 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 123 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 295d551..3bbd4df 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5245,6 +5245,19 @@ int virDomainFSTrim(virDomainPtr dom, unsigned long long minimum, unsigned int flags); +int virDomainGetTime(virDomainPtr dom, + long long *time, + unsigned int flags); + +typedef enum { + VIR_DOMAIN_TIME_SYNC = (1 << 0), /* Re-sync domain time from domain's RTC */ +} virDomainSetTimeFlags; + +int virDomainSetTime(virDomainPtr dom, + long long time, + const char *timezone, + unsigned int flags); + /** * virSchedParameterType: * diff --git a/src/driver.h b/src/driver.h index 5f4cd8d..a825434 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1064,6 +1064,17 @@ typedef int unsigned int flags); typedef int +(*virDrvDomainGetTime)(virDomainPtr dom, + long long *time, + unsigned int flags); + +typedef int +(*virDrvDomainSetTime)(virDomainPtr dom, + long long time, + const char *timezone, + unsigned int flags); + +typedef int (*virDrvDomainLxcOpenNamespace)(virDomainPtr dom, int **fdlist, unsigned int flags); @@ -1340,6 +1351,8 @@ struct _virDriver { virDrvDomainMigrateFinish3Params domainMigrateFinish3Params; virDrvDomainMigrateConfirm3Params domainMigrateConfirm3Params; virDrvConnectGetCPUModelNames connectGetCPUModelNames; + virDrvDomainGetTime domainGetTime; + virDrvDomainSetTime domainSetTime; }; diff --git a/src/libvirt.c b/src/libvirt.c index 666ab1e..aa063fd 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20547,3 +20547,94 @@ error: virDispatchError(dom->conn); return -1; } + + +/** + * 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. + * + * Please note that some hypoervisors may require guest agent to + * 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); + + if (dom->conn->driver->domainGetTime) { + int ret = dom->conn->driver->domainGetTime(dom, time, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + +error: + virDispatchError(dom->conn); + return -1; +} + + +/** + * 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 + * + * 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 + * 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, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "time=%lld, timezone=%s, flags=%x", + time, NULLSTR(timezone), flags); + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + virCheckReadOnlyGoto(dom->conn->flags, error); + + if (dom->conn->driver->domainSetTime) { + int ret = dom->conn->driver->domainSetTime(dom, time, timezone, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + +error: + virDispatchError(dom->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 6ed6ce6..2cb3fae 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -645,5 +645,11 @@ LIBVIRT_1.2.1 { virConnectNetworkEventDeregisterAny; } LIBVIRT_1.1.3; +LIBVIRT_1.2.2 { + global: + virDomainGetTime; + virDomainSetTime; +} LIBVIRT_1.2.1; + # .... define new API here using predicted next version number .... -- 1.8.5.3
On Thu, Feb 13, 2014 at 07:51:42PM +0100, 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@redhat.com> --- include/libvirt/libvirt.h.in | 13 +++++++ src/driver.h | 13 +++++++ src/libvirt.c | 91 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 123 insertions(+)
[...]
diff --git a/src/libvirt.c b/src/libvirt.c index 666ab1e..aa063fd 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20547,3 +20547,94 @@ error: virDispatchError(dom->conn); return -1; } + + +/** + * 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. + * + * Please note that some hypoervisors may require guest agent to + * 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);
Can you add virCheckNonNullArgGoto(time, error); here? Or doesn't it make sense? I haven't got to other patches, yet. Preliminary ACK. Martin
On 02/13/2014 07:51 PM, 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@redhat.com> --- include/libvirt/libvirt.h.in | 13 +++++++ src/driver.h | 13 +++++++ src/libvirt.c | 91 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 123 insertions(+)
+int virDomainGetTime(virDomainPtr dom, + long long *time, + unsigned int flags); + +typedef enum { + VIR_DOMAIN_TIME_SYNC = (1 << 0), /* Re-sync domain time from domain's RTC */ +} virDomainSetTimeFlags; + +int virDomainSetTime(virDomainPtr dom, + long long time, + const char *timezone,
Both 'time' and 'timezone' generate a warning about shadowed global declaration with older GCC.
+ unsigned int flags); + /** * virSchedParameterType: *
Jan
On 14.02.2014 09:10, Ján Tomko wrote:
On 02/13/2014 07:51 PM, 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@redhat.com> --- include/libvirt/libvirt.h.in | 13 +++++++ src/driver.h | 13 +++++++ src/libvirt.c | 91 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 123 insertions(+)
+int virDomainGetTime(virDomainPtr dom, + long long *time, + unsigned int flags); + +typedef enum { + VIR_DOMAIN_TIME_SYNC = (1 << 0), /* Re-sync domain time from domain's RTC */ +} virDomainSetTimeFlags; + +int virDomainSetTime(virDomainPtr dom, + long long time, + const char *timezone,
Both 'time' and 'timezone' generate a warning about shadowed global declaration with older GCC.
Sigh. That's another case where a syntax-check rule prohibiting some variable names would be useful. Michal
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@redhat.com> ---
+/** + * 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? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 14.02.2014 15:16, Eric Blake wrote:
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@redhat.com> ---
+/** + * 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.
Sure, great idea.
+ * + * 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.
Ouch. I'm already introducing virDomainSetTimeFlags and even in this patch. So the description is bogus. I'll fix it.
+ * + * 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?
Yeah. That would ease things. Okay, I'll change that and repost. Michal
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.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 13 +++++++ src/driver.h | 13 +++++++ src/libvirt.c | 91 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 123 insertions(+)
+/** + * 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 + * + * 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.
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: 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) Michal
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. 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) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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
This is also adding new ACL permission to check 'set_time'. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- daemon/remote.c | 35 +++++++++++++++++++++++++++++++++++ src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 6 ++++++ src/remote/remote_driver.c | 32 ++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 31 ++++++++++++++++++++++++++++++- src/remote_protocol-structs | 16 ++++++++++++++++ 6 files changed, 120 insertions(+), 2 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 932f65f..8020e60 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6027,3 +6027,38 @@ error: } return -1; } + +static int +remoteDispatchDomainGetTime(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_time_args *args, + remote_domain_get_time_ret *ret) +{ + int rv = -1; + virDomainPtr dom = NULL; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + long long time; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainGetTime(dom, &time, args->flags) < 0) + goto cleanup; + + ret->time = time; + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c index d517c66..bbcb6c1 100644 --- a/src/access/viraccessperm.c +++ b/src/access/viraccessperm.c @@ -42,7 +42,7 @@ VIR_ENUM_IMPL(virAccessPermDomain, "init_control", "inject_nmi", "send_input", "send_signal", "fs_trim", "block_read", "block_write", "mem_read", "open_graphics", "open_device", "screenshot", - "open_namespace"); + "open_namespace", "set_time"); VIR_ENUM_IMPL(virAccessPermInterface, VIR_ACCESS_PERM_INTERFACE_LAST, diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h index 6d14f05..6bfd787 100644 --- a/src/access/viraccessperm.h +++ b/src/access/viraccessperm.h @@ -289,6 +289,12 @@ typedef enum { */ VIR_ACCESS_PERM_DOMAIN_OPEN_NAMESPACE, + /** + * @desc: Write domain time + * @message: Setting the domain time requires authorization + */ + VIR_ACCESS_PERM_DOMAIN_SET_TIME, + VIR_ACCESS_PERM_DOMAIN_LAST, } virAccessPermDomain; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 955465a..4acb745 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7331,6 +7331,36 @@ done: } +static int +remoteDomainGetTime(virDomainPtr dom, + long long *time, + unsigned int flags) +{ + int rv = -1; + struct private_data *priv = dom->conn->privateData; + remote_domain_get_time_args args; + remote_domain_get_time_ret ret; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, dom); + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + + if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_TIME, + (xdrproc_t) xdr_remote_domain_get_time_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_get_time_ret, (char *) &ret) == -1) + goto cleanup; + + *time = ret.time; + rv = ret.ret; + +cleanup: + remoteDriverUnlock(priv); + return rv; +} + /* get_nonnull_domain and get_nonnull_network turn an on-wire * (name, uuid) pair into virDomainPtr or virNetworkPtr object. * These can return NULL if underlying memory allocations fail, @@ -7660,6 +7690,8 @@ static virDriver remote_driver = { .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ .connectGetCPUModelNames = remoteConnectGetCPUModelNames, /* 1.1.3 */ + .domainGetTime = remoteDomainGetTime, /* 1.2.2 */ + .domainSetTime = remoteDomainSetTime, /* 1.2.2 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f1f2359..3fda38c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2845,6 +2845,23 @@ struct remote_domain_fstrim_args { unsigned int flags; }; +struct remote_domain_get_time_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_get_time_ret { + hyper time; + int ret; +}; + +struct remote_domain_set_time_args { + remote_nonnull_domain dom; + hyper time; + remote_string timezone; + unsigned int flags; +}; + struct remote_domain_migrate_begin3_params_args { remote_nonnull_domain dom; remote_typed_param params<REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX>; @@ -5262,5 +5279,17 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVED = 333 + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVED = 333, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_TIME = 334, + + /** + * @generate: both + * @acl: domain:set_time + */ + REMOTE_PROC_DOMAIN_SET_TIME = 335 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 5636d55..0effa27 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2284,6 +2284,20 @@ struct remote_domain_fstrim_args { uint64_t minimum; u_int flags; }; +struct remote_domain_get_time_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_get_time_ret { + int64_t time; + int ret; +}; +struct remote_domain_set_time_args { + remote_nonnull_domain dom; + int64_t time; + remote_string timezone; + u_int flags; +}; struct remote_domain_migrate_begin3_params_args { remote_nonnull_domain dom; struct { @@ -2755,4 +2769,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_BALLOON_CHANGE = 331, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_PMSUSPEND_DISK = 332, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVED = 333, + REMOTE_PROC_DOMAIN_GET_TIME = 334, + REMOTE_PROC_DOMAIN_SET_TIME = 335, }; -- 1.8.5.3
On Thu, Feb 13, 2014 at 07:51:43PM +0100, Michal Privoznik wrote:
This is also adding new ACL permission to check 'set_time'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- daemon/remote.c | 35 +++++++++++++++++++++++++++++++++++ src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 6 ++++++ src/remote/remote_driver.c | 32 ++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 31 ++++++++++++++++++++++++++++++- src/remote_protocol-structs | 16 ++++++++++++++++ 6 files changed, 120 insertions(+), 2 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 932f65f..8020e60 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6027,3 +6027,38 @@ error: } return -1; } + +static int +remoteDispatchDomainGetTime(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_time_args *args, + remote_domain_get_time_ret *ret) +{ + int rv = -1; + virDomainPtr dom = NULL; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + long long time; +
Applies to previous patch too, this 'time' will be a problem with '-Wshadow-declarations' on older (some) compilers. ACK with that variable name changed. Martin
On 02/14/2014 06:23 AM, Martin Kletzander wrote:
On Thu, Feb 13, 2014 at 07:51:43PM +0100, Michal Privoznik wrote:
This is also adding new ACL permission to check 'set_time'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Applies to previous patch too, this 'time' will be a problem with '-Wshadow-declarations' on older (some) compilers.
ACK with that variable name changed.
I'm half-tempted to just tweak m4/virt-compile-warnings.m4 to drop -Wshadow-declarations on older gcc. Since newer gcc is sane about local variables not conflicting with public functions, it's not worth worrying about the collisions that only older gcc reports. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Feb 14, 2014 at 06:32:21AM -0700, Eric Blake wrote:
On 02/14/2014 06:23 AM, Martin Kletzander wrote:
On Thu, Feb 13, 2014 at 07:51:43PM +0100, Michal Privoznik wrote:
This is also adding new ACL permission to check 'set_time'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Applies to previous patch too, this 'time' will be a problem with '-Wshadow-declarations' on older (some) compilers.
ACK with that variable name changed.
I'm half-tempted to just tweak m4/virt-compile-warnings.m4 to drop -Wshadow-declarations on older gcc. Since newer gcc is sane about local variables not conflicting with public functions, it's not worth worrying about the collisions that only older gcc reports.
The problem is shadow decls can occur within libvirt code too in which case they would likely be genuine bugs. eg someone declares 'foo' at the start of a method and some time later redeclares it in a for/while loop body or some such. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 02/14/2014 06:34 AM, Daniel P. Berrange wrote:
On Fri, Feb 14, 2014 at 06:32:21AM -0700, Eric Blake wrote:
On 02/14/2014 06:23 AM, Martin Kletzander wrote:
On Thu, Feb 13, 2014 at 07:51:43PM +0100, Michal Privoznik wrote:
This is also adding new ACL permission to check 'set_time'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Applies to previous patch too, this 'time' will be a problem with '-Wshadow-declarations' on older (some) compilers.
ACK with that variable name changed.
I'm half-tempted to just tweak m4/virt-compile-warnings.m4 to drop -Wshadow-declarations on older gcc. Since newer gcc is sane about local variables not conflicting with public functions, it's not worth worrying about the collisions that only older gcc reports.
The problem is shadow decls can occur within libvirt code too in which case they would likely be genuine bugs. eg someone declares 'foo' at the start of a method and some time later redeclares it in a for/while loop body or some such.
Yes, but -Wshadow-declarations catches that on newer gcc. Thus, my proposal is: older gcc: omit the warning option, since it is prone to noise that devs on newer systems have to fix after the fact newer gcc: use -Wshadow-declarations, and catch the real problems (and not the conflict between global functions and local variables) Most dev work is done on newer gcc and thus will avoid the real problems, and patches submitted from devs on older gcc may cause issues that have to be fixed up by devs on newer machines, but it will be less frequent than the case of devs submitting patches that then cause old gcc to barf on -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
These APIs are exposed under new virsh command 'domtime' which both gets and sets (not at the same time of course :)). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain-monitor.c | 126 +++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 16 ++++++ 2 files changed, 142 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index de4afbb..8e21e37 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1391,6 +1391,126 @@ cleanup: } /* + * "domtime" command + */ +static const vshCmdInfo info_domtime[] = { + {.name = "help", + .data = N_("domain time") + }, + {.name = "desc", + .data = N_("Gets or sets a domain time") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_domtime[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "now", + .type = VSH_OT_BOOL, + .help = N_("set current host time") + }, + {.name = "pretty", + .type = VSH_OT_BOOL, + .help = N_("print domain's time in human readable form") + }, + {.name = "sync", + .type = VSH_OT_BOOL, + .help = N_("instead of setting given time, synchronize from domain's RTC"), + }, + {.name = "time", + .type = VSH_OT_INT, + .help = N_("time to set") + }, + {.name = NULL} +}; + +static bool +cmdDomTime(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + bool ret = false; + bool now = vshCommandOptBool(cmd, "now"); + bool pretty = vshCommandOptBool(cmd, "pretty"); + bool sync = vshCommandOptBool(cmd, "sync"); + bool doSet = false; + long long guest_time; + const char *timezone = NULL; + int rv; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + rv = vshCommandOptLongLong(cmd, "time", &guest_time); + + if (rv < 0) { + /* invalid integer format */ + goto cleanup; + } else if (rv > 0) { + /* --time is used, so set time instead of get time. + * However, --time and --now are mutually exclusive. */ + if (now) { + vshError(ctl, _("--time and --now are mutually exclusive")); + goto cleanup; + } + + /* Neither is --time and --sync */ + if (sync) { + vshError(ctl, _("--time and --sync are mutually exclusive")); + goto cleanup; + + } + doSet = true; + } + + if (sync && now) { + vshError(ctl, _("--sync and --now are mutually exclusive")); + goto cleanup; + } + + /* --now or --sync means setting */ + doSet |= now | sync; + + if (doSet) { + if (now && ((guest_time = time(NULL)) == (time_t) -1)) { + vshError(ctl, _("unable to get current time")); + goto cleanup; + } + if (virDomainSetTime(dom, guest_time, timezone, + sync ? VIR_DOMAIN_TIME_SYNC : 0) < 0) + goto cleanup; + } else { + if (virDomainGetTime(dom, &guest_time, 0) < 0) + goto cleanup; + + if (pretty) { + char timestr[100]; + time_t cur_time = guest_time; + struct tm time_info; + + if (!gmtime_r(&cur_time, &time_info)) { + vshError(ctl, _("Unable to format time")); + goto cleanup; + } + strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info); + + vshPrint(ctl, _("Time: %s"), timestr); + } else { + vshPrint(ctl, _("Time: %llu"), guest_time); + } + } + + ret = true; + +cleanup: + virDomainFree(dom); + return ret; +} + +/* * "list" command */ static const vshCmdInfo info_list[] = { @@ -1946,6 +2066,12 @@ const vshCmdDef domMonitoringCmds[] = { .info = info_domstate, .flags = 0 }, + {.name = "domtime", + .handler = cmdDomTime, + .opts = opts_domtime, + .info = info_domtime, + .flags = 0 + }, {.name = "list", .handler = cmdList, .opts = opts_list, diff --git a/tools/virsh.pod b/tools/virsh.pod index f221475..40cb5b5 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -969,6 +969,22 @@ Convert a domain Id (or UUID) to domain name Returns state about a domain. I<--reason> tells virsh to also print reason for the state. +=item B<domtime> I<domain> { [I<--now>] [I<--pretty>] [I<--sync>] +[I<--time> B<time>] } + +Gets or sets the domain's system time. When run without any arguments +(but I<domain>), the current domain's system time is printed out. The +I<--pretty> modifier can be used to print the time in more human +readable form. When I<--time> B<time> is specified, the domain's time is +not get but set instead. The I<--now> modifier acts like if it was an +alias for I<--time> B<$now>, which means it sets the time that is +currently on the host virsh is running at. In both cases (setting and +getting), time is in seconds relative to Epoch of 1970-01-01 in UTC. +The I<--sync> modifies the set behavior a bit: The time passed is +ignored, but the time to set is read from domain's RTC instead. Please +note, that some hypervisors may require a guest agent to be configured +in order to get or set the guest time. + =item B<domcontrol> I<domain> Returns state of an interface to VMM used to control a domain. For -- 1.8.5.3
On Thu, Feb 13, 2014 at 07:51:44PM +0100, Michal Privoznik wrote:
These APIs are exposed under new virsh command 'domtime' which both gets and sets (not at the same time of course :)).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain-monitor.c | 126 +++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 16 ++++++ 2 files changed, 142 insertions(+)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index de4afbb..8e21e37 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1391,6 +1391,126 @@ cleanup: }
/* + * "domtime" command + */ +static const vshCmdInfo info_domtime[] = { + {.name = "help", + .data = N_("domain time") + }, + {.name = "desc", + .data = N_("Gets or sets a domain time") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_domtime[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "now", + .type = VSH_OT_BOOL, + .help = N_("set current host time") + }, + {.name = "pretty", + .type = VSH_OT_BOOL, + .help = N_("print domain's time in human readable form") + }, + {.name = "sync", + .type = VSH_OT_BOOL, + .help = N_("instead of setting given time, synchronize from domain's RTC"), + }, + {.name = "time", + .type = VSH_OT_INT, + .help = N_("time to set") + }, + {.name = NULL} +}; + +static bool +cmdDomTime(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + bool ret = false; + bool now = vshCommandOptBool(cmd, "now"); + bool pretty = vshCommandOptBool(cmd, "pretty"); + bool sync = vshCommandOptBool(cmd, "sync"); + bool doSet = false; + long long guest_time; + const char *timezone = NULL; + int rv; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + rv = vshCommandOptLongLong(cmd, "time", &guest_time); + + if (rv < 0) { + /* invalid integer format */
vshCommandOptLongLong() does not set an error, please set one.
+ goto cleanup; + } else if (rv > 0) { + /* --time is used, so set time instead of get time. + * However, --time and --now are mutually exclusive. */ + if (now) { + vshError(ctl, _("--time and --now are mutually exclusive")); + goto cleanup; + } + + /* Neither is --time and --sync */ + if (sync) { + vshError(ctl, _("--time and --sync are mutually exclusive")); + goto cleanup; + + } + doSet = true; + } + + if (sync && now) { + vshError(ctl, _("--sync and --now are mutually exclusive")); + goto cleanup; + } +
And VSH_EXCLUSIVE_OPTIONS will deal with the rest for you (or it's _EXPR variant if you already have the booleans in some variable.
+ /* --now or --sync means setting */ + doSet |= now | sync; + + if (doSet) { + if (now && ((guest_time = time(NULL)) == (time_t) -1)) { + vshError(ctl, _("unable to get current time")); + goto cleanup; + } + if (virDomainSetTime(dom, guest_time, timezone,
You don't make the use of 'timezone' anywhere in the code. And it has the same problem as 'time' with older GCCs.
+ sync ? VIR_DOMAIN_TIME_SYNC : 0) < 0) + goto cleanup; + } else { + if (virDomainGetTime(dom, &guest_time, 0) < 0) + goto cleanup; + + if (pretty) { + char timestr[100]; + time_t cur_time = guest_time; + struct tm time_info; + + if (!gmtime_r(&cur_time, &time_info)) { + vshError(ctl, _("Unable to format time")); + goto cleanup; + } + strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
use space instead of dash (hyphen) after the date, better than that is to use "%F" instead of "%Y-%m-%d" and even best would be to use "%c". Question on the side, can you get the timezone from the guest agent, too? That would be great...
+ + vshPrint(ctl, _("Time: %s"), timestr); + } else { + vshPrint(ctl, _("Time: %llu"), guest_time); + } + } + + ret = true; + +cleanup: + virDomainFree(dom); + return ret; +} + +/* * "list" command */ static const vshCmdInfo info_list[] = { @@ -1946,6 +2066,12 @@ const vshCmdDef domMonitoringCmds[] = { .info = info_domstate, .flags = 0 }, + {.name = "domtime", + .handler = cmdDomTime, + .opts = opts_domtime, + .info = info_domtime, + .flags = 0 + }, {.name = "list", .handler = cmdList, .opts = opts_list, diff --git a/tools/virsh.pod b/tools/virsh.pod index f221475..40cb5b5 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -969,6 +969,22 @@ Convert a domain Id (or UUID) to domain name Returns state about a domain. I<--reason> tells virsh to also print reason for the state.
+=item B<domtime> I<domain> { [I<--now>] [I<--pretty>] [I<--sync>] +[I<--time> B<time>] } + +Gets or sets the domain's system time. When run without any arguments +(but I<domain>), the current domain's system time is printed out. The
or '--pretty', I'd reword it to say '--now', '--time' and '--sync' set the time in the guest, if none of them is specified, then the current time is printed out. It also clears out the rest of the paragraph.
+I<--pretty> modifier can be used to print the time in more human +readable form. When I<--time> B<time> is specified, the domain's time is +not get but set instead. The I<--now> modifier acts like if it was an +alias for I<--time> B<$now>, which means it sets the time that is +currently on the host virsh is running at. In both cases (setting and +getting), time is in seconds relative to Epoch of 1970-01-01 in UTC. +The I<--sync> modifies the set behavior a bit: The time passed is +ignored, but the time to set is read from domain's RTC instead. Please +note, that some hypervisors may require a guest agent to be configured +in order to get or set the guest time. + =item B<domcontrol> I<domain>
Returns state of an interface to VMM used to control a domain. For -- 1.8.5.3
Other than the mentioned nuances the patch looks fine. Martin
One caveat though, qemu-ga is expecting time and returning time in nanoseconds. With all the buffering and propagation delay, the time is already wrong once it gets to the qemu-ga, but there's nothing we can do about it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 81 +++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 6 +++ src/qemu/qemu_driver.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 222 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 4a3820c..28f14ea 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1657,3 +1657,84 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, return 0; } + + +int +qemuAgentGetTime(qemuAgentPtr mon, + long long *time) +{ + int ret = -1; + unsigned long long json_time; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuAgentMakeCommand("guest-get-time", + NULL); + if (!cmd) + return ret; + + if (qemuAgentCommand(mon, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (!reply || qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + if (virJSONValueObjectGetNumberUlong(reply, "return", &json_time) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + goto cleanup; + } + + /* guest agent returns time in nanoseconds, + * we need it in seconds here */ + *time = json_time / 1000000000LL; + ret = 0; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + +/** + * qemuAgentSetTime: + * @sync: let guest agent to read domain's RTC (@time is ignored) + */ +int +qemuAgentSetTime(qemuAgentPtr mon, + long long time, + bool sync) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (sync) { + cmd = qemuAgentMakeCommand("guest-set-time", NULL); + } else { + /* guest agent expect time with nanosecond granularity. + * Impressing. */ + unsigned long long json_time = time * 1000000000LL; + cmd = qemuAgentMakeCommand("guest-set-time", + "U:time", json_time, + NULL); + } + + if (!cmd) + return ret; + + if (qemuAgentCommand(mon, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (!reply || qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 5fbacdb..4618f84 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -97,4 +97,10 @@ int qemuAgentSetVCPUs(qemuAgentPtr mon, qemuAgentCPUInfoPtr cpus, size_t ncpus); int qemuAgentUpdateCPUInfo(unsigned int nvcpus, qemuAgentCPUInfoPtr cpuinfo, int ncpuinfo); + +int qemuAgentGetTime(qemuAgentPtr mon, + long long *time); +int qemuAgentSetTime(qemuAgentPtr mon, + long long time, + bool sync); #endif /* __QEMU_AGENT_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 59e018d..7ed7120 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16493,6 +16493,139 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, } +static int +qemuDomainGetTime(virDomainPtr dom, + long long *time, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(0, ret); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + priv = vm->privateData; + + if (virDomainGetTimeEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + if (!priv->agent) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + + if (priv->agentError) { + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("QEMU guest agent is not " + "available due to an error")); + goto cleanup; + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + qemuDomainObjEnterAgent(vm); + ret = qemuAgentGetTime(priv->agent, time); + qemuDomainObjExitAgent(vm); + +endjob: + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int +qemuDomainSetTime(virDomainPtr dom, + long long set_time, + const char *timezone, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + bool sync = flags & VIR_DOMAIN_TIME_SYNC; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(VIR_DOMAIN_TIME_SYNC, ret); + + if (timezone) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Supplying timezone is not supported yet")); + return ret; + } + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + priv = vm->privateData; + + if (virDomainSetTimeEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + if (!priv->agent) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + + if (priv->agentError) { + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("QEMU guest agent is not " + "available due to an error")); + goto cleanup; + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + qemuDomainObjEnterAgent(vm); + ret = qemuAgentSetTime(priv->agent, set_time, sync); + qemuDomainObjExitAgent(vm); + +endjob: + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU_DRIVER_NAME, @@ -16680,6 +16813,8 @@ static virDriver qemuDriver = { .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ + .domainGetTime = qemuDomainGetTime, /* 1.2.2 */ + .domainSetTime = qemuDomainSetTime, /* 1.2.2 */ }; -- 1.8.5.3
On Thu, Feb 13, 2014 at 07:51:45PM +0100, Michal Privoznik wrote:
One caveat though, qemu-ga is expecting time and returning time in nanoseconds. With all the buffering and propagation delay, the time is already wrong once it gets to the qemu-ga, but there's nothing we can do about it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 81 +++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 6 +++ src/qemu/qemu_driver.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 222 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 4a3820c..28f14ea 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1657,3 +1657,84 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus,
return 0; } + + +int +qemuAgentGetTime(qemuAgentPtr mon, + long long *time) +{ + int ret = -1; + unsigned long long json_time; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuAgentMakeCommand("guest-get-time", + NULL); + if (!cmd) + return ret; + + if (qemuAgentCommand(mon, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (!reply || qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; +
I don't like that qemu is not that introspectable for us to know whether it has the 'sync' functionality, because otherwise it will fail with not-very-descriptive internal error :( However, I don't see an easy way out of it.
+ if (virJSONValueObjectGetNumberUlong(reply, "return", &json_time) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + goto cleanup; + } + + /* guest agent returns time in nanoseconds, + * we need it in seconds here */ + *time = json_time / 1000000000LL; + ret = 0; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + +/** + * qemuAgentSetTime: + * @sync: let guest agent to read domain's RTC (@time is ignored) + */ +int +qemuAgentSetTime(qemuAgentPtr mon, + long long time, + bool sync) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (sync) { + cmd = qemuAgentMakeCommand("guest-set-time", NULL); + } else { + /* guest agent expect time with nanosecond granularity.
s/expect/expects/
+ * Impressing. */
s/Impressing/Impressive/ ;-) Definitely, especially when it takes so long to communicate with him sometimes :)
+ unsigned long long json_time = time * 1000000000LL; + cmd = qemuAgentMakeCommand("guest-set-time", + "U:time", json_time, + NULL); + } + + if (!cmd) + return ret; + + if (qemuAgentCommand(mon, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (!reply || qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 5fbacdb..4618f84 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -97,4 +97,10 @@ int qemuAgentSetVCPUs(qemuAgentPtr mon, qemuAgentCPUInfoPtr cpus, size_t ncpus); int qemuAgentUpdateCPUInfo(unsigned int nvcpus, qemuAgentCPUInfoPtr cpuinfo, int ncpuinfo); + +int qemuAgentGetTime(qemuAgentPtr mon, + long long *time); +int qemuAgentSetTime(qemuAgentPtr mon, + long long time, + bool sync); #endif /* __QEMU_AGENT_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 59e018d..7ed7120 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c [...] +static int +qemuDomainSetTime(virDomainPtr dom, + long long set_time, + const char *timezone, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + bool sync = flags & VIR_DOMAIN_TIME_SYNC; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(VIR_DOMAIN_TIME_SYNC, ret); + + if (timezone) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Supplying timezone is not supported yet")); + return ret;
OK, now I get why you didn't use that variable :) Are you planning on adding the possibility into qemu driver or it will be available in guest agent? Not that it matters for this patch, just curious. Rest looks fine, Martin
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Ján Tomko -
Martin Kletzander -
Michal Privoznik