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

Re: [libvirt] [PATCH] fix WIN64 compatibility bug in external EventImpl API



On Wed, Nov 19, 2008 at 03:11:35PM -0500, David Lively wrote:
> On Wed, 2008-11-19 at 18:33 +0000, Daniel P. Berrange wrote:
> > On Wed, Nov 19, 2008 at 01:28:23PM -0500, David Lively wrote:
> > > On Wed, 2008-11-19 at 16:49 +0000, Daniel P. Berrange wrote:
> > > > On Wed, Nov 19, 2008 at 11:41:43AM -0500, David Lively wrote:
> > > > > While starting to think about Windows compability, I realized the newly
> > > > > exposed API for registering an external EventImpl is not adequate.
> > > > > Currently it's assuming 32-bit unix fds.  But Windows uses a pointer
> > > > > (HANDLE) here.  So we need to generalize this interface so it can be
> > > > > implemented for 64-bit Windows.  The attached patch does this.  (I'm
> > > > > sure it conflicts with work Dan B is doing, so I'm hoping he'll just
> > > > > incorporate this into his changes.)
> > > > 
> > > > I'm not sure whether this is actually required. We're using gnulib for
> > > > socket stuff, and that wraps the Winsock socket() call so that it returns
> > > > a real file descriptor rather than a socket handle. It does this calling
> > > > _open_osfhandle which appears to be declared to accept a 'long' and return
> > > > an 'int' - at least in MinGW headers.
> > > 
> > > That means that the Windows application using libvirt must use gnulib as
> > > well.  If the Windows version of libvirt actually exports the gnulib
> > > bindings and headers, then I guess that's not a problem.  But does it
> > > export gnulib?
> > 
> > No, the gnulib stuff is internal only - we don't force any apps to also
> > use gnulib.
> > 
> > It does however mean we should document that the 'fd' arg of the
> > the  virEventAddHandle callback is an file handle, and not a socket
> > handle on Win32, so apps are clear on what to expect.
> 
> Does Windows support integer file handles?  Or are they a winsock
> concept - in which case we're assuming the app uses winsock, right?

It is a complicated situation :-)

The standard Winsock API equivalent to socket() is called
WSASocket() and returns a datatype SOCKET, which I believe is
just an alias for HANDLE.

GnuLib's goal is to fix non-POSIX-ness, so it provides a impl
called socket() which returns an 'int' as per POSIX. Internally
this is implemented by calling WSASocket(), and then using the
_open_osfhandle() method to convert the SOCKET into a regular
'int' file handle which is compatible with Windows' UNIX API
layer. eg close, ioctl, etc

So, yes, you are correct, that regular Winsock is not compatible with
int file handles, but by using GNUlib and the compat layer, we do
re-gain proper compatability.

The compatability works both ways - if an application using libvirt
really does want the original SOCKET handle again, it can convert
the file handle back.

So _open_osfhandle() and _get_osfhandle() are idempotent

MicroSoft have some docs on this here

  http://msdn.microsoft.com/en-us/library/bdts1c9x(VS.71).aspx
  http://msdn.microsoft.com/en-us/library/ks2530z6.aspx

As an example, we use GNULIB in GTK-VNC too, and need to integrate
with GLib event loop. GLib on Win32 is written to assume use of
traditional SOCKET handles, so when we register the GTK-VNC socket 
with GLib, we use _get_osfhandle to convert the file descriptor back
into a SOCKET handle.

NB, the OSF handles are avialable in Win 98, Win Me, Win NT, Win 2000,
Win XP

We don't care about Win 95 so Win CE is the only currently existing
Windows platform we'd be unable to support in this way. I'm personally
not worried by this restriction...

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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