[Libguestfs] [nbdkit] Windows errno handling

[Richard W.M. Jones]

On Mon, Aug 17, 2020 at 08:07:12PM +0100, Daniel P. Berrangé wrote:
> On Mon, Aug 17, 2020 at 01:56:10PM -0500, Eric Blake wrote:
> > On 8/17/20 1:46 PM, Daniel P. Berrangé wrote:
> > > On Mon, Aug 17, 2020 at 07:36:00PM +0100, Richard W.M. Jones wrote:
> > > > The Windows port of nbdkit
> > > > (https://github.com/rwmjones/nbdkit/tree/2020-windows-mingw) now works
> > > > to some extent.  However errno handling doesn't work.  The way that
> > > > Winsock handles errors is incompatible with the way we expect to work
> > > > errno in several ways.  The long story is here:
> > > > 
> > > > https://docs.microsoft.com/en-us/windows/win32/winsock/error-codes-errno-h-errno-and-wsagetlasterror-2
> > > > https://docs.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2
> > > > 
> > 
> > > > 
> > > > This is very invasive for existing code.  There are ~60 places in the
> > > > existing code which seem to assign to errno, but some of these either
> > > > set errno = 0 or are preserving errno (item (3) above), and so we'd
> > > > probably want to handle those a bit differently.
> > > > 
> > > > Printing of Winsock codes through perror() or %m actually works
> > 
> > That _is_ a surprise.  But if it makes like easier, we'll take it.
> > 
> > > > (surprisingly).  However it does _not_ seem to work if we try to
> > > > translate the codes to errno E* values.  I need to look at exactly
> > > > what's going on here.
> > > > 
> > > > Number (5) is actually fairly easy to deal with because there's only
> > > > one place where we handle the errno returned by plugins
> > > > (server/plugins.c:get_error).  I think we'd probably want
> > > > errno_is_preserved to mean "WSAGetLastError" or "GetLastError"
> > > > contains something of interest.
> > > > 
> > > > Thoughts?
> > > > 
> > > > Also I really need to look at how some other portable libraries like
> > > > curl and gnutls are handling this.  Maybe they've already come up with
> > > > something.
> > > 
> > > Take a look at what libvirt has done. We follow a simplified version
> > > of what GNULIB does, by defining custom wrapper functions around
> > > all winsock APIs we need that set errno, and then use a macro to
> > > transparently replace calls to use our wrappers:
> > > 
> > > https://gitlab.com/libvirt/libvirt/-/blob/master/src/util/virsocket.h
> > > https://gitlab.com/libvirt/libvirt/-/blob/master/src/util/virsocket.c
> > > 
> > > You should be able to just lift those two files straight into your
> > > git repo as they have no deps on other libvirt infra.
> > 
> > Except that libvirt is GPLv2+, and nbdkit is not, so we'd need to relicense
> > (if all those lines are due to you, we may be safe; but if they derive too
> > much from gnulib, we're stuck).
> 
> I wrote it all from scratch, but I was referencing the gnulib code,
> so can't claim its completely clean room, and the set_errno method
> was directly lifted from gnulib code.
> 
> If nothing else you could do a similar kind of approach though. I
> felt it was preferrable to wrap the socket APIs in one place, instead
> of adding extra magic in every caller, so the cruft is isolated.

Thanks - I'll study the code to see if we can reimplement
the technique anyhow.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/