[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Fri, Jul 13, 2018 at 09:32:59AM +0000, Paul Durrant wrote:
> > -----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.

Ok, I'll adjust that. Is there anything else to fix, or should I submit
v2 right now?

-- 
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?

Attachment: signature.asc
Description: PGP signature

_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.