From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43545) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDVoW-0007wY-VF for qemu-devel@nongnu.org; Thu, 07 Mar 2013 03:04:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDVoV-0006Hl-Ln for qemu-devel@nongnu.org; Thu, 07 Mar 2013 03:04:32 -0500 Received: from e28smtp09.in.ibm.com ([122.248.162.9]:41476) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDVoV-0006HN-1f for qemu-devel@nongnu.org; Thu, 07 Mar 2013 03:04:31 -0500 Received: from /spool/local by e28smtp09.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 7 Mar 2013 13:32:15 +0530 Received: from d28relay02.in.ibm.com (d28relay02.in.ibm.com [9.184.220.59]) by d28dlp01.in.ibm.com (Postfix) with ESMTP id C5007E0053 for ; Thu, 7 Mar 2013 13:35:39 +0530 (IST) Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay02.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r2784M9m23199900 for ; Thu, 7 Mar 2013 13:34:22 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r2784MT5007374 for ; Thu, 7 Mar 2013 19:04:22 +1100 Message-ID: <513849FC.4040109@linux.vnet.ibm.com> Date: Thu, 07 Mar 2013 16:04:12 +0800 From: Lei Li MIME-Version: 1.0 References: <1362577504-7994-1-git-send-email-lilei@linux.vnet.ibm.com> <1362577504-7994-3-git-send-email-lilei@linux.vnet.ibm.com> <51375B2A.5000200@redhat.com> In-Reply-To: <51375B2A.5000200@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] qga: add windows implementation for guest-set-time List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com 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 >> --- >> 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. -- Lei