Message ID | 1363169431-1561-2-git-send-email-lilei@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On Wed, Mar 13, 2013 at 06:10:30PM +0800, lilei@linux.vnet.ibm.com wrote: > From: Lei Li <lilei@linux.vnet.ibm.com> > > Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> > --- > qga/commands-win32.c | 32 ++++++++++++++++++++++++++++++++ > 1 files changed, 32 insertions(+), 0 deletions(-) > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 7e8ecb3..0a2bb34 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,32 @@ 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)); Don't we need to free this at some point? > + int64_t time_ns; > + union { > + UINT64 ns100; > + FILETIME tf; > + } time; > + > + GetSystemTime(ts); > + if (!ts) { > + slog("guest-get-time failed: %d", GetLastError()); GetSystemTime() does not seem to set an error that can be retrieved by GetLastError(). > + error_setg_errno(errp, errno, "Failed to get time"); > + return -1; > + } > + > + if (!SystemTimeToFileTime(ts, &time.tf)) { > + error_setg_errno(errp, errno, "Failed to convert system time"); > + return -1; > + } > + > + time_ns = (int64_t)((time.ns100 - W32_FT_OFFSET) * 100); I'm not sure how safe this union stuff is. The documentation suggests that in some circumstances the low/high fields in FILETIME might be padded for 64-bit alignment and that doing this type of cast could generate an alignment fault: http://msdn.microsoft.com/en-us/library/windows/desktop/ms724284(v=vs.85).aspx or it might, perhaps even worse, just silently report the wrong time. I think we should just do the math explicitly: (((tf.dwHighDateTime << 32) | tf.dwLowDateTime) - W32_FT_OFFSET) * 100) (or something along that line) > + > + return time_ns; > +} > + > int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err) > { > error_set(err, QERR_UNSUPPORTED); > -- > 1.7.7.6 >
On Wed, Mar 13, 2013 at 03:07:52PM -0500, mdroth wrote: > On Wed, Mar 13, 2013 at 06:10:30PM +0800, lilei@linux.vnet.ibm.com wrote: > > From: Lei Li <lilei@linux.vnet.ibm.com> > > > > Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> > > --- > > qga/commands-win32.c | 32 ++++++++++++++++++++++++++++++++ > > 1 files changed, 32 insertions(+), 0 deletions(-) > > > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > > index 7e8ecb3..0a2bb34 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,32 @@ 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)); > > Don't we need to free this at some point? Actually, do we even need malloc? Can't be just allocate SYSTEMTIME struct on the stack and pass a pointer to GetSystemTime? > > > + int64_t time_ns; > > + union { > > + UINT64 ns100; > > + FILETIME tf; > > + } time; > > + > > + GetSystemTime(ts); > > + if (!ts) { > > + slog("guest-get-time failed: %d", GetLastError()); > > GetSystemTime() does not seem to set an error that can be retrieved by > GetLastError(). > > > + error_setg_errno(errp, errno, "Failed to get time"); > > + return -1; > > + } > > + > > + if (!SystemTimeToFileTime(ts, &time.tf)) { > > + error_setg_errno(errp, errno, "Failed to convert system time"); > > + return -1; > > + } > > + > > + time_ns = (int64_t)((time.ns100 - W32_FT_OFFSET) * 100); > > I'm not sure how safe this union stuff is. The documentation suggests that in > some circumstances the low/high fields in FILETIME might be padded for > 64-bit alignment and that doing this type of cast could generate an > alignment fault: > > http://msdn.microsoft.com/en-us/library/windows/desktop/ms724284(v=vs.85).aspx > > or it might, perhaps even worse, just silently report the wrong time. > > I think we should just do the math explicitly: > > (((tf.dwHighDateTime << 32) | tf.dwLowDateTime) - W32_FT_OFFSET) * 100) > > (or something along that line) > > > + > > + return time_ns; > > +} > > + > > int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err) > > { > > error_set(err, QERR_UNSUPPORTED); > > -- > > 1.7.7.6 > >
On 03/14/2013 04:07 AM, mdroth wrote: > On Wed, Mar 13, 2013 at 06:10:30PM +0800, lilei@linux.vnet.ibm.com wrote: >> From: Lei Li <lilei@linux.vnet.ibm.com> >> >> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> >> --- >> qga/commands-win32.c | 32 ++++++++++++++++++++++++++++++++ >> 1 files changed, 32 insertions(+), 0 deletions(-) >> >> diff --git a/qga/commands-win32.c b/qga/commands-win32.c >> index 7e8ecb3..0a2bb34 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,32 @@ 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)); > Don't we need to free this at some point? > Sorry, my fault here... >> + int64_t time_ns; >> + union { >> + UINT64 ns100; >> + FILETIME tf; >> + } time; >> + >> + GetSystemTime(ts); >> + if (!ts) { >> + slog("guest-get-time failed: %d", GetLastError()); > GetSystemTime() does not seem to set an error that can be retrieved by > GetLastError(). Yes, you are right. >> + error_setg_errno(errp, errno, "Failed to get time"); >> + return -1; >> + } >> + >> + if (!SystemTimeToFileTime(ts, &time.tf)) { >> + error_setg_errno(errp, errno, "Failed to convert system time"); >> + return -1; >> + } >> + >> + time_ns = (int64_t)((time.ns100 - W32_FT_OFFSET) * 100); > I'm not sure how safe this union stuff is. The documentation suggests that in > some circumstances the low/high fields in FILETIME might be padded for > 64-bit alignment and that doing this type of cast could generate an > alignment fault: > > http://msdn.microsoft.com/en-us/library/windows/desktop/ms724284(v=vs.85).aspx > > or it might, perhaps even worse, just silently report the wrong time. > > I think we should just do the math explicitly: > > (((tf.dwHighDateTime << 32) | tf.dwLowDateTime) - W32_FT_OFFSET) * 100) > > (or something along that line) OK, thanks. >> + >> + return time_ns; >> +} >> + >> int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err) >> { >> error_set(err, QERR_UNSUPPORTED); >> -- >> 1.7.7.6 >>
diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 7e8ecb3..0a2bb34 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,32 @@ 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)); + int64_t time_ns; + union { + UINT64 ns100; + FILETIME tf; + } time; + + GetSystemTime(ts); + if (!ts) { + slog("guest-get-time failed: %d", GetLastError()); + error_setg_errno(errp, errno, "Failed to get time"); + return -1; + } + + if (!SystemTimeToFileTime(ts, &time.tf)) { + error_setg_errno(errp, errno, "Failed to convert system time"); + return -1; + } + + time_ns = (int64_t)((time.ns100 - W32_FT_OFFSET) * 100); + + return time_ns; +} + int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err) { error_set(err, QERR_UNSUPPORTED);