Eric Blake <ebl...@redhat.com> writes: > 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] > >> +{ >> + 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?
Moreover, there's no need to specify microseconds and seconds. A 64-bit value for nanoseconds is sufficient. A signed value can represent 1678 -> 2262. If anyone is using qemu-ga in 2262, they'll have to introduce a new command for their quantum emulators :-) Setting time independent of date is a bit silly because time cannot be interpreted correctly without a date. A single value of nanoseconds since the epoch (interpreted as UTC/GMT time) is really the best strategy. The host shouldn't be involved in guest time zones. In a Cloud environment, it's pretty normal to have different guests using different time zones. Regards, Anthony Liguori > >> + >> + 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. >> > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org