Re: [win-pv-devel] [PATCH] Add a user mode library wrapper for XENIFACE IOCTLs

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf Of Marek Marczykowski-Górecki
> Sent: 11 July 2018 16:29
> To: Owen Smith <owen.smith@xxxxxxxxxx>
> Cc: win-pv-devel@xxxxxxxxxxxxxxxxxxxx; Rafal Wojdyla
> <omeg@xxxxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [win-pv-devel] [PATCH] Add a user mode library wrapper for
> XENIFACE IOCTLs
> 
> On Wed, Jul 11, 2018 at 03:16:44PM +0000, Owen Smith wrote:
> >
> > > -----Original Message-----
> > > From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx]
> On
> > > Behalf Of Marek Marczykowski-Górecki
> > > Sent: 09 July 2018 11:22
> > > To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> > > Cc: Rafal Wojdyla <omeg@xxxxxxxxxxxxxxxxxxxxxx>; Marek Marczykowski-
> > > Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > > Subject: [win-pv-devel] [PATCH] Add a user mode library wrapper for
> XENIFACE
> > > IOCTLs
> > >
> > > From: Rafal Wojdyla <omeg@xxxxxxxxxxxxxxxxxxxxxx>
> > >
> > > Signed-off-by: Rafal Wojdyla <omeg@xxxxxxxxxxxxxxxxxxxxxx>
> > > [fix compile warnings, update visual studio files]
> > > Signed-off-by: Marek Marczykowski-Górecki
> > > <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > > ---
> > > This was posted before here:
> > > https://lists.xenproject.org/archives/html/win-pv-devel/2015-
> > > 11/msg00014.html
> > >
> > > Back then I've raised a concern about code duplication caused by a
> > > different API than libxc (having libxenvchan in mind). But two years
> > > latter it looks like it isn't such a problem. libxenchan is the only
> > > piece being effectively duplicated (at least in Qubes OS), and
> > > everything else is really different anyway because of Linux/Windows
> > > differences. So, I think it isn't an issue.
> > >
> > > Also I've renamed XcEvtchnBindUnbound to XcEvtchnOpenUnbound, as
> > > requested in review back then.
> > >
> > > This has been tested with vs2017/WDK10 build for Windows 7 64bit, both
> > > on Windows 7 and Windows 10. The patch assume "Add Windows 7 build
> > > target" patches applied, but it should be easy to apply without them
> > > too.
> > > I've updated vs2015 files too, but don't have tools to test them (it
> > > isn't possible to download free vs2015 anymore).
> > > ---
> > >  include/xencontrol.h                         | 342 ++++++++++
> > >  src/xencontrol/xencontrol.c                  | 919
> +++++++++++++++++++++++++++
> > >  src/xencontrol/xencontrol.rc                 |  24 +
> > >  src/xencontrol/xencontrol_private.h          |  49 ++
> > >  vs2015/package/package.vcxproj               |   3 +
> > >  vs2015/xencontrol/xencontrol.vcxproj         |  67 ++
> > >  vs2015/xencontrol/xencontrol.vcxproj.filters |  13 +
> > >  vs2015/xeniface.sln                          |  38 ++
> > >  vs2017/package/package.vcxproj               |   3 +
> > >  vs2017/xencontrol/xencontrol.vcxproj         |  67 ++
> > >  vs2017/xencontrol/xencontrol.vcxproj.filters |  13 +
> > >  vs2017/xeniface.sln                          |  38 ++
> > >  12 files changed, 1576 insertions(+)
> > >  create mode 100644 include/xencontrol.h
> > >  create mode 100644 src/xencontrol/xencontrol.c
> > >  create mode 100644 src/xencontrol/xencontrol.rc
> > >  create mode 100644 src/xencontrol/xencontrol_private.h
> > >  create mode 100644 vs2015/xencontrol/xencontrol.vcxproj
> > >  create mode 100644 vs2015/xencontrol/xencontrol.vcxproj.filters
> > >  create mode 100644 vs2017/xencontrol/xencontrol.vcxproj
> > >  create mode 100644 vs2017/xencontrol/xencontrol.vcxproj.filters
> >
> > [snip]
> >
> > > +
> > > +DWORD
> > > +XcOpen(
> > > +    IN  XENCONTROL_LOGGER *Logger,
> > > +    OUT PXENCONTROL_CONTEXT *Xc
> > > +    )
> > > +{
> > > +    HDEVINFO DevInfo;
> > > +    SP_DEVICE_INTERFACE_DATA InterfaceData;
> > > +    SP_DEVICE_INTERFACE_DETAIL_DATA *DetailData = NULL;
> > > +    DWORD BufferSize;
> > > +    PXENCONTROL_CONTEXT Context;
> > > +
> > > +    Context = malloc(sizeof(*Context));
> > > +    if (Context == NULL)
> > > +        return ERROR_NOT_ENOUGH_MEMORY;
> > > +
> > > +    Context->Logger = Logger;
> > > +    Context->LogLevel = XLL_INFO;
> > > +    Context->RequestId = 1;
> > > +    InitializeListHead(&Context->RequestList);
> > > +    InitializeCriticalSection(&Context->RequestListLock);
> > > +
> > > +    DevInfo = SetupDiGetClassDevs(&GUID_INTERFACE_XENIFACE, 0,
> NULL,
> > > DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
> > > +    if (DevInfo == INVALID_HANDLE_VALUE) {
> > > +        _Log(Logger, XLL_ERROR, Context->LogLevel, __FUNCTION__,
> > > +             L"XENIFACE device class doesn't exist");
> > > +        goto fail;
> > > +    }
> > > +
> > > +    InterfaceData.cbSize = sizeof(InterfaceData);
> > > +    if (!SetupDiEnumDeviceInterfaces(DevInfo, NULL,
> > > &GUID_INTERFACE_XENIFACE, 0, &InterfaceData)) {
> > > +        _Log(Logger, XLL_ERROR, Context->LogLevel, __FUNCTION__,
> > > +             L"Failed to enumerate XENIFACE devices");
> > > +        goto fail;
> > > +    }
> > > +
> > > +    SetupDiGetDeviceInterfaceDetail(DevInfo, &InterfaceData, NULL, 0,
> > > &BufferSize, NULL);
> > > +    if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
> > > +        _Log(Logger, XLL_ERROR, Context->LogLevel, __FUNCTION__,
> > > +             L"Failed to get buffer size for device details");
> > > +        goto fail;
> > > +    }
> > > +
> > > +    // Using 'BufferSize' from failed function call
> > > +#pragma warning(suppress: 6102)
> > > +    DetailData = (SP_DEVICE_INTERFACE_DETAIL_DATA
> *)malloc(BufferSize);
> > > +    if (!DetailData) {
> > > +        SetLastError(ERROR_OUTOFMEMORY);
> > > +        goto fail;
> > > +    }
> > > +
> > > +    DetailData->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA);
> > > +
> > > +    if (!SetupDiGetDeviceInterfaceDetail(DevInfo, &InterfaceData,
> DetailData,
> > > BufferSize, NULL, NULL)) {
> > > +        _Log(Logger, XLL_ERROR, Context->LogLevel, __FUNCTION__,
> > > +             L"Failed to get XENIFACE device path");
> > > +        goto fail;
> > > +    }
> > > +
> > > +    Context->XenIface = CreateFile(DetailData->DevicePath,
> > > +                                   FILE_GENERIC_READ | 
> > > FILE_GENERIC_WRITE,
> > > +                                   0,
> > > +                                   NULL,
> > > +                                   OPEN_EXISTING,
> > > +                                   FILE_ATTRIBUTE_NORMAL | 
> > > FILE_FLAG_OVERLAPPED,
> > > +                                   NULL);
> >
> > No FILE_SHARE_READ/WRITE options set, preventing creation of other
> handles to xeniface.
> > This includes the xenagent service (if started after a XenControl user), so
> no power control, etc.
> >
> > Should it be possible for more than one program to use XenControl at the
> same time?
> 
> Yes, it should. Are you sure about the above? I do have multiple
> processes using xencontrol interface at the same time and they work just
> fine.
> 

I think you do need to explicitly set sharing. I don't know why you can open 
multiple handles successfully; you really should not be able to.

  Paul

> --
> Best Regards,
> Marek Marczykowski-Górecki
> Invisible Things Lab
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel