Re: [win-pv-devel] [PATCH] Check WinTime and XenTime for drift

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On 
> Behalf Of Owen Smith
> Sent: 15 April 2019 14:39
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH] Check WinTime and XenTime for drift
> 
> Windows 10/Server 2016 changed how Windows maintains the accuracy of its clock
> This can allow reported time to drift away from host time, and Windows time
> will need to be updated regularly in order to minimise the drift.
> 
> * Add a wrapper for SetXenTime to manage the critical section
> * Remove SetXenTime from CheckSuspend and call it from the calling function
> * Add a timeout to the WaitForMultipleObjectsEx, which calls SetXenTime
> * Prevent SetXenTime from updating the time if its not changed (and avoid
>   outputting the log lines)
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

although...

> ---
>  src/xenagent/service.cpp | 20 +++++++++++++++++---
>  src/xenagent/service.h   |  1 +
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/src/xenagent/service.cpp b/src/xenagent/service.cpp
> index a8d45fc..427faee 100644
> --- a/src/xenagent/service.cpp
> +++ b/src/xenagent/service.cpp
> @@ -239,6 +239,15 @@ bool CXenIfaceCreator::CheckShutdown()
>      return false;
>  }
> 
> +void CXenIfaceCreator::CheckXenTime()
> +{
> +    CCritSec crit(&m_crit);
> +    if (m_device == NULL)
> +        return;
> +
> +    SetXenTime();
> +}
> +
>  void CXenIfaceCreator::CheckSuspend()
>  {
>      CCritSec crit(&m_crit);
> @@ -268,7 +277,6 @@ void CXenIfaceCreator::CheckSuspend()
>      if (m_agent.ConvDevicePresent())
>          StartSlateModeWatch();
> 
> -    SetXenTime();
>      m_count = count;
>  }
> 
> @@ -473,6 +481,9 @@ void CXenIfaceCreator::SetXenTime()
>      SYSTEMTIME cur = { 0 };
>      GetLocalTime(&cur);
> 
> +    if (memcmp(&cur, &sys, sizeof(SYSTEMTIME)) == 0)
> +        return;
> +
>      CXenAgent::Log("Time Now = %d/%d/%d %d:%02d:%02d.%d\n",
>                     cur.wYear, cur.wMonth, cur.wDay,
>                     cur.wHour, cur.wMinute, cur.wSecond, cur.wMilliseconds);
> @@ -732,11 +743,12 @@ void CXenAgent::OnPowerEvent(DWORD evt, LPVOID data)
> 
>  bool CXenAgent::ServiceMainLoop()
>  {
> +    DWORD   timeout = 30 * 60 * 1000;

... I suspect this should really be read in from a registry key... but that can 
be done by a subsequent patch.

>      HANDLE  events[] = { m_svc_stop,
>                           m_xeniface.m_evt_shutdown,
>                           m_xeniface.m_evt_suspend,
>                           m_xeniface.m_evt_slate_mode };
> -    DWORD   wait = WaitForMultipleObjectsEx(4, events, FALSE, INFINITE, 
> TRUE);
> +    DWORD   wait = WaitForMultipleObjectsEx(4, events, FALSE, timeout, TRUE);
> 
>      switch (wait) {
>      case WAIT_OBJECT_0:
> @@ -749,6 +761,7 @@ bool CXenAgent::ServiceMainLoop()
> 
>      case WAIT_OBJECT_0+2:
>          ResetEvent(m_xeniface.m_evt_suspend);
> +        m_xeniface.CheckXenTime();
>          m_xeniface.CheckSuspend();
>          return true; // continue loop
> 
> @@ -761,8 +774,9 @@ bool CXenAgent::ServiceMainLoop()
> 
>          return true; // continue loop
>      }
> -    case WAIT_IO_COMPLETION:
>      case WAIT_TIMEOUT:
> +        m_xeniface.CheckXenTime();
> +    case WAIT_IO_COMPLETION:
>          m_xeniface.CheckSuspend();
>          return !m_xeniface.CheckShutdown();
> 
> diff --git a/src/xenagent/service.h b/src/xenagent/service.h
> index 47b7352..4126841 100644
> --- a/src/xenagent/service.h
> +++ b/src/xenagent/service.h
> @@ -69,6 +69,7 @@ public: // IDeviceCreator
> 
>  public:
>      bool CheckShutdown();
> +    void CheckXenTime();
>      void CheckSuspend();
>      bool CheckSlateMode(std::string *mode);
> 
> --
> 2.16.2.windows.1
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel