[Libguestfs] Patchable build problems on OS X 10.10

[Pino Toscano]

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:
> > Hello,
> > 
> > I'm attempting to create a Homebrew formula to get libguestfs to
> > compile on Mac OS X. I've managed to achieve success with several
> > monkey patches, but since Homebrew's policy is to contact maintainers
> > about proper fixes in upstream, I would like to ask if there are any
> > plans to fix these issues. I'm afraid I don't know C well enough to
> > propose decent solutions myself.
> 
> Thanks for looking at this.  I am interested in getting libguestfs to
> work on Mac OS X again.

Thanks for your patches! In the past months I've done a number of
commits to help making libguestfs (without guestfsd & appliance) build
and work on FreeBSD.

> I have included the patch below for review.  As a general comment,
> it's easier for us if the patch is split into separate reviewable
> commits, and then sent to the mailing list using 'git send-email'.
> 
> > diff --git fuse/guestunmount.c fuse/guestunmount.c
> > index c36c336..1ab7ff5 100644
> > --- fuse/guestunmount.c
> > +++ fuse/guestunmount.c
> > @@ -257,7 +257,7 @@ do_fusermount (const char *mountpoint, char **error_rtn)
> >      /* We have to parse error messages from fusermount, so ... */
> >      setenv ("LC_ALL", "C", 1);
> >  
> > -    execlp ("fusermount", "fusermount", "-u", mountpoint, NULL);
> > +    execlp ("/sbin/umount", "umount", mountpoint, NULL);
> 
> fusermount is needed on Linux, so this is an example of a patch
> which could be written instead as:
> 
>   #if !(defined __APPLE__ && defined __MACH__)
>       execlp ("fusermount", "fusermount", "-u", mountpoint, NULL);
>   #else
>       execlp ("/sbin/umount", "umount", mountpoint, NULL);
>   #endif

Or rather use fusermount only on Linux and umount (with no path)
elsewhere.

> > diff --git src/launch-libvirt.c src/launch-libvirt.c
> > index aaa8501..c0adc80 100644
> > --- src/launch-libvirt.c
> > +++ src/launch-libvirt.c
> > @@ -57,6 +57,18 @@
> >  #include "guestfs-internal-actions.h"
> >  #include "guestfs_protocol.h"
> >  
> > +/* 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.
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

Also, sys/un.h is POSIX [1], so it can be included unconditionally.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_un.h.html

Thanks,
-- 
Pino Toscano