Re: [win-pv-devel] [PATCH 09/20] Resolve lifecycle bugs

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf Of Owen Smith
> Sent: 24 May 2016 15:21
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith
> Subject: [win-pv-devel] [PATCH 09/20] Resolve lifecycle bugs
> 
> * make CDevice destructor virtual
> * call IDeviceCreator::OnDeviceRemoved when cleaning up device list
>   on service stop
> * add logging to OutputDebugString
>

It looks like this is bug-fixing a previous patch (patch #6 at a guest) so can 
it not be folded in?

  Paul
 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>  src/liteagent/DeviceList.cpp | 10 ++++++++--
>  src/liteagent/DeviceList.h   |  2 +-
>  src/liteagent/LiteAgent.cpp  | 19 ++++++++++++++++---
>  src/liteagent/LiteAgent.h    |  4 +++-
>  4 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/src/liteagent/DeviceList.cpp b/src/liteagent/DeviceList.cpp
> index 3fc9a85..ce0ccd2 100644
> --- a/src/liteagent/DeviceList.cpp
> +++ b/src/liteagent/DeviceList.cpp
> @@ -55,7 +55,7 @@ CDevice::CDevice(const wchar_t* path) :
>  {
>  }
> 
> -CDevice::~CDevice()
> +/*virtual*/ CDevice::~CDevice()
>  {
>      Close();
>  }
> @@ -195,6 +195,8 @@ void CDeviceList::Stop()
>      for (DeviceMap::iterator it = m_devs.begin();
>              it != m_devs.end();
>              ++it) {
> +        if (m_impl)
> +            m_impl->OnDeviceRemoved(it->second);
>          delete it->second;
>      }
>      m_devs.clear();
> @@ -277,6 +279,9 @@ void CDeviceList::OnDeviceQueryRemove(HANDLE
> handle)
>          return; // spurious event?
> 
>      CDevice* dev = it->second;
> +    if (dev == NULL)
> +        return; // bad map entry
> +
>      if (m_impl)
>          m_impl->OnDeviceRemoved(dev);
>      dev->Close();
> @@ -289,6 +294,7 @@ void CDeviceList::OnDeviceRemoved(HANDLE handle)
>          return; // spurious event?
> 
>      CDevice* dev = it->second;
> -    delete dev;
> +    if (dev)
> +        delete dev;
>      m_devs.erase(it);
>  }
> diff --git a/src/liteagent/DeviceList.h b/src/liteagent/DeviceList.h
> index 6a1b0b4..6c8b6cb 100644
> --- a/src/liteagent/DeviceList.h
> +++ b/src/liteagent/DeviceList.h
> @@ -41,7 +41,7 @@ class CDevice
>  {
>  public:
>      CDevice(const wchar_t* path);
> -    ~CDevice();
> +    virtual ~CDevice();
> 
>      HANDLE Open(HANDLE svc);
>      void Close();
> diff --git a/src/liteagent/LiteAgent.cpp b/src/liteagent/LiteAgent.cpp
> index 908c0c9..ba48fc0 100644
> --- a/src/liteagent/LiteAgent.cpp
> +++ b/src/liteagent/LiteAgent.cpp
> @@ -31,6 +31,7 @@
> 
>  #define INITGUID
>  #include <windows.h>
> +#include <stdio.h>
> 
>  #include "LiteAgent.h"
>  #include "xeniface_ioctls.h"
> @@ -50,6 +51,14 @@ static CLiteAgent s_service;
> 
>  /*static*/ void CLiteAgent::Log(const char* fmt, ...)
>  {
> +    char message[256];
> +    va_list args;
> +
> +    va_start(args, fmt);
> +    vsnprintf_s(message, sizeof(message),
> sizeof(message)/sizeof(message[0]) - 1, fmt, args);
> +    va_end(args);
> +
> +    OutputDebugString(message);
>  }
> 
>  /*static*/ int CLiteAgent::ServiceInstall()
> @@ -127,11 +136,15 @@ CLiteAgent::CLiteAgent() : m_handle(NULL),
> m_devs(GUID_INTERFACE_XENIFACE), m_de
>      m_status.dwWaitHint           = 0;
> 
>      m_svc_stop = CreateEvent(FALSE, NULL, NULL, FALSE);
> +    m_shutdown = CreateEvent(FALSE, NULL, NULL, FALSE);
> +    m_suspend  = CreateEvent(FALSE, NULL, NULL, FALSE);
>  }
> 
>  CLiteAgent::~CLiteAgent()
>  {
>      CloseHandle(m_svc_stop);
> +    CloseHandle(m_shutdown);
> +    CloseHandle(m_suspend);
>  }
> 
>  /*virtual*/ CDevice* CLiteAgent::Create(const wchar_t* path)
> @@ -146,6 +159,7 @@ CLiteAgent::~CLiteAgent()
>      if (m_dev == NULL) {
>          m_dev = (CXenIfaceItf*)dev;
>          // setting active device
> +        CLiteAgent::Log("Starting Active Device\n");
>      }
>  }
> 
> @@ -154,7 +168,8 @@ CLiteAgent::~CLiteAgent()
>      CLiteAgent::Log("OnDeviceRemoved(%ws)\n", dev->Path());
>      if ((CXenIfaceItf*)dev == m_dev) {
>          m_dev = NULL;
> -        // active device
> +        // active device removed
> +        CLiteAgent::Log("Active Device Removed\n");
>      }
>  }
> 
> @@ -172,7 +187,6 @@ void CLiteAgent::OnServiceStop()
> 
>  void CLiteAgent::OnDeviceEvent(DWORD evt, LPVOID data)
>  {
> -    CLiteAgent::Log("OnDeviceEvent()\n");
>      m_devs.OnDeviceEvent(evt, data);
>  }
> 
> @@ -220,7 +234,6 @@ DWORD WINAPI
> CLiteAgent::__ServiceControlHandlerEx(DWORD req, DWORD evt, LPVOID
>          return NO_ERROR;
> 
>      case SERVICE_CONTROL_DEVICEEVENT:
> -        CLiteAgent::Log("SERVICE_CONTROL_DEVICEEVENT\n");
>          SetServiceStatus(SERVICE_RUNNING);
>          OnDeviceEvent(evt, data);
>          return NO_ERROR;
> diff --git a/src/liteagent/LiteAgent.h b/src/liteagent/LiteAgent.h
> index 3680afd..2ec1f55 100644
> --- a/src/liteagent/LiteAgent.h
> +++ b/src/liteagent/LiteAgent.h
> @@ -76,7 +76,9 @@ private: // service support
>      SERVICE_STATUS          m_status;
>      SERVICE_STATUS_HANDLE   m_handle;
>      HANDLE                  m_svc_stop;
> -
> +    HANDLE                  m_shutdown;
> +    HANDLE                  m_suspend;
> +
>      CDeviceList             m_devs;
>      CXenIfaceItf*           m_dev;
>  };
> --
> 1.9.4.msysgit.1
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel