[Libguestfs] Patchable build problems on OS X 10.10

Richard W.M. Jones rjones at redhat.com
Fri Feb 6 10:03:37 UTC 2015


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.

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

> @@ -334,7 +334,14 @@ do_fuser (const char *mountpoint)
>    }
>  
>    if (pid == 0) {               /* Child - run /sbin/fuser. */
> -    execlp ("/sbin/fuser", "fuser", "-v", "-m", mountpoint, NULL);
> +    const char *cmd_prefix = "/bin/ps -p \"$(fuser -c ";
> +    const char *cmd_suffix = " 2>/dev/null)\" -o user,pid,comm 2>/dev/null";
> +    char *cmd = malloc (strlen(cmd_prefix) + strlen(mountpoint) + strlen(cmd_suffix) + 1);
> +    if (cmd) {
> +      sprintf (cmd, "%s%s%s", cmd_prefix, mountpoint, cmd_suffix);
> +      execlp ("/bin/sh", "sh", "-c", cmd, NULL);
> +      free (cmd);
> +    }
>      _exit (EXIT_FAILURE);
>    }
>  
> diff --git gnulib/lib/Makefile.in gnulib/lib/Makefile.in
> index 426004d..6d1db79 100644
> --- gnulib/lib/Makefile.in
> +++ gnulib/lib/Makefile.in
> @@ -265,7 +265,7 @@ am_libgnu_la_OBJECTS = accept4.lo allocator.lo areadlink.lo \
>  	fd-hook.lo filenamecat-lgpl.lo filevercmp.lo full-read.lo \
>  	full-write.lo gettime.lo hash.lo hash-pjw.lo human.lo \
>  	i-ring.lo localcharset.lo glthread/lock.lo malloca.lo \
> -	openat-die.lo openat-safer.lo pipe2.lo quotearg.lo \
> +	open_memstream.lo openat-die.lo openat-safer.lo pipe2.lo quotearg.lo \
>  	read-file.lo safe-read.lo safe-write.lo save-cwd.lo sockets.lo \
>  	stat-time.lo strnlen1.lo sys_socket.lo tempname.lo \
>  	glthread/threadlib.lo timespec.lo unistd.lo dup-safer.lo \

Changes in the gnulib/ directory come from gnulib, so
need to be sent/suggested to them:

https://www.gnu.org/software/gnulib/

[...]
> diff --git ruby/Makefile.am ruby/Makefile.am
> index f605188..d28d77b 100644
> --- ruby/Makefile.am
> +++ ruby/Makefile.am
> @@ -21,6 +21,8 @@ generator_built = \
>  	ext/guestfs/_guestfs.c \
>  	bindtests.rb
>  
> +DLEXT := $(shell $(RUBY) -rrbconfig -e "puts RbConfig::CONFIG['DLEXT']")
> +
>  EXTRA_DIST = \
>  	$(generator_built) \
>  	Rakefile.in \
> @@ -38,7 +40,7 @@ CLEANFILES = \
>  	ext/guestfs/*~ \
>  	ext/guestfs/extconf.h \
>  	ext/guestfs/_guestfs.o \
> -	ext/guestfs/_guestfs.so \
> +	ext/guestfs/_guestfs.$(DLEXT) \
>  	ext/guestfs/mkmf.log \
>  	ext/guestfs/Makefile
>  
> @@ -59,7 +61,7 @@ install:
>  	$(MKDIR_P) $(DESTDIR)$(RUBY_LIBDIR)
>  	$(MKDIR_P) $(DESTDIR)$(RUBY_ARCHDIR)
>  	$(INSTALL) -p -m 0644 $(srcdir)/lib/guestfs.rb $(DESTDIR)$(RUBY_LIBDIR)
> -	$(INSTALL) -p -m 0755 ext/guestfs/_guestfs.so $(DESTDIR)$(RUBY_ARCHDIR)
> +	$(INSTALL) -p -m 0755 ext/guestfs/_guestfs.$(DLEXT) $(DESTDIR)$(RUBY_ARCHDIR)
>  
>  TESTS = run-bindtests run-ruby-tests
>  
>
> diff --git ruby/Makefile.in ruby/Makefile.in
> index 8718724..1693abb 100644
> --- ruby/Makefile.in
> +++ ruby/Makefile.in
> @@ -1428,6 +1428,7 @@ generator_built = \
>  	ext/guestfs/_guestfs.c \
>  	bindtests.rb
>  
> +DLEXT := $(shell $(RUBY) -rrbconfig -e "puts RbConfig::CONFIG['DLEXT']")
>  EXTRA_DIST = \
>  	$(generator_built) \
>  	Rakefile.in \
> @@ -1445,7 +1446,7 @@ CLEANFILES = \
>  	ext/guestfs/*~ \
>  	ext/guestfs/extconf.h \
>  	ext/guestfs/_guestfs.o \
> -	ext/guestfs/_guestfs.so \
> +	ext/guestfs/_guestfs.$(DLEXT) \
>  	ext/guestfs/mkmf.log \
>  	ext/guestfs/Makefile
>  
> @@ -1788,7 +1789,7 @@ $(top_builddir)/generator/generator:
>  @HAVE_RUBY_TRUE@	$(MKDIR_P) $(DESTDIR)$(RUBY_LIBDIR)
>  @HAVE_RUBY_TRUE@	$(MKDIR_P) $(DESTDIR)$(RUBY_ARCHDIR)
>  @HAVE_RUBY_TRUE@	$(INSTALL) -p -m 0644 $(srcdir)/lib/guestfs.rb $(DESTDIR)$(RUBY_LIBDIR)
> - at HAVE_RUBY_TRUE@	$(INSTALL) -p -m 0755 ext/guestfs/_guestfs.so $(DESTDIR)$(RUBY_ARCHDIR)
> + at HAVE_RUBY_TRUE@	$(INSTALL) -p -m 0755 ext/guestfs/_guestfs.$(DLEXT) $(DESTDIR)$(RUBY_ARCHDIR)
>  
>  # Tell versions [3.59,3.63) of GNU make to not export all variables.
>  # Otherwise a system limit (for SysV at least) may be exceeded.
> diff --git ruby/Rakefile.in ruby/Rakefile.in
> index 94a25fd..1dfc600 100644
> --- ruby/Rakefile.in
> +++ ruby/Rakefile.in
> @@ -40,9 +40,11 @@ end
>  PKG_NAME='@PACKAGE_NAME@'
>  PKG_VERSION='@PACKAGE_VERSION@'
>  
> +DLEXT=RbConfig::CONFIG['DLEXT']
> +
>  EXT_CONF='@abs_builddir@/ext/guestfs/extconf.rb'
>  MAKEFILE='@builddir@/ext/guestfs/Makefile'
> -GUESTFS_MODULE='@builddir@/ext/guestfs/_guestfs.so'
> +GUESTFS_MODULE="@builddir@/ext/guestfs/_guestfs.#{DLEXT}"
>  GUESTFS_SRC='@srcdir@/ext/guestfs/_guestfs.c'
>  
>  CLEAN.include [ "@builddir@/ext/**/*.o", GUESTFS_MODULE,

The DLEXT changes in the ruby directory all look good to me.  I have
pushed these upstream:

https://github.com/libguestfs/libguestfs/commit/eaae0b614c59f799885ee940117c4fb507a1f2d3

> diff --git src/guestfs-internal-frontend.h src/guestfs-internal-frontend.h
> index ba3ddde..f9079c5 100644
> --- src/guestfs-internal-frontend.h
> +++ src/guestfs-internal-frontend.h
> @@ -167,7 +167,7 @@ extern GUESTFS_DLL_PUBLIC int guestfs___add_libvirt_dom (guestfs_h *g, virDomain
>  #if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME == 1
>  #  define program_name program_invocation_short_name
>  #else
> -#  define program_name "libguestfs"
> +#  define program_name getprogname()
>  #endif

Linux doesn't have getprogname.  One way around this is to add
getprogname to the list of functions in configure.ac AC_CHECK_FUNCS.
That will cause a macro to be defined which you can use like this:

  #if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME == 1
  #  define program_name program_invocation_short_name
  #elif HAVE_GETPROGNAME
  #  define program_name getprogname()
  #else
  #  define program_name "libguestfs"
  #endif

>  /* Close all file descriptors matching the condition. */
> 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

I pushed this hunk upstream:

https://github.com/libguestfs/libguestfs/commit/7ddf6bcbfdc66855b594afaaacdc4998177f2286

> diff --git src/proto.c src/proto.c
> index 8001c8c..53d1d6b 100644
> --- src/proto.c
> +++ src/proto.c
> @@ -252,7 +252,7 @@ guestfs___send (guestfs_h *g, int proc_nr,
>     * have no parameters.
>     */
>    if (xdrp) {
> -    if (!(*xdrp) (&xdr, args)) {
> +    if (!(*xdrp) (&xdr, args, 0)) {
>        error (g, _("dispatch failed to marshal args"));
>        return -1;
>      }
> @@ -681,7 +681,7 @@ guestfs___recv (guestfs_h *g, const char *fn,
>        return -1;
>      }
>    } else {
> -    if (xdrp && ret && !xdrp (&xdr, ret)) {
> +    if (xdrp && ret && !xdrp (&xdr, ret, 0)) {
>        error (g, "%s: failed to parse reply", fn);
>        xdr_destroy (&xdr);
>        return -1;

This would break glibc's implementation of XDR.  Not quite
sure how to solve this except with #if defined ... #endif.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW




More information about the Libguestfs mailing list