[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] qga: Don't require 'time' argument in guest-
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH v2] qga: Don't require 'time' argument in guest-set-time command |
Date: |
Sun, 23 Feb 2014 19:00:46 -0600 |
User-agent: |
alot/0.3.4 |
Quoting Michal Privoznik (2014-01-31 04:29:51)
> As the description to the guest-set-time states, the command is
> there to ease time synchronization after resume. If guest was
> suspended for longer period of time, its system time can go off
> so badly, that even NTP refuses to set it. That's why the command
> was invented: to give users chance to set the time (not
> necessarily 100% correct). However, there's is no real need for
> us to require users to pass an arbitrary time. Especially if we
> can read the correct value from RTC (boiling down to reading
> host's time). Hence this commit enables logic:
>
> guest-set-time() == guest-set-time($now_from_rtc)
>
> Signed-off-by: Michal Privoznik <address@hidden>
Thanks, applied to qga tree:
https://github.com/mdroth/qemu/commits/qga
> ---
> diff to v1:
> -Fix checkpatch.pl warnings
>
> qga/commands-posix.c | 41 +++++++++++++++++++++++++----------------
> qga/commands-win32.c | 34 +++++++++++++++++++++++-----------
> qga/qapi-schema.json | 9 +++++----
> 3 files changed, 53 insertions(+), 31 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 8100bee..7d6665a 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -142,7 +142,7 @@ int64_t qmp_guest_get_time(Error **errp)
> return time_ns;
> }
>
> -void qmp_guest_set_time(int64_t time_ns, Error **errp)
> +void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
> {
> int ret;
> int status;
> @@ -150,22 +150,28 @@ void qmp_guest_set_time(int64_t time_ns, Error **errp)
> Error *local_err = NULL;
> struct timeval tv;
>
> - /* year-2038 will overflow in case time_t is 32bit */
> - if (time_ns / 1000000000 != (time_t)(time_ns / 1000000000)) {
> - error_setg(errp, "Time %" PRId64 " is too large", time_ns);
> - return;
> + /* If user has passed a time, validate and set it. */
> + if (has_time) {
> + /* year-2038 will overflow in case time_t is 32bit */
> + if (time_ns / 1000000000 != (time_t)(time_ns / 1000000000)) {
> + error_setg(errp, "Time %" PRId64 " is too large", time_ns);
> + return;
> + }
> +
> + tv.tv_sec = time_ns / 1000000000;
> + tv.tv_usec = (time_ns % 1000000000) / 1000;
> +
> + ret = settimeofday(&tv, NULL);
> + if (ret < 0) {
> + error_setg_errno(errp, errno, "Failed to set time to guest");
> + return;
> + }
> }
>
> - tv.tv_sec = time_ns / 1000000000;
> - tv.tv_usec = (time_ns % 1000000000) / 1000;
> -
> - ret = settimeofday(&tv, NULL);
> - if (ret < 0) {
> - error_setg_errno(errp, errno, "Failed to set time to guest");
> - return;
> - }
> -
> - /* Set the Hardware Clock to the current System Time. */
> + /* Now, if user has passed a time to set and the system time is set, we
> + * just need to synchronize the hardware clock. However, if no time was
> + * passed, user is requesting the opposite: set the system time from the
> + * hardware clock. */
> pid = fork();
> if (pid == 0) {
> setsid();
> @@ -173,7 +179,10 @@ void qmp_guest_set_time(int64_t time_ns, Error **errp)
> reopen_fd_to_null(1);
> reopen_fd_to_null(2);
>
> - execle("/sbin/hwclock", "hwclock", "-w", NULL, environ);
> + /* Use '/sbin/hwclock -w' to set RTC from the system time,
> + * or '/sbin/hwclock -s' to set the system time from RTC. */
> + execle("/sbin/hwclock", "hwclock", has_time ? "-w" : "-s",
> + NULL, environ);
> _exit(EXIT_FAILURE);
> } else if (pid < 0) {
> error_setg_errno(errp, errno, "failed to create child process");
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index a6a0af2..8245f95 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -370,25 +370,37 @@ int64_t qmp_guest_get_time(Error **errp)
> return time_ns;
> }
>
> -void qmp_guest_set_time(int64_t time_ns, Error **errp)
> +void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
> {
> SYSTEMTIME ts;
> FILETIME tf;
> LONGLONG time;
>
> - if (time_ns < 0 || time_ns / 100 > INT64_MAX - W32_FT_OFFSET) {
> - error_setg(errp, "Time %" PRId64 "is invalid", time_ns);
> - return;
> - }
> + if (has_time) {
> + /* Okay, user passed a time to set. Validate it. */
> + if (time_ns < 0 || time_ns / 100 > INT64_MAX - W32_FT_OFFSET) {
> + error_setg(errp, "Time %" PRId64 "is invalid", time_ns);
> + return;
> + }
>
> - time = time_ns / 100 + W32_FT_OFFSET;
> + time = time_ns / 100 + W32_FT_OFFSET;
>
> - tf.dwLowDateTime = (DWORD) time;
> - tf.dwHighDateTime = (DWORD) (time >> 32);
> + tf.dwLowDateTime = (DWORD) time;
> + tf.dwHighDateTime = (DWORD) (time >> 32);
>
> - if (!FileTimeToSystemTime(&tf, &ts)) {
> - error_setg(errp, "Failed to convert system time %d",
> (int)GetLastError());
> - return;
> + if (!FileTimeToSystemTime(&tf, &ts)) {
> + error_setg(errp, "Failed to convert system time %d",
> + (int)GetLastError());
> + return;
> + }
> + } else {
> + /* Otherwise read the time from RTC which contains the correct value.
> + * Hopefully. */
> + GetSystemTime(&ts);
> + if (ts.wYear < 1601 || ts.wYear > 30827) {
> + error_setg(errp, "Failed to get time");
> + return;
> + }
> }
>
> acquire_privilege(SE_SYSTEMTIME_NAME, errp);
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 245f968..80edca1 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -120,17 +120,18 @@
> # This command tries to set guest time to the given value,
> # then sets the Hardware Clock to the current System Time.
> # This will make it easier for a guest to resynchronize
> -# without waiting for NTP.
> +# without waiting for NTP. If no @time is specified, then
> +# the time to set is read from RTC.
> #
> -# @time: time of nanoseconds, relative to the Epoch of
> -# 1970-01-01 in UTC.
> +# @time: #optional time of nanoseconds, relative to the Epoch
> +# of 1970-01-01 in UTC.
> #
> # Returns: Nothing on success.
> #
> # Since: 1.5
> ##
> { 'command': 'guest-set-time',
> - 'data': { 'time': 'int' } }
> + 'data': { '*time': 'int' } }
>
> ##
> # @GuestAgentCommandInfo:
> --
> 1.8.5.2
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2] qga: Don't require 'time' argument in guest-set-time command,
Michael Roth <=