[libvirt] [PATCHv2 0/7] resend: various cleanups
I've been sitting on several unreviewed cleanups. Some of them needed tweaks given recent changes in the tree, so I've rebased them and am posting them in one group to make review easier. Eric Blake (7): build: allow autoconf 2.59 again build: allow older libselinux again build: improve testsuite results with older automake tests: avoid data race tests: avoid spurious failure of nodeinfotest qemu: use virAsprintf instead of PATH_MAX build: find xdr headers on cygwin .gnulib | 2 +- configure.ac | 37 +++++++++++++++++++++++--- src/Makefile.am | 2 +- src/nodeinfo.c | 11 ++++++-- src/qemu/qemu_driver.c | 53 ++++++++++++++++++++------------------- src/security/security_selinux.c | 29 +++++++++++++++++---- tests/Makefile.am | 8 +++--- tests/commandhelper.c | 2 +- tests/nodeinfotest.c | 5 ++- 9 files changed, 100 insertions(+), 49 deletions(-) -- 1.7.3.3
Autoconf 2.59 doesn't define ${localedir}, so libvirt was failing to compile due to a missing LOCALEDIR until today's configmake fix. * .gnulib: Update to latest for configmake fix. * configure.ac (libpcap): Avoid AS_CASE. --- .gnulib | 2 +- configure.ac | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.gnulib b/.gnulib index 980f9d2..3e464b4 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 980f9d2ceb43f9d86ea57db0367e569267c8571b +Subproject commit 3e464b4c6a6db9ca2e186b47fe2b72780bdd60e7 diff --git a/configure.ac b/configure.ac index 558bb77..c44d024 100644 --- a/configure.ac +++ b/configure.ac @@ -1211,11 +1211,11 @@ LIBPCAP_FOUND="no" AC_ARG_WITH([libpcap], AC_HELP_STRING([--with-libpcap=@<:@PFX@:>@], [libpcap location])) if test "$with_qemu" = "yes"; then - AS_CASE(["x$with_libpcap"], - [xno], [LIBPCAP_CONFIG=""], - [x|xyes], [LIBPCAP_CONFIG="pcap-config"], - [LIBPCAP_CONFIG="$with_libpcap/bin/pcap-config"] - ) + case $with_libpcap in + no) LIBPCAP_CONFIG= ;; + ''|yes) LIBPCAP_CONFIG="pcap-config" ;; + *) LIBPCAP_CONFIG="$with_libpcap/bin/pcap-config" ;; + esac AS_IF([test "x$LIBPCAP_CONFIG" != "x"], [ AC_MSG_CHECKING(libpcap $LIBPCAP_CONFIG >= $LIBPCAP_REQUIRED ) if ! $LIBPCAP_CONFIG --libs > /dev/null 2>&1 ; then -- 1.7.3.3
2010/12/18 Eric Blake <eblake@redhat.com>:
Autoconf 2.59 doesn't define ${localedir}, so libvirt was failing to compile due to a missing LOCALEDIR until today's configmake fix.
* .gnulib: Update to latest for configmake fix. * configure.ac (libpcap): Avoid AS_CASE. --- .gnulib | 2 +- configure.ac | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/.gnulib b/.gnulib index 980f9d2..3e464b4 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 980f9d2ceb43f9d86ea57db0367e569267c8571b +Subproject commit 3e464b4c6a6db9ca2e186b47fe2b72780bdd60e7 diff --git a/configure.ac b/configure.ac index 558bb77..c44d024 100644 --- a/configure.ac +++ b/configure.ac @@ -1211,11 +1211,11 @@ LIBPCAP_FOUND="no"
AC_ARG_WITH([libpcap], AC_HELP_STRING([--with-libpcap=@<:@PFX@:>@], [libpcap location])) if test "$with_qemu" = "yes"; then - AS_CASE(["x$with_libpcap"], - [xno], [LIBPCAP_CONFIG=""], - [x|xyes], [LIBPCAP_CONFIG="pcap-config"], - [LIBPCAP_CONFIG="$with_libpcap/bin/pcap-config"] - ) + case $with_libpcap in + no) LIBPCAP_CONFIG= ;; + ''|yes) LIBPCAP_CONFIG="pcap-config" ;; + *) LIBPCAP_CONFIG="$with_libpcap/bin/pcap-config" ;; + esac AS_IF([test "x$LIBPCAP_CONFIG" != "x"], [ AC_MSG_CHECKING(libpcap $LIBPCAP_CONFIG >= $LIBPCAP_REQUIRED ) if ! $LIBPCAP_CONFIG --libs > /dev/null 2>&1 ; then
ACK. Matthias
* configure.ac (with_selinux): Check for <selinux/label.h>. * src/security/security_selinux.c (getContext): New function. (SELinuxRestoreSecurityFileLabel): Use it to restore compilation when using older libselinux. --- configure.ac | 3 +++ src/security/security_selinux.c | 29 +++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index c44d024..4df915a 100644 --- a/configure.ac +++ b/configure.ac @@ -1023,6 +1023,9 @@ fi if test "$with_selinux" = "yes"; then SELINUX_LIBS="-lselinux" AC_DEFINE_UNQUOTED([HAVE_SELINUX], 1, [whether basic SELinux functionality is available]) + dnl We prefer to use <selinux/label.h> and selabel_open, but can fall + dnl back to matchpathcon for the sake of RHEL 5's version of libselinux. + AC_CHECK_HEADERS([selinux/label.h]) fi AM_CONDITIONAL([HAVE_SELINUX], [test "$with_selinux" != "no"]) AC_SUBST([SELINUX_CFLAGS]) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 37539c2..49efa75 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -14,11 +14,13 @@ */ #include <config.h> #include <selinux/selinux.h> -#include <selinux/label.h> #include <selinux/context.h> #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> +#if HAVE_SELINUX_LABEL_H +# include <selinux/label.h> +#endif #include "security_driver.h" #include "security_selinux.h" @@ -355,6 +357,25 @@ SELinuxSetFilecon(const char *path, char *tcon) return 0; } +/* Set fcon to the appropriate label for path and mode, or return -1. */ +static int +getContext(const char *newpath, mode_t mode, security_context_t *fcon) +{ +#if HAVE_SELINUX_LABEL_H + struct selabel_handle *handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); + int ret; + + if (handle == NULL) + return -1; + + ret = selabel_lookup(handle, fcon, newpath, mode); + selabel_close(handle); + return ret; +#else + return matchpathcon(newpath, mode, fcon); +#endif +} + /* This method shouldn't raise errors, since they'll overwrite * errors that the caller(s) are already dealing with */ @@ -363,7 +384,6 @@ SELinuxRestoreSecurityFileLabel(const char *path) { struct stat buf; security_context_t fcon = NULL; - struct selabel_handle *handle = NULL; int rc = -1; char *newpath = NULL; char ebuf[1024]; @@ -382,16 +402,13 @@ SELinuxRestoreSecurityFileLabel(const char *path) goto err; } - if ((handle = selabel_open(SELABEL_CTX_FILE, NULL, 0)) == NULL || - selabel_lookup(handle, &fcon, newpath, buf.st_mode) < 0) { + if (getContext(newpath, buf.st_mode, &fcon) < 0) { VIR_WARN("cannot lookup default selinux label for %s", newpath); } else { rc = SELinuxSetFilecon(newpath, fcon); } err: - if (handle) - selabel_close(handle); freecon(fcon); VIR_FREE(newpath); return rc; -- 1.7.3.3
2010/12/18 Eric Blake <eblake@redhat.com>:
* configure.ac (with_selinux): Check for <selinux/label.h>. * src/security/security_selinux.c (getContext): New function. (SELinuxRestoreSecurityFileLabel): Use it to restore compilation when using older libselinux. --- configure.ac | 3 +++ src/security/security_selinux.c | 29 +++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-)
ACK. Matthias
* tests/Makefile.am (TESTS_ENVIRONMENT, commandtest_CFLAGS) (commandhelper_CFLAGS): Avoid $(builddir) and $(abs_builddir) in automake 1.9.6; fixes spurious failures of commandtest. --- tests/Makefile.am | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index de2ff91..69731d3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -209,12 +209,12 @@ TESTS += cputest path_add = $$abs_top_builddir/src$(PATH_SEPARATOR)$$abs_top_builddir/daemon$(PATH_SEPARATOR)$$abs_top_builddir/tools # NB, automake < 1.10 does not provide the real -# abs_top_{src/build}dir variables, so don't rely +# abs_top_{src/build}dir or builddir variables, so don't rely # on them here. Fake them with 'pwd' TESTS_ENVIRONMENT = \ abs_top_builddir=`cd '$(top_builddir)'; pwd` \ abs_top_srcdir=`cd '$(top_srcdir)'; pwd` \ - abs_builddir=`cd '$(builddir)'; pwd` \ + abs_builddir=`pwd` \ abs_srcdir=`cd '$(srcdir)'; pwd` \ CONFIG_HEADER="`cd '$(top_builddir)'; pwd`/config.h" \ PATH="$(path_add)$(PATH_SEPARATOR)$$PATH" \ @@ -356,12 +356,12 @@ nodeinfotest_LDADD = $(LDADDS) commandtest_SOURCES = \ commandtest.c testutils.h testutils.c -commandtest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" +commandtest_CFLAGS = -Dabs_builddir="\"`pwd`\"" commandtest_LDADD = $(LDADDS) commandhelper_SOURCES = \ commandhelper.c -commandhelper_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" +commandhelper_CFLAGS = -Dabs_builddir="\"`pwd`\"" commandhelper_LDADD = $(LDADDS) if WITH_SECDRIVER_SELINUX -- 1.7.3.3
2010/12/18 Eric Blake <eblake@redhat.com>:
* tests/Makefile.am (TESTS_ENVIRONMENT, commandtest_CFLAGS) (commandhelper_CFLAGS): Avoid $(builddir) and $(abs_builddir) in automake 1.9.6; fixes spurious failures of commandtest. --- tests/Makefile.am | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
ACK. Matthias
I got some spurious failures when commandhelper won the race and ran to the point of parent detection prior to the intermediate daemonizing process getting a chance to exit. This fixes it. * tests/commandhelper.c (main): Checking for re-parenting to init(1) is racy; instead check that we belong to a new session. --- tests/commandhelper.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 5b2f301..f400e8d 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -95,7 +95,7 @@ int main(int argc, char **argv) { fprintf(log, "FD:%d\n", i); } - fprintf(log, "DAEMON:%s\n", getppid() == 1 ? "yes" : "no"); + fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no"); char cwd[1024]; if (!getcwd(cwd, sizeof(cwd))) return EXIT_FAILURE; -- 1.7.3.3
2010/12/18 Eric Blake <eblake@redhat.com>:
I got some spurious failures when commandhelper won the race and ran to the point of parent detection prior to the intermediate daemonizing process getting a chance to exit. This fixes it.
* tests/commandhelper.c (main): Checking for re-parenting to init(1) is racy; instead check that we belong to a new session. --- tests/commandhelper.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 5b2f301..f400e8d 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -95,7 +95,7 @@ int main(int argc, char **argv) { fprintf(log, "FD:%d\n", i); }
- fprintf(log, "DAEMON:%s\n", getppid() == 1 ? "yes" : "no"); + fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no"); char cwd[1024]; if (!getcwd(cwd, sizeof(cwd))) return EXIT_FAILURE; --
ACK. Matthias
When running 'make check' under a multi-cpu Dom0 xen machine, nodeinfotest had a spurious failure it was reading from /sys/devices/system/cpu, but xen has no notion of topology. The test was intended to be isolated from reading any real system files; the regression was introduced in Mar 2010 with commit aa2f6f96dd. Fix things by allowing an early exit for the testsuite. * src/nodeinfo.c (linuxNodeInfoCPUPopulate): Add parameter. (nodeGetInfo): Adjust caller. * tests/nodeinfotest.c (linuxTestCompareFiles): Likewise. --- src/nodeinfo.c | 11 ++++++++--- tests/nodeinfotest.c | 5 +++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index acd3188..22d53e5 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -61,7 +61,8 @@ /* NB, this is not static as we need to call it from the testsuite */ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, - virNodeInfoPtr nodeinfo); + virNodeInfoPtr nodeinfo, + bool need_hyperthreads); /* Return the positive decimal contents of the given * CPU_SYS_PATH/cpu%u/FILE, or -1 on error. If MISSING_OK and the @@ -167,7 +168,8 @@ static int parse_socket(unsigned int cpu) } int linuxNodeInfoCPUPopulate(FILE *cpuinfo, - virNodeInfoPtr nodeinfo) + virNodeInfoPtr nodeinfo, + bool need_hyperthreads) { char line[1024]; DIR *cpudir = NULL; @@ -244,6 +246,9 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, return -1; } + if (!need_hyperthreads) + return 0; + /* OK, we've parsed what we can out of /proc/cpuinfo. Get the socket * and thread information from /sys */ @@ -338,7 +343,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { _("cannot open %s"), CPUINFO_PATH); return -1; } - ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo); + ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo, true); VIR_FORCE_FCLOSE(cpuinfo); if (ret < 0) return -1; diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c index f56247b..c690403 100644 --- a/tests/nodeinfotest.c +++ b/tests/nodeinfotest.c @@ -26,7 +26,8 @@ static char *abs_srcdir; # define MAX_FILE 4096 -extern int linuxNodeInfoCPUPopulate(FILE *cpuinfo, virNodeInfoPtr nodeinfo); +extern int linuxNodeInfoCPUPopulate(FILE *cpuinfo, virNodeInfoPtr nodeinfo, + bool need_hyperthreads); static int linuxTestCompareFiles(const char *cpuinfofile, const char *outputfile) { char actualData[MAX_FILE]; @@ -43,7 +44,7 @@ static int linuxTestCompareFiles(const char *cpuinfofile, const char *outputfile return -1; memset(&nodeinfo, 0, sizeof(nodeinfo)); - if (linuxNodeInfoCPUPopulate(cpuinfo, &nodeinfo) < 0) { + if (linuxNodeInfoCPUPopulate(cpuinfo, &nodeinfo, false) < 0) { if (virTestGetDebug()) { virErrorPtr error = virSaveLastError(); if (error && error->code != VIR_ERR_OK) -- 1.7.3.3
2010/12/18 Eric Blake <eblake@redhat.com>:
When running 'make check' under a multi-cpu Dom0 xen machine, nodeinfotest had a spurious failure it was reading from /sys/devices/system/cpu, but xen has no notion of topology. The test was intended to be isolated from reading any real system files; the regression was introduced in Mar 2010 with commit aa2f6f96dd.
Fix things by allowing an early exit for the testsuite.
* src/nodeinfo.c (linuxNodeInfoCPUPopulate): Add parameter. (nodeGetInfo): Adjust caller. * tests/nodeinfotest.c (linuxTestCompareFiles): Likewise. --- src/nodeinfo.c | 11 ++++++++--- tests/nodeinfotest.c | 5 +++-- 2 files changed, 11 insertions(+), 5 deletions(-)
ACK. Matthias
* src/qemu/qemu_driver.c (qemudLogFD, qemudLogReadFD) (qemudStartup, qemudGetProcessInfo): Use heap instead of stack. (qemudDomainDetachPciDiskDevice) (qemudDomainDetachSCSIDiskDevice): Minor optimization. --- src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++----------------------- 1 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0fef042..1a26a50 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -178,13 +178,11 @@ static int doStopCPUs(struct qemud_driver *driver, virDomainObjPtr vm) static int qemudLogFD(struct qemud_driver *driver, const char* name, bool append) { - char logfile[PATH_MAX]; + char *logfile; mode_t logmode; - int ret, fd = -1; + int fd = -1; - if ((ret = snprintf(logfile, sizeof(logfile), "%s/%s.log", - driver->logDir, name)) - < 0 || ret >= sizeof(logfile)) { + if (virAsprintf(&logfile, "%s/%s.log", driver->logDir, name) < 0) { virReportOOMError(); return -1; } @@ -200,8 +198,10 @@ qemudLogFD(struct qemud_driver *driver, const char* name, bool append) virReportSystemError(errno, _("failed to create logfile %s"), logfile); + VIR_FREE(logfile); return -1; } + VIR_FREE(logfile); if (virSetCloseExec(fd) < 0) { virReportSystemError(errno, "%s", _("Unable to set VM logfile close-on-exec flag")); @@ -215,29 +215,29 @@ qemudLogFD(struct qemud_driver *driver, const char* name, bool append) static int qemudLogReadFD(const char* logDir, const char* name, off_t pos) { - char logfile[PATH_MAX]; + char *logfile; mode_t logmode = O_RDONLY; - int ret, fd = -1; + int fd = -1; - if ((ret = snprintf(logfile, sizeof(logfile), "%s/%s.log", logDir, name)) - < 0 || ret >= sizeof(logfile)) { + if (virAsprintf(&logfile, "%s/%s.log", logDir, name) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("failed to build logfile name %s/%s.log"), logDir, name); return -1; } - if ((fd = open(logfile, logmode)) < 0) { virReportSystemError(errno, _("failed to create logfile %s"), logfile); + VIR_FREE(logfile); return -1; } if (virSetCloseExec(fd) < 0) { virReportSystemError(errno, "%s", _("Unable to set VM logfile close-on-exec flag")); VIR_FORCE_CLOSE(fd); + VIR_FREE(logfile); return -1; } if (pos < 0 || lseek(fd, pos, SEEK_SET) < 0) { @@ -246,6 +246,7 @@ qemudLogReadFD(const char* logDir, const char* name, off_t pos) (long long) pos, logfile); VIR_FORCE_CLOSE(fd); } + VIR_FREE(logfile); return fd; } @@ -1178,7 +1179,7 @@ cleanup: static int qemudStartup(int privileged) { char *base = NULL; - char driverConf[PATH_MAX]; + char *driverConf = NULL; int rc; virConnectPtr conn = NULL; @@ -1318,14 +1319,9 @@ qemudStartup(int privileged) { /* Configuration paths are either ~/.libvirt/qemu/... (session) or * /etc/libvirt/qemu/... (system). */ - if (snprintf (driverConf, sizeof(driverConf), "%s/qemu.conf", base) == -1) - goto out_of_memory; - driverConf[sizeof(driverConf)-1] = '\0'; - - if (virAsprintf(&qemu_driver->configDir, "%s/qemu", base) == -1) - goto out_of_memory; - - if (virAsprintf(&qemu_driver->autostartDir, "%s/qemu/autostart", base) == -1) + if (virAsprintf(&driverConf, "%s/qemu.conf", base) < 0 || + virAsprintf(&qemu_driver->configDir, "%s/qemu", base) < 0 || + virAsprintf(&qemu_driver->autostartDir, "%s/qemu/autostart", base) < 0) goto out_of_memory; VIR_FREE(base); @@ -1340,6 +1336,7 @@ qemudStartup(int privileged) { if (qemudLoadDriverConfig(qemu_driver, driverConf) < 0) { goto error; } + VIR_FREE(driverConf); if (qemudSecurityInit(qemu_driver) < 0) goto error; @@ -1456,6 +1453,7 @@ error: if (conn) virConnectClose(conn); VIR_FREE(base); + VIR_FREE(driverConf); qemudShutdown(); return -1; } @@ -3300,21 +3298,22 @@ cleanup: } -static int qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, int tid) { - char proc[PATH_MAX]; +static int +qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, + int tid) +{ + char *proc; FILE *pidinfo; unsigned long long usertime, systime; int cpu; int ret; if (tid) - ret = snprintf(proc, sizeof(proc), "/proc/%d/task/%d/stat", pid, tid); + ret = virAsprintf(&proc, "/proc/%d/task/%d/stat", pid, tid); else - ret = snprintf(proc, sizeof(proc), "/proc/%d/stat", pid); - if (ret >= (int)sizeof(proc)) { - errno = E2BIG; + ret = virAsprintf(&proc, "/proc/%d/stat", pid); + if (ret < 0) return -1; - } if (!(pidinfo = fopen(proc, "r"))) { /* VM probably shut down, so fake 0 */ @@ -3322,8 +3321,10 @@ static int qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pi *cpuTime = 0; if (lastCpu) *lastCpu = 0; + VIR_FREE(proc); return 0; } + VIR_FREE(proc); /* See 'man proc' for information about what all these fields are. We're * only interested in a very few of them */ -- 1.7.3.3
2010/12/18 Eric Blake <eblake@redhat.com>:
* src/qemu/qemu_driver.c (qemudLogFD, qemudLogReadFD) (qemudStartup, qemudGetProcessInfo): Use heap instead of stack. (qemudDomainDetachPciDiskDevice) (qemudDomainDetachSCSIDiskDevice): Minor optimization. --- src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++----------------------- 1 files changed, 27 insertions(+), 26 deletions(-)
ACK. Matthias
On 12/18/2010 09:22 AM, Matthias Bolte wrote:
2010/12/18 Eric Blake <eblake@redhat.com>:
* src/qemu/qemu_driver.c (qemudLogFD, qemudLogReadFD) (qemudStartup, qemudGetProcessInfo): Use heap instead of stack. (qemudDomainDetachPciDiskDevice) (qemudDomainDetachSCSIDiskDevice): Minor optimization. --- src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++----------------------- 1 files changed, 27 insertions(+), 26 deletions(-)
ACK.
Thanks for the review. I've pushed 1 through 6, and am now doing some more extensive testing on the combined version of your and my updates to patch 7; I'll reply to that one separately once I complete testing. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* configure.ac (XDR_CFLAGS): Define when needed. * src/Makefile.am (libvirt_driver_remote_la_CFLAGS): Use it. --- configure.ac | 24 ++++++++++++++++++++++++ src/Makefile.am | 2 +- 2 files changed, 25 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index 4df915a..3b02209 100644 --- a/configure.ac +++ b/configure.ac @@ -339,6 +339,25 @@ if test x"$with_remote" = x"yes" || test x"$with_libvirtd" = x"yes"; then dnl check for cygwin's variation in xdr function names AC_CHECK_FUNCS([xdr_u_int64_t],[],[],[#include <rpc/xdr.h>]) + + dnl Cygwin requires -I/usr/include/tirpc for <rpc/rpc.h> + old_CFLAGS=$CFLAGS + AC_CACHE_CHECK([where to find <rpc/rpc.h>], [lv_cv_xdr_cflags], [ + for CFLAGS in '' '-I/usr/include/tirpc' 'missing'; do + if test x"$CFLAGS" = xmissing; then + lv_cv_xdr_cflags=missing; break + fi + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <rpc/rpc.h> + ]])], [lv_cv_xdr_cflags=${CFLAGS:-none}; break]) + done + ]) + CFLAGS=$old_CFLAGS + case $lv_cv_xdr_cflags in + none) XDR_CFLAGS= ;; + missing) AC_MSG_ERROR([Unable to find <rpc/rpc.h>]) ;; + *) XDR_CFLAGS=$lv_cv_xdr_cflags ;; + esac + AC_SUBST([XDR_CFLAGS]) fi @@ -2438,6 +2457,11 @@ AC_MSG_NOTICE([ mscom: $MSCOM_LIBS]) else AC_MSG_NOTICE([ mscom: no]) fi +if test "$with_remote" = "yes" || test "$with_libvirtd" = "yes" ; then +AC_MSG_NOTICE([ xdr: $XDR_CFLAGS]) +else +AC_MSG_NOTICE([ xdr: no]) +fi AC_MSG_NOTICE([]) AC_MSG_NOTICE([Test suite]) AC_MSG_NOTICE([]) diff --git a/src/Makefile.am b/src/Makefile.am index 6749786..39f537c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -486,7 +486,7 @@ libvirt_la_BUILT_LIBADD += libvirt_driver_remote.la endif libvirt_driver_remote_la_CFLAGS = \ $(GNUTLS_CFLAGS) \ - $(SASL_CFLAGS) \ + $(SASL_CFLAGS) $(XDR_CFLAGS) \ -I@top_srcdir@/src/conf \ $(AM_CFLAGS) libvirt_driver_remote_la_LDFLAGS = $(AM_LDFLAGS) -- 1.7.3.3
* configure.ac (dlopen): Cygwin dlopen is in libc; avoid spurious failure. (XDR_CFLAGS): Define when needed. * src/Makefile.am (libvirt_driver_remote_la_CFLAGS): Use it. --- This fixed things so I could again compile on cygwin. configure.ac | 33 +++++++++++++++++++++++++++------ src/Makefile.am | 2 +- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 4df915a..50ee862 100644 --- a/configure.ac +++ b/configure.ac @@ -339,6 +339,25 @@ if test x"$with_remote" = x"yes" || test x"$with_libvirtd" = x"yes"; then dnl check for cygwin's variation in xdr function names AC_CHECK_FUNCS([xdr_u_int64_t],[],[],[#include <rpc/xdr.h>]) + + dnl Cygwin requires -I/usr/include/tirpc for <rpc/rpc.h> + old_CFLAGS=$CFLAGS + AC_CACHE_CHECK([where to find <rpc/rpc.h>], [lv_cv_xdr_cflags], [ + for CFLAGS in '' '-I/usr/include/tirpc' 'missing'; do + if test x"$CFLAGS" = xmissing; then + lv_cv_xdr_cflags=missing; break + fi + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <rpc/rpc.h> + ]])], [lv_cv_xdr_cflags=${CFLAGS:-none}; break]) + done + ]) + CFLAGS=$old_CFLAGS + case $lv_cv_xdr_cflags in + none) XDR_CFLAGS= ;; + missing) AC_MSG_ERROR([Unable to find <rpc/rpc.h>]) ;; + *) XDR_CFLAGS=$lv_cv_xdr_cflags ;; + esac + AC_SUBST([XDR_CFLAGS]) fi @@ -359,12 +378,9 @@ AC_DEFINE_UNQUOTED([VBOX_XPCOMC_DIR], ["$vbox_xpcomc_dir"], if test "x$with_vbox" = "xyes"; then AC_SEARCH_LIBS([dlopen], [dl],,) - case $ac_cv_search_dlopen in - no*) DLOPEN_LIBS= - case "$host" in - *-*-mingw* | *-*-msvc*) ;; - *) AC_MSG_ERROR([Unable to find dlopen()]) ;; - esac ;; + case $ac_cv_search_dlopen:$host_os in + 'none required'* | *:mingw* | *:msvc*) DLOPEN_LIBS= ;; + no*) AC_MSG_ERROR([Unable to find dlopen()]) ;; *) DLOPEN_LIBS=$ac_cv_search_dlopen ;; esac AC_SUBST([DLOPEN_LIBS]) @@ -2438,6 +2454,11 @@ AC_MSG_NOTICE([ mscom: $MSCOM_LIBS]) else AC_MSG_NOTICE([ mscom: no]) fi +if test "$with_remote" = "yes" || test "$with_libvirtd" = "yes" ; then +AC_MSG_NOTICE([ xdr: $XDR_CFLAGS]) +else +AC_MSG_NOTICE([ xdr: no]) +fi AC_MSG_NOTICE([]) AC_MSG_NOTICE([Test suite]) AC_MSG_NOTICE([]) diff --git a/src/Makefile.am b/src/Makefile.am index 6749786..39f537c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -486,7 +486,7 @@ libvirt_la_BUILT_LIBADD += libvirt_driver_remote.la endif libvirt_driver_remote_la_CFLAGS = \ $(GNUTLS_CFLAGS) \ - $(SASL_CFLAGS) \ + $(SASL_CFLAGS) $(XDR_CFLAGS) \ -I@top_srcdir@/src/conf \ $(AM_CFLAGS) libvirt_driver_remote_la_LDFLAGS = $(AM_LDFLAGS) -- 1.7.3.3
2010/12/18 Eric Blake <eblake@redhat.com>:
* configure.ac (dlopen): Cygwin dlopen is in libc; avoid spurious failure. (XDR_CFLAGS): Define when needed. * src/Makefile.am (libvirt_driver_remote_la_CFLAGS): Use it. ---
This fixed things so I could again compile on cygwin.
configure.ac | 33 +++++++++++++++++++++++++++------ src/Makefile.am | 2 +- 2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/configure.ac b/configure.ac index 4df915a..50ee862 100644 --- a/configure.ac +++ b/configure.ac @@ -339,6 +339,25 @@ if test x"$with_remote" = x"yes" || test x"$with_libvirtd" = x"yes"; then
dnl check for cygwin's variation in xdr function names AC_CHECK_FUNCS([xdr_u_int64_t],[],[],[#include <rpc/xdr.h>]) + + dnl Cygwin requires -I/usr/include/tirpc for <rpc/rpc.h> + old_CFLAGS=$CFLAGS + AC_CACHE_CHECK([where to find <rpc/rpc.h>], [lv_cv_xdr_cflags], [ + for CFLAGS in '' '-I/usr/include/tirpc' 'missing'; do + if test x"$CFLAGS" = xmissing; then + lv_cv_xdr_cflags=missing; break + fi + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <rpc/rpc.h> + ]])], [lv_cv_xdr_cflags=${CFLAGS:-none}; break]) + done + ]) + CFLAGS=$old_CFLAGS + case $lv_cv_xdr_cflags in + none) XDR_CFLAGS= ;; + missing) AC_MSG_ERROR([Unable to find <rpc/rpc.h>]) ;; + *) XDR_CFLAGS=$lv_cv_xdr_cflags ;; + esac + AC_SUBST([XDR_CFLAGS]) fi
Due to the specific directory layout I use in my msys_setup this breaks my MinGW build. The problem is that I have MSYS and MinGW in different base directories and "mount" the MinGW directory into the MSYS environment. I have rpc/rpc.h in MSYS in /include (MSYS internally "symlinks" /usr/include to /include), due to the directory layout this is not in the default GCC include path. I use "CFLAGS=-I/include ./configure" to fix this. Now the new check ignores CFLAGS making my build fail. This incremental patch fixes it, by including the current CFLAGS instead of overriding them. diff --git a/configure.ac b/configure.ac index 27239f6..93532a6 100644 --- a/configure.ac +++ b/configure.ac @@ -343,12 +343,13 @@ if test x"$with_remote" = x"yes" || test x"$with_libvirtd" = x"yes"; then dnl Cygwin requires -I/usr/include/tirpc for <rpc/rpc.h> old_CFLAGS=$CFLAGS AC_CACHE_CHECK([where to find <rpc/rpc.h>], [lv_cv_xdr_cflags], [ - for CFLAGS in '' '-I/usr/include/tirpc' 'missing'; do - if test x"$CFLAGS" = xmissing; then + for add_CFLAGS in '' '-I/usr/include/tirpc' 'missing'; do + if test x"$add_CFLAGS" = xmissing; then lv_cv_xdr_cflags=missing; break fi + CFLAGS="$old_CFLAGS $add_CFLAGS" AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <rpc/rpc.h> - ]])], [lv_cv_xdr_cflags=${CFLAGS:-none}; break]) + ]])], [lv_cv_xdr_cflags=${add_CFLAGS:-none}; break]) done ]) CFLAGS=$old_CFLAGS Matthias
On 12/18/2010 02:52 PM, Matthias Bolte wrote:
2010/12/18 Eric Blake <eblake@redhat.com>:
* configure.ac (dlopen): Cygwin dlopen is in libc; avoid spurious failure. (XDR_CFLAGS): Define when needed. * src/Makefile.am (libvirt_driver_remote_la_CFLAGS): Use it. ---
This incremental patch fixes it, by including the current CFLAGS instead of overriding them.
diff --git a/configure.ac b/configure.ac index 27239f6..93532a6 100644 --- a/configure.ac +++ b/configure.ac @@ -343,12 +343,13 @@ if test x"$with_remote" = x"yes" || test x"$with_libvirtd" = x"yes"; then dnl Cygwin requires -I/usr/include/tirpc for <rpc/rpc.h> old_CFLAGS=$CFLAGS AC_CACHE_CHECK([where to find <rpc/rpc.h>], [lv_cv_xdr_cflags], [ - for CFLAGS in '' '-I/usr/include/tirpc' 'missing'; do - if test x"$CFLAGS" = xmissing; then + for add_CFLAGS in '' '-I/usr/include/tirpc' 'missing'; do + if test x"$add_CFLAGS" = xmissing; then lv_cv_xdr_cflags=missing; break fi + CFLAGS="$old_CFLAGS $add_CFLAGS" AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <rpc/rpc.h> - ]])], [lv_cv_xdr_cflags=${CFLAGS:-none}; break]) + ]])], [lv_cv_xdr_cflags=${add_CFLAGS:-none}; break])
ACK to your additions, and coupled with your ACK of the rest of my patch, this passed testing on Fedora 14, RHEL 5, a Linux-hosted cross-build to mingw, and a native cygwin build, so I've gone ahead and pushed it. However, in the meantime, I've already mentioned to you on IRC that --with-vmware --without-esx creates a broken build right now, which can easily be the case when you don't have curl-devel headers installed (esx won't build without curl, but vmware will). ./autobuild.sh fails as a result of that, because 'make rpm' doesn't yet know to build --without-vmware by default. I'll let you work on the patch for dependencies between vmware and esx, while I work on cleaning up the .spec file for 'make rpm'. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 21/12/2010, at 6:52 AM, Eric Blake wrote:
ACK to your additions, and coupled with your ACK of the rest of my patch, this passed testing on Fedora 14, RHEL 5, a Linux-hosted cross-build to mingw, and a native cygwin build, so I've gone ahead and pushed it.
Thanks guys. This also fixed the dlopen() problem in ./configure on OSX too, mentioned yesterday, as Eric suggested it might. ./configure working fine now. :)
2010/12/18 Eric Blake <eblake@redhat.com>:
* configure.ac (XDR_CFLAGS): Define when needed. * src/Makefile.am (libvirt_driver_remote_la_CFLAGS): Use it. --- configure.ac | 24 ++++++++++++++++++++++++ src/Makefile.am | 2 +- 2 files changed, 25 insertions(+), 1 deletions(-)
ACK. Matthias
2010/12/18 Matthias Bolte <matthias.bolte@googlemail.com>:
2010/12/18 Eric Blake <eblake@redhat.com>:
* configure.ac (XDR_CFLAGS): Define when needed. * src/Makefile.am (libvirt_driver_remote_la_CFLAGS): Use it. --- configure.ac | 24 ++++++++++++++++++++++++ src/Makefile.am | 2 +- 2 files changed, 25 insertions(+), 1 deletions(-)
ACK.
Matthias
Well, I revoke my ACK, as this breaks my MinGW build, I should have tested first :( Also, I should have noticed v3 earlier. Matthias
participants (3)
-
Eric Blake -
Justin Clift -
Matthias Bolte