On Mon, Feb 09, 2015 at 10:56:54AM +0100, Pino Toscano wrote:
On Friday 06 February 2015 10:03:37 Richard W.M. Jones wrote:
> On Thu, Feb 05, 2015 at 10:53:06PM +0000, Margaret Lewicka wrote:
> > +/* Fixes for Mac OS X */
> > +#if defined __APPLE__ && defined __MACH__
> > +#include <sys/un.h>
> > +#endif
> > +#ifndef SOCK_CLOEXEC
> > +# define SOCK_CLOEXEC O_CLOEXEC
> > +#endif
> > +#ifndef SOCK_NONBLOCK
> > +# define SOCK_NONBLOCK O_NONBLOCK
> > +#endif
> > +/* End of fixes for Mac OS X */
> > +
> > /* Check minimum required version of libvirt. The libvirt backend
> > * is new and not the default, so we can get away with forcing
> > * people who want to try it to have a reasonably new version of
This IMHO is clearly wrong: the O_* constants are for open() & friends,
not for socket & socket4.
I checked this out before committing it, and I accepted it because on
Linux/glibc, SOCK_CLOEXEC == O_CLOEXEC and SOCK_NONBLOCK == O_NONBLOCK
(see the definitions in bits/socket.h and bits/socket_type.h on a
Linux system).
Of course this is not a law of nature and there could be systems where
this is not true.
The macros as defined only affect systems that don't define SOCK_* at
all.
Theoretically, we could switch the socket() usages in
launch-libvirt.c
to socket4(), which can be replaced by gnulib if missing (we already
use the "accept4" gnulib module). On the other hand, it seems that
such gnulib emulation does not provide SOCK_NONBLOCK, so either
a) fix that in gnulib
b) use the "nonblocking" gnulib module, using set_nonblocking_flag()
instead of SOCK_NONBLOCK
(b) would not be atomic. I'm not certain if there's a case where that
matters for O_NONBLOCK. (Of course it definitely matters for
O_CLOEXEC and we've have bugs caused by that in the past)
Also, sys/un.h is POSIX [1], so it can be included unconditionally.
Agreed.
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/