Re: [Qemu-devel] [PATCH 1/2] qga: add windows implementation for guest-get-time

[mdroth <mdroth@linux.vnet.ibm.com>]

On Thu, Mar 14, 2013 at 03:07:50PM +0800, Lei Li wrote:
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  qga/commands-win32.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 7e8ecb3..e24fb4a 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -22,6 +22,12 @@
>  #define SHTDN_REASON_FLAG_PLANNED 0x80000000
>  #endif
> 
> +/* multiple of 100 nanoseconds elapsed between windows baseline
> +   (1/1/1601) and Unix Epoch (1/1/1970), accounting for leap years */
> +#define W32_FT_OFFSET (10000000ULL * 60 * 60 * 24 * \
> +                       (365 * (1970 - 1601) +       \
> +                        (1970 - 1601) / 4 - 3))
> +
>  static void acquire_privilege(const char *name, Error **err)
>  {
>      HANDLE token;
> @@ -108,6 +114,33 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
>      }
>  }
> 
> +int64_t qmp_guest_get_time(Error **errp)
> +{
> +    SYSTEMTIME *ts = g_malloc0(sizeof(SYSTEMTIME));

I still don't understand why we do just do:

SYSTEM ts = {0};

> +    int64_t time_ns;
> +    FILETIME tf;
> +
> +    GetSystemTime(ts);

followed by:

GetSystemTime(&ts);

(and same for SystemTimeToFileTime() below)

This would avoid the need to add common cleanup code for all the
return paths.

> +    if (!ts) {

this is gonna always be false since we initialize it at the start of
this function.

also, GetSystemTime() does seem to provide any error indication
whatsoever, which is strange. But it also doesn't seem to have any
guarantee this it will always succeed...

I think the best we could do is probably just some kind of sanity
check, like making sure ts.wYear != 0, or maybe that
1601 <= ts.wYear <= 30827

> +        error_setg(errp, "Failed to get time");
> +        goto out;
> +    }
> +
> +    if (!SystemTimeToFileTime(ts, &tf)) {
> +        error_setg_errno(errp, errno, "Failed to convert system time");
> +        goto out;
> +    }
> +
> +    time_ns = ((((int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime)
> +                - W32_FT_OFFSET) * 100;
> +
> +    return time_ns;
> +
> +out:
> +    g_free(ts);
> +    return -1;
> +}
> +
>  int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err)
>  {
>      error_set(err, QERR_UNSUPPORTED);
> -- 
> 1.7.11.7
>