Message ID | 1362577504-7994-3-git-send-email-lilei@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On 03/06/2013 06:45 AM, Lei Li wrote: > Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> > --- > qga/commands-win32.c | 35 +++++++++++++++++++++++++++++++++++ > 1 files changed, 35 insertions(+), 0 deletions(-) > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 4febec7..1a90aa7 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -136,6 +136,41 @@ int64_t qmp_guest_get_time(Error **errp) > return time_ns; > } > > +void qmp_guest_set_time(int64_t time_ns, Error **errp) > +{ > + SYSTEMTIME ts; > + FILETIME tf; > + LONGLONG 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; > + } Do you really need this? That is, don't you already know what size time_t is on windows; not to mention that on Windows, you aren't going through time_t, but directly to tf.dwHighDateTime. > + > + acquire_privilege(SE_SYSTEMTIME_NAME, errp); > + if (error_is_set(errp)) { > + error_setg(errp, "Failed to acquire privilege"); > + return; > + } > + > + time = time_ns / 100 + _W32_FT_OFFSET; On the other hand, _this_ operation can overflow, so you should be checking that time_ns doesn't result in an unexpected time value. > + > + tf.dwLowDateTime = (DWORD) time; > + tf.dwHighDateTime = (DWORD) (time >> 32); > + > + if (!FileTimeToSystemTime(&tf, &ts)) { > + error_setg(errp, "Failed to convert system time"); > + return; > + } > + > + if (!SetSystemTime(&ts)) { > + slog("guest-set-time failed: %d", GetLastError()); > + error_setg_errno(errp, errno, "Failed to set time to guest"); > + return; > + } Trailing whitespace. Run your submission through scripts/checkpatch.pl. Should you relinquish the SE_SYSTEMTIME_NAME privilege when exiting this function, instead of leaving it always enabled for the remaining life of the agent service?
On 03/06/2013 11:05 PM, Eric Blake wrote: > On 03/06/2013 06:45 AM, Lei Li wrote: >> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> >> --- >> qga/commands-win32.c | 35 +++++++++++++++++++++++++++++++++++ >> 1 files changed, 35 insertions(+), 0 deletions(-) >> >> diff --git a/qga/commands-win32.c b/qga/commands-win32.c >> index 4febec7..1a90aa7 100644 >> --- a/qga/commands-win32.c >> +++ b/qga/commands-win32.c >> @@ -136,6 +136,41 @@ int64_t qmp_guest_get_time(Error **errp) >> return time_ns; >> } >> >> +void qmp_guest_set_time(int64_t time_ns, Error **errp) >> +{ >> + SYSTEMTIME ts; >> + FILETIME tf; >> + LONGLONG 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; >> + } > Do you really need this? That is, don't you already know what size > time_t is on windows; not to mention that on Windows, you aren't going > through time_t, but directly to tf.dwHighDateTime. You are right. > >> + >> + acquire_privilege(SE_SYSTEMTIME_NAME, errp); >> + if (error_is_set(errp)) { >> + error_setg(errp, "Failed to acquire privilege"); >> + return; >> + } >> + >> + time = time_ns / 100 + _W32_FT_OFFSET; > On the other hand, _this_ operation can overflow, so you should be > checking that time_ns doesn't result in an unexpected time value. OK. >> + >> + tf.dwLowDateTime = (DWORD) time; >> + tf.dwHighDateTime = (DWORD) (time >> 32); >> + >> + if (!FileTimeToSystemTime(&tf, &ts)) { >> + error_setg(errp, "Failed to convert system time"); >> + return; >> + } >> + >> + if (!SetSystemTime(&ts)) { >> + slog("guest-set-time failed: %d", GetLastError()); >> + error_setg_errno(errp, errno, "Failed to set time to guest"); >> + return; >> + } > Trailing whitespace. Run your submission through scripts/checkpatch.pl. Yes. > Should you relinquish the SE_SYSTEMTIME_NAME privilege when exiting this > function, instead of leaving it always enabled for the remaining life of > the agent service? According to the remarks on msdn, this SetSystemTime function will disables the SE_SYSTEMTIME_NAME privilege before returning.
diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 4febec7..1a90aa7 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -136,6 +136,41 @@ int64_t qmp_guest_get_time(Error **errp) return time_ns; } +void qmp_guest_set_time(int64_t time_ns, Error **errp) +{ + SYSTEMTIME ts; + FILETIME tf; + LONGLONG 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; + } + + acquire_privilege(SE_SYSTEMTIME_NAME, errp); + if (error_is_set(errp)) { + error_setg(errp, "Failed to acquire privilege"); + return; + } + + time = time_ns / 100 + _W32_FT_OFFSET; + + tf.dwLowDateTime = (DWORD) time; + tf.dwHighDateTime = (DWORD) (time >> 32); + + if (!FileTimeToSystemTime(&tf, &ts)) { + error_setg(errp, "Failed to convert system time"); + return; + } + + if (!SetSystemTime(&ts)) { + slog("guest-set-time failed: %d", GetLastError()); + error_setg_errno(errp, errno, "Failed to set time to guest"); + return; + } +} + int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err) { error_set(err, QERR_UNSUPPORTED);
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> --- qga/commands-win32.c | 35 +++++++++++++++++++++++++++++++++++ 1 files changed, 35 insertions(+), 0 deletions(-)