On 01/29/2013 02:29 AM, Eric Blake wrote:
On 01/27/2013 11:14 AM, Lei Li wrote:
Signed-off-by: Lei Li <li...@linux.vnet.ibm.com>
---
include/qapi/qmp/qerror.h | 3 +++
qga/commands-posix.c | 30 ++++++++++++++++++++++++++++++
qga/qapi-schema.json | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..0baf1a4 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -129,6 +129,9 @@ void assert_no_error(Error *err);
#define QERR_FEATURE_DISABLED \
ERROR_CLASS_GENERIC_ERROR, "The feature '%s' is not enabled"
+#define QERR_GET_TIME_FAILED \
+ ERROR_CLASS_GENERIC_ERROR, "Failed to get time"
+
These days, you shouldn't be defining a new QERR wrapper. Instead,...[1]
+static TimeInfo *get_host_time(Error **errp)
This name is unusual...[2]
Okay, I think there might be misunderstanding here.
The current implementation of this 'guest-get-time'
command is trying to get the* *_host_ time, since the guest
time might be not correct. This qemu_gettimeofday should
return the host time, or am I wrong?
+{
+ int ret;
+ qemu_timeval tq;
+ TimeInfo *time_info;
+
+ ret = qemu_gettimeofday(&tq);
+ if (ret < 0) {
+ error_set_errno(errp, errno, QERR_GET_TIME_FAILED);
[1]...use the right idiom here:
error_setg_errno(errp, errno, "Failed to get time");
+ return NULL;
+ }
+
+ time_info = g_malloc0(sizeof(TimeInfo));
+ time_info->seconds = tq.tv_sec;
+ time_info->microseconds = tq.tv_usec;
Is microseconds the right unit to expose through JSON, or are we going
to wish we had exposed nanoseconds down the road (you can still use
qemu_gettimeofday() and scale tv_usec by 1000 before assigning to
time_info->nanoseconds, if we desire the extra granularity).
You aren't setting time_info->utc_offset. Is that intentional?
Yes, since 'guest-get-time' is supposed to get host time in UTC, the utc_offset
is zero. When guest want to set different time zone it can set the
utc_offset through 'guest-set-time'.
+
+ return time_info;
+}
+
+struct TimeInfo *qmp_guest_get_time(Error **errp)
+{
+ TimeInfo *time_info = get_host_time(errp);
[2]...given that it is only ever called from qmp_guest_get_time.
+
+ if (!time_info) {
+ return NULL;
+ }
These three lines can be deleted,
+
+ return time_info;
since this line will do the same thing if you let NULL time_info get
this far.
+++ b/qga/qapi-schema.json
@@ -83,6 +83,44 @@
{ 'command': 'guest-ping' }
##
+# @TimeInfo
+#
+# Time Information. It is relative to the Epoch of 1970-01-01.
+#
+# @seconds: "seconds" time unit.
+#
+# @microseconds: "microseconds" time unit.
+#
+# @utc-offset: Information about utc offset. Represented as:
+# ±[mmmm] based at a minimum on minutes, with
s/based at a minimum on//
This still doesn't state whether two hours east of UTC is represented as
120 or as 0200.
+# negative values are west and positive values
+# are east of UTC. The bounds of @utc-offset is
+# at most 24 hours away from UTC.
+#
+# Since: 1.4
+##
+{ 'type': 'TimeInfo',
+ 'data': { 'seconds': 'int', 'microseconds': 'int',
+ 'utc-offset': 'int' } }
+
+##
+# @guest-get-time:
+#
+# Get the information about host time in UTC and the
s/host/guest/
+# UTC offset.
+#
+# This command tries to get the host time which is
+# presumably correct, since we need to be able to
+# resynchronize guest clock to host's in guest.
This sentence doesn't make sense. Better would be:
Get the guest's notion of the current time.
+#
+# Returns: @TimeInfo on success.
+#
+# Since 1.4
+##
+{ 'command': 'guest-get-time',
+ 'returns': 'TimeInfo' }
+
+##
# @GuestAgentCommandInfo:
#
# Information about guest agent commands.
--
Lei
