Re: [libvirt] broken OpenBSD net/if.h [was: [PATCH] Include some extra headers needed for OpenBSD.]
On Tue, Sep 04, 2012 at 11:37:30AM -0600, Eric Blake wrote: On 09/04/2012 11:23 AM, Jasper Lievisse Adriaanse wrote: On Tue, Sep 04, 2012 at 11:08:30AM -0600, Eric Blake wrote: [adding gnulib] Ouch. The POSIX definition of net/if.h doesn't include any interface that needs to use struct sockaddr. Which OpenBSD extension function is triggering this warning? According to POSIX, this .c file should compile: #define _POSIX_C_SOURCE 200809L #include net/if.h #include sys/socket.h struct if_nameindex i; That snippet of example code does not compile on OpenBSD: In file included from foo.c:2: /usr/include/net/if.h:112: error: expected specifier-qualifier-list before 'u_int' Could you please reference where POSIX states it should, so this can hopefully get fixed in OpenBSD. http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/net_if.h.html#tag_13_30 doesn't mention any prerequisite headers for net/if.h. Furthermore, looking at each of the functions in that header, none of them call out a prerequisite: http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_freenameindex.html# http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_indextoname.html# http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_nameindex.html# http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_nametoindex.html# If a header cannot be used in isolation, then POSIX would call out multiple headers. For comparison, here's a case where POSIX 2008 _does_ call out multiple headers: http://pubs.opengroup.org/onlinepubs/9699919799/functions/creat.html creat() cannot use the S_IRUSR and other constants for its mode_t argument unless you also include sys/stat.h, since fcntl.h was not required to be self-contained on that front (there's talk underway about getting that fixed in the next version of POSIX, but that's another story... http://austingroupbugs.net/view.php?id=591) -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org A diff for the OpenBSD header file has been created and is currently being tested. So this should be fixed pretty soon (and in time for OpenBSD 5.3). -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Pass a correct pointer type to localtime_r(3).
From b53dc971cc50b5ac397e4568449d25041477c8d6 Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 16:47:26 +0200 Subject: [PATCH] Pass a correct pointer type to localtime_r(3). Fixes a warning: warning: passing argument 1 of 'localtime_r' from incompatible pointer type --- tools/virsh-domain.c |3 ++- tools/virsh.c|3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f0ec742..535779c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3711,6 +3711,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) struct tm time_info; const char *ext = NULL; char *ret = NULL; +time_t sec = (time_t) cur_time.tv_sec; if (!dom) { vshError(ctl, %s, _(Invalid domain supplied)); @@ -3724,7 +3725,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) /* add mime type here */ gettimeofday(cur_time, NULL); -localtime_r(cur_time.tv_sec, time_info); +localtime_r(sec, time_info); strftime(timestr, sizeof(timestr), %Y-%m-%d-%H:%M:%S, time_info); if (virAsprintf(ret, %s-%s%s, virDomainGetName(dom), diff --git a/tools/virsh.c b/tools/virsh.c index 5cf3237..5be2a3c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2189,6 +2189,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, const char *lvl = ; struct timeval stTimeval; struct tm *stTm; +time_t sec = stTimeval.tv_sec; if (ctl-log_fd == -1) return; @@ -2199,7 +2200,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, * [.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message */ gettimeofday(stTimeval, NULL); -stTm = localtime(stTimeval.tv_sec); +stTm = localtime(sec); virBufferAsprintf(buf, [%d.%02d.%02d %02d:%02d:%02d %s %d] , (1900 + stTm-tm_year), (1 + stTm-tm_mon), -- 1.7.6 -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Define DYNLIB_NAME on OpenBSD.
From 05dd99030d865127c874d1b489b9c17412bdbb3b Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 16:48:51 +0200 Subject: [PATCH] Define DYNLIB_NAME on OpenBSD. --- src/vbox/vbox_XPCOMCGlue.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/vbox/vbox_XPCOMCGlue.c b/src/vbox/vbox_XPCOMCGlue.c index e7e9c37..63470ae 100644 --- a/src/vbox/vbox_XPCOMCGlue.c +++ b/src/vbox/vbox_XPCOMCGlue.c @@ -48,7 +48,7 @@ /*** * Defined Constants And Macros * ***/ -#if defined(__linux__) || defined(__linux_gnu__) || defined(__sun__) || defined(__FreeBSD__) +#if defined(__linux__) || defined(__linux_gnu__) || defined(__sun__) || defined(__FreeBSD__) || defined(__OpenBSD__) # define DYNLIB_NAMEVBoxXPCOMC.so #elif defined(__APPLE__) # define DYNLIB_NAMEVBoxXPCOMC.dylib -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Pass a correct pointer type to localtime_r(3).
FYI, these patches and those I will send later supersede the previous patchsets I sent yesterday for fixing and making libvirt work on OpenBSD. On Tue, Sep 04, 2012 at 04:49:52PM +0200, Jasper Lievisse Adriaanse wrote: From b53dc971cc50b5ac397e4568449d25041477c8d6 Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 16:47:26 +0200 Subject: [PATCH] Pass a correct pointer type to localtime_r(3). Fixes a warning: warning: passing argument 1 of 'localtime_r' from incompatible pointer type --- tools/virsh-domain.c |3 ++- tools/virsh.c|3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f0ec742..535779c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3711,6 +3711,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) struct tm time_info; const char *ext = NULL; char *ret = NULL; +time_t sec = (time_t) cur_time.tv_sec; if (!dom) { vshError(ctl, %s, _(Invalid domain supplied)); @@ -3724,7 +3725,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) /* add mime type here */ gettimeofday(cur_time, NULL); -localtime_r(cur_time.tv_sec, time_info); +localtime_r(sec, time_info); strftime(timestr, sizeof(timestr), %Y-%m-%d-%H:%M:%S, time_info); if (virAsprintf(ret, %s-%s%s, virDomainGetName(dom), diff --git a/tools/virsh.c b/tools/virsh.c index 5cf3237..5be2a3c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2189,6 +2189,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, const char *lvl = ; struct timeval stTimeval; struct tm *stTm; +time_t sec = stTimeval.tv_sec; if (ctl-log_fd == -1) return; @@ -2199,7 +2200,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, * [.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message */ gettimeofday(stTimeval, NULL); -stTm = localtime(stTimeval.tv_sec); +stTm = localtime(sec); virBufferAsprintf(buf, [%d.%02d.%02d %02d:%02d:%02d %s %d] , (1900 + stTm-tm_year), (1 + stTm-tm_mon), -- 1.7.6 -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Use sockpeercred on OpenBSD.
From 0f352c0577f76a64924636a8c9ba72de5862b6d6 Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 16:59:55 +0200 Subject: [PATCH] Use sockpeercred on OpenBSD. --- src/rpc/virnetsocket.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 5a48300..27ead13 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -988,7 +988,11 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, gid_t *gid, pid_t *pid) { +#if defined (__OpenBSD__) +struct sockpeercred cr; +#else struct ucred cr; +#endif socklen_t cr_len = sizeof(cr); virMutexLock(sock-lock); -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Include some extra headers needed for OpenBSD.
From bafcb4ed2b90b5ba845ca6b61861e3caa548b16a Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 16:57:09 +0200 Subject: [PATCH] Include some extra headers needed for OpenBSD. --- src/util/virnetdevbridge.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 7b11bee..8559223 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -30,6 +30,15 @@ #include intprops.h #include sys/ioctl.h + +#ifdef HAVE_SYS_PARAM_H +# include sys/param.h +#endif + +#ifdef HAVE_SYS_SOCKET_H +# include sys/socket.h +#endif + #ifdef HAVE_NET_IF_H # include net/if.h #endif -- 1.7.6 -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Don't link with -lrt on OpenBSD
From bdf3bce37531ec346474bc5c4f37a5d2985d1d35 Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 17:03:43 +0200 Subject: [PATCH] Don't link with -lrt on OpenBSD --- configure.ac |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index e0d00d5..54d049d 100644 --- a/configure.ac +++ b/configure.ac @@ -188,7 +188,13 @@ RT_LIBS= LIBS=$LIBS $LIB_PTHREAD -lrt AC_CHECK_FUNC([clock_gettime],[ AC_DEFINE([HAVE_CLOCK_GETTIME],[],[Defined if clock_gettime() exists in librt.so]) - RT_LIBS=-lrt + case $host in + *-*-openbsd*) + ;; + *) + RT_LIBS=-lrt + ;; + esac ]) LIBS=$old_libs AC_SUBST(RT_LIBS) -- 1.7.6 -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Include some extra headers needed for OpenBSD.
On Tue, Sep 04, 2012 at 09:18:20AM -0600, Eric Blake wrote: On 09/04/2012 08:57 AM, Jasper Lievisse Adriaanse wrote: From bafcb4ed2b90b5ba845ca6b61861e3caa548b16a Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 16:57:09 +0200 Subject: [PATCH] Include some extra headers needed for OpenBSD. --- src/util/virnetdevbridge.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) Please show the compiler errors that you got without these includes. I can't help but wonder if you have instead uncovered a bug in the gnulib headers, but knowing which symbols were not declared makes a difference in answering that question. diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 7b11bee..8559223 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -30,6 +30,15 @@ #include intprops.h #include sys/ioctl.h + +#ifdef HAVE_SYS_PARAM_H sys/param.h is non-standard; what are we using that requires us to probe for its existence? Should gnulib consider guaranteeing this header in spite of it being non-standard? Nope, it was an error on my side to include it. I thought it was needed by socket.h, but after trying to show you the error...there was none. +# include sys/param.h +#endif + +#ifdef HAVE_SYS_SOCKET_H This line shouldn't be necessary; gnulib guarantees a working sys/socket.h on all architectures. OK. Could you please push this one then? -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Use sockpeercred on OpenBSD.
On Tue, Sep 04, 2012 at 09:22:56AM -0600, Eric Blake wrote: On 09/04/2012 09:00 AM, Jasper Lievisse Adriaanse wrote: From 0f352c0577f76a64924636a8c9ba72de5862b6d6 Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 16:59:55 +0200 Subject: [PATCH] Use sockpeercred on OpenBSD. --- src/rpc/virnetsocket.c |4 1 files changed, 4 insertions(+), 0 deletions(-) I _hate_ OS-specific #ifdefs. I'd much rather see a configure-time check to determine whether 'struct ucred', 'struct sockpeercred', or both, exist, and then base the decision on which struct to use based on the result of that configure-time check. diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 5a48300..27ead13 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -988,7 +988,11 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, gid_t *gid, pid_t *pid) { +#if defined (__OpenBSD__) +struct sockpeercred cr; +#else struct ucred cr; +#endif socklen_t cr_len = sizeof(cr); virMutexLock(sock-lock); Give me some time to try to help turn this into a proper configure-time check. Great, thanks! -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Pass a correct pointer type to localtime_r(3).
On Tue, Sep 04, 2012 at 09:20:24AM -0600, Eric Blake wrote: [adding bug-gnulib] On 09/04/2012 08:49 AM, Jasper Lievisse Adriaanse wrote: From b53dc971cc50b5ac397e4568449d25041477c8d6 Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 16:47:26 +0200 Subject: [PATCH] Pass a correct pointer type to localtime_r(3). Fixes a warning: warning: passing argument 1 of 'localtime_r' from incompatible pointer type --- tools/virsh-domain.c |3 ++- tools/virsh.c|3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) NACK from the libvirt point of view. tv_sec is required by POSIX to be of type time_t; so this is a bug in the OpenBSD header, and gnulib should be working around this bug. OpenBSD's sys/time.h has this: /* * Structure returned by gettimeofday(2) system call, * and used in other calls. */ struct timeval { longtv_sec; /* seconds */ longtv_usec;/* and microseconds */ }; #ifndef _TIMESPEC_DECLARED #define _TIMESPEC_DECLARED /* * Structure defined by POSIX.1b to be like a timeval. */ struct timespec { time_t tv_sec; /* seconds */ longtv_nsec;/* and nanoseconds */ }; #endif diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f0ec742..535779c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3711,6 +3711,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) struct tm time_info; const char *ext = NULL; char *ret = NULL; +time_t sec = (time_t) cur_time.tv_sec; if (!dom) { vshError(ctl, %s, _(Invalid domain supplied)); @@ -3724,7 +3725,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) /* add mime type here */ gettimeofday(cur_time, NULL); -localtime_r(cur_time.tv_sec, time_info); +localtime_r(sec, time_info); strftime(timestr, sizeof(timestr), %Y-%m-%d-%H:%M:%S, time_info); +++ b/tools/virsh.c @@ -2189,6 +2189,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, const char *lvl = ; struct timeval stTimeval; struct tm *stTm; +time_t sec = stTimeval.tv_sec; if (ctl-log_fd == -1) return; @@ -2199,7 +2200,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, * [.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message */ gettimeofday(stTimeval, NULL); -stTm = localtime(stTimeval.tv_sec); +stTm = localtime(sec); Even grosser - why is virsh using localtime() instead of localtime_r()? Oversight probably.. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Include some extra headers needed for OpenBSD.
On Tue, Sep 04, 2012 at 10:45:41AM -0600, Eric Blake wrote: On 09/04/2012 10:23 AM, Jasper Lievisse Adriaanse wrote: On Tue, Sep 04, 2012 at 09:18:20AM -0600, Eric Blake wrote: On 09/04/2012 08:57 AM, Jasper Lievisse Adriaanse wrote: From bafcb4ed2b90b5ba845ca6b61861e3caa548b16a Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 16:57:09 +0200 Subject: [PATCH] Include some extra headers needed for OpenBSD. --- src/util/virnetdevbridge.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) Please show the compiler errors that you got without these includes. I can't help but wonder if you have instead uncovered a bug in the gnulib headers, but knowing which symbols were not declared makes a difference in answering that question. I'd still like to know the compiler error you got when sys/socket.h was not present, but just guessing from the source code, I see one call of socket() (protected behind #if defined(HAVE_NET_IF_H) defined(SIOCBRADDBR), but maybe those are both true for OpenBSD?). Even though I'm pushing, I would STILL like to know why. Of course, here it is: In file included from util/virnetdevbridge.c:35: /usr/include/net/if.h:276: warning: 'struct sockaddr' declared inside parameter list /usr/include/net/if.h:276: warning: its scope is only this definition or declaration, which is probably not what you want /usr/include/net/if.h:280: warning: 'struct sockaddr' declared inside parameter list /usr/include/net/if.h:293: error: 'AF_MAX' undeclared here (not in a function) /usr/include/net/if.h:606: error: field 'ifru_addr' has incomplete type /usr/include/net/if.h:607: error: field 'ifru_dstaddr' has incomplete type /usr/include/net/if.h:608: error: field 'ifru_broadaddr' has incomplete type /usr/include/net/if.h:626: error: field 'ifra_addr' has incomplete type /usr/include/net/if.h:627: error: field 'ifra_dstaddr' has incomplete type /usr/include/net/if.h:629: error: field 'ifra_mask' has incomplete type /usr/include/net/if.h:673: error: field 'addr' has incomplete type /usr/include/net/if.h:674: error: field 'dstaddr' has incomplete type In file included from /usr/include/net/if.h:692, from util/virnetdevbridge.c:35: /usr/include/net/if_arp.h:79: error: field 'arp_pa' has incomplete type /usr/include/net/if_arp.h:80: error: field 'arp_ha' has incomplete type Error while executing cc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib -I../gnulib/lib -I../include -I../include -I../src/util -DIN_LIBVIRT -I/usr/local/include -I/usr/local/include/libxml2 -I/usr/local/include -Wall -W -Wformat-y2k -Wformat-security -Winit-self -Wmissing-include-dirs -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align -Wwrite-strings -Wstrict-prototypes -Wold-style-definition -Wmissing-noreturn -Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline -Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization -Wattributes -Wdeprecated-declarations -Wdiv-by-zero -Wendif-labels -Wextra -Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar -Wnormalized=nfc -Woverflow -Wpointer-to-int-cast -Wpragmas -Wtrampolines -Wno-missing-field-initializers -Wno-sign-compare -Wno-format-nonliteral -fexceptions -fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time -fipa-pure-const -I/usr/local/include/dbus-1.0 -I/usr/local/lib/dbus-1.0/include -O2 -pipe -MT libvirt_util_la-virnetdevbridge.lo -MD -MP -MF .deps/libvirt_util_la-virnetdevbridge.Tpo -c util/virnetdevbridge.c -fPIC -DPIC -o .libs/libvirt_util_la-virnetdevbridge.o gmake[3]: *** [libvirt_util_la-virnetdevbridge.lo] Error 1 gmake[3]: Leaving directory `/usr/obj/ports/libvirt-0.10.1/libvirt-0.10.1/src' gmake[2]: *** [all] Error 2 gmake[2]: Leaving directory `/usr/obj/ports/libvirt-0.10.1/libvirt-0.10.1/src' gmake[1]: *** [all-recursive] Error 1 gmake[1]: Leaving directory `/usr/obj/ports/libvirt-0.10.1/libvirt-0.10.1' gmake: *** [all] Error 2 This line shouldn't be necessary; gnulib guarantees a working sys/socket.h on all architectures. OK. Could you please push this one then? Here's what I squashed in before pushing; I also added you to AUTHORS. Let me know if you prefer an alternate spelling or email address (the file is in UTF-8, if that matters). Thanks, my name and e-mail as I sent them are ok. diff --git i/src/util/virnetdevbridge.c w/src/util/virnetdevbridge.c index 8559223..a29e4b2 100644 --- i/src/util/virnetdevbridge.c +++ w/src/util/virnetdevbridge.c @@ -30,14 +30,7 @@ #include intprops.h #include sys/ioctl.h - -#ifdef HAVE_SYS_PARAM_H -# include sys/param.h -#endif - -#ifdef HAVE_SYS_SOCKET_H -# include sys/socket.h -#endif +#include sys/socket.h #ifdef HAVE_NET_IF_H # include net/if.h -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Cheers, Jasper
Re: [libvirt] [PATCH] Don't link with -lrt on OpenBSD
On Tue, Sep 04, 2012 at 10:59:05AM -0600, Eric Blake wrote: On 09/04/2012 09:26 AM, Eric Blake wrote: On 09/04/2012 09:04 AM, Jasper Lievisse Adriaanse wrote: From bdf3bce37531ec346474bc5c4f37a5d2985d1d35 Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse jas...@humppa.nl Date: Tue, 4 Sep 2012 17:03:43 +0200 Subject: [PATCH] Don't link with -lrt on OpenBSD --- configure.ac |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) I'd much rather write this as a probe to see if linking with -rt makes a difference, and only add it when it matters, than trying to blindly add it (the pre-patch version) or to blindly exclude it for some platforms (the post-patch version). I also wonder if gnulib has some help on this front. NACK to this version while I investigate. In fact, gnulib already provides the LGPLv2+ clock-time module for clock_gettime and clock_settime, with an appropriate link library $(LIB_CLOCK_GETTIME) that is set to -lrt when necessary. Does this patch work for you? Yep, this seems to work just fine. From d74e5a4dfc434d3a1d01856d013a7f50d910fa95 Mon Sep 17 00:00:00 2001 From: Eric Blake ebl...@redhat.com Date: Tue, 4 Sep 2012 10:57:25 -0600 Subject: [PATCH] build: use correct libraries for clock_gettime On OpenBSD, clock_gettime() exists in libc rather than librt, and blindly linking with -lrt made the build fail. Gnulib already did the work for determining which libraries to use, so we should reuse that work rather than doing it ourselves. * bootstrap.conf (gnulib_modules): Pull in clock-time. * configure.ac (RT_LIBS): Drop. * src/Makefile.am (libvirt_util_la_LIBADD): Use gnulib variable instead. * src/util/virtime.c (includes): Simplify. --- bootstrap.conf | 1 + configure.ac | 10 -- src/Makefile.am| 2 +- src/util/virtime.c | 6 ++ 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index a6cfe24..7fefb69 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -30,6 +30,7 @@ c-strcasestr calloc-posix canonicalize-lgpl chown +clock-time close connect configmake diff --git a/configure.ac b/configure.ac index e0d00d5..cb91e7d 100644 --- a/configure.ac +++ b/configure.ac @@ -183,16 +183,6 @@ LIBS=$LIBS $LIB_PTHREAD $LIBMULTITHREAD AC_CHECK_FUNCS([pthread_mutexattr_init]) LIBS=$old_libs -old_LIBS=$LIBS -RT_LIBS= -LIBS=$LIBS $LIB_PTHREAD -lrt -AC_CHECK_FUNC([clock_gettime],[ - AC_DEFINE([HAVE_CLOCK_GETTIME],[],[Defined if clock_gettime() exists in librt.so]) - RT_LIBS=-lrt -]) -LIBS=$old_libs -AC_SUBST(RT_LIBS) - dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ diff --git a/src/Makefile.am b/src/Makefile.am index 95e1bea..39adeac 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -668,7 +668,7 @@ libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ $(DBUS_CFLAGS) $(LDEXP_LIBM) libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ - $(RT_LIBS) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) + $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) noinst_LTLIBRARIES += libvirt_conf.la diff --git a/src/util/virtime.c b/src/util/virtime.c index 926bb50..d34e8ab 100644 --- a/src/util/virtime.c +++ b/src/util/virtime.c @@ -1,7 +1,7 @@ /* * virtime.c: Time handling functions * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -34,9 +34,7 @@ #include config.h #include stdio.h -#ifndef HAVE_CLOCK_GETTIME -# include sys/time.h -#endif +#include sys/time.h #include virtime.h #include util.h -- 1.7.11.4 -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] broken OpenBSD net/if.h [was: [PATCH] Include some extra headers needed for OpenBSD.]
On Tue, Sep 04, 2012 at 11:08:30AM -0600, Eric Blake wrote: [adding gnulib] On 09/04/2012 10:52 AM, Jasper Lievisse Adriaanse wrote: I'd still like to know the compiler error you got when sys/socket.h was not present, but just guessing from the source code, I see one call of socket() (protected behind #if defined(HAVE_NET_IF_H) defined(SIOCBRADDBR), but maybe those are both true for OpenBSD?). Even though I'm pushing, I would STILL like to know why. Of course, here it is: In file included from util/virnetdevbridge.c:35: /usr/include/net/if.h:276: warning: 'struct sockaddr' declared inside parameter list Ouch. The POSIX definition of net/if.h doesn't include any interface that needs to use struct sockaddr. Which OpenBSD extension function is triggering this warning? According to POSIX, this .c file should compile: #define _POSIX_C_SOURCE 200809L #include net/if.h #include sys/socket.h struct if_nameindex i; and it might just compile on OpenBSD (I haven't checked myself); the difference is that we have explicitly asked for namespace pollution beyond what _POSIX_C_SOURCE guarantees, which may explain why 'struct sockaddr' is interfering. But since net/if.h is required to be self-contained when in a strict environment, it makes sense for it to also be self-contained in an extension environment. It sounds like gnulib should consider providing a replacement net/if.h function to work around this lameness. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org That snippet of example code does not compile on OpenBSD: In file included from foo.c:2: /usr/include/net/if.h:112: error: expected specifier-qualifier-list before 'u_int' /usr/include/net/if.h:125: error: expected specifier-qualifier-list before 'u_char' /usr/include/net/if.h:188: error: expected specifier-qualifier-list before 'u_char' /usr/include/net/if.h:262: error: expected specifier-qualifier-list before 'u_short' /usr/include/net/if.h:474: error: expected specifier-qualifier-list before 'u_int' /usr/include/net/if.h:485: error: expected specifier-qualifier-list before 'u_int' /usr/include/net/if.h:493: error: expected specifier-qualifier-list before 'u_short' /usr/include/net/if.h:512: error: expected specifier-qualifier-list before 'u_short' /usr/include/net/if.h:530: error: expected specifier-qualifier-list before 'u_short' /usr/include/net/if.h:551: error: expected specifier-qualifier-list before 'u_int' /usr/include/net/if.h:586: error: expected specifier-qualifier-list before 'u_int' /usr/include/net/if.h:606: error: field 'ifru_addr' has incomplete type /usr/include/net/if.h:607: error: field 'ifru_dstaddr' has incomplete type /usr/include/net/if.h:608: error: field 'ifru_broadaddr' has incomplete type /usr/include/net/if.h:626: error: field 'ifra_addr' has incomplete type /usr/include/net/if.h:627: error: field 'ifra_dstaddr' has incomplete type /usr/include/net/if.h:629: error: field 'ifra_mask' has incomplete type /usr/include/net/if.h:673: error: field 'addr' has incomplete type /usr/include/net/if.h:674: error: field 'dstaddr' has incomplete type In file included from /usr/include/net/if.h:691, from foo.c:2: /usr/include/net/if_arp.h:79: error: field 'arp_pa' has incomplete type /usr/include/net/if_arp.h:80: error: field 'arp_ha' has incomplete type *** Error code 1 Could you please reference where POSIX states it should, so this can hopefully get fixed in OpenBSD. -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Pass a correct pointer type to localtime_r(3).
On Tue, Sep 04, 2012 at 10:03:41AM -0700, Paul Eggert wrote: On 09/04/2012 08:20 AM, Eric Blake wrote: tv_sec is required by POSIX to be of type time_t; so this is a bug in the OpenBSD header Most likely this problem arose because of the patch I pushed in gnulib commit e07d7c40f3ca5ec410cf5aa6fa03cfe51e712039. Previously, gnulib required timeval's tv_sec to be the same size as time_t. But now, it requires only that tv_sec be big enough to hold a time_t. This patch was needed for Emacs. Without the patch, gnulib replaced struct timeval on OpenBSD, and this messed up utimens.c, and Emacs wouldn't build. Alternatively, gnulib could substitute its own struct timeval for the system's, wrapping every struct timeval-using function (gettimeofday, futimesat, futimes, lutimes, etc. That'd be more work, though. And it would introduce some performance issues with gettimeofday, which is supposed to be fast. I've been trying to get away from using struct timeval, and to use the higher-resolution struct timespec instead, so messing with these obsolescent interfaces has been lower priority for me. But if someone wants to take the more-ambitious approach that'd be fine, I expect. For this particular case, though, how about if we avoid the problem entirely? libvirt doesn't need to use struct timeval here at all. I'd use the following (untested) patch: it makes libvirt smaller and probably faster, and it ports to OpenBSD without messing with gnulib: Works fine here on OpenBSD. diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f0ec742..3cef782 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3707,7 +3707,7 @@ static char * vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) { char timestr[100]; -struct timeval cur_time; +time_t cur_time; struct tm time_info; const char *ext = NULL; char *ret = NULL; @@ -3723,8 +3723,8 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) ext = .png; /* add mime type here */ -gettimeofday(cur_time, NULL); -localtime_r(cur_time.tv_sec, time_info); +time (cur_time); +localtime_r(cur_time, time_info); strftime(timestr, sizeof(timestr), %Y-%m-%d-%H:%M:%S, time_info); if (virAsprintf(ret, %s-%s%s, virDomainGetName(dom), diff --git a/tools/virsh.c b/tools/virsh.c index 5cf3237..88da429 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2187,7 +2187,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, char *str; size_t len; const char *lvl = ; -struct timeval stTimeval; +time_t stTime; struct tm *stTm; if (ctl-log_fd == -1) @@ -2198,8 +2198,8 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, * * [.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message */ -gettimeofday(stTimeval, NULL); -stTm = localtime(stTimeval.tv_sec); +time (stTime); +stTm = localtime(stTime); virBufferAsprintf(buf, [%d.%02d.%02d %02d:%02d:%02d %s %d] , (1900 + stTm-tm_year), (1 + stTm-tm_mon), -- Cheers, Jasper Stay Hungry. Stay Foolish -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] Fixup some headers in virnetdevbridge.c, needed for OpenBSD.
--- src/util/virnetdevbridge.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 7b11bee..8559223 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -30,6 +30,15 @@ #include intprops.h #include sys/ioctl.h + +#ifdef HAVE_SYS_PARAM_H +# include sys/param.h +#endif + +#ifdef HAVE_SYS_SOCKET_H +# include sys/socket.h +#endif + #ifdef HAVE_NET_IF_H # include net/if.h #endif -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Initial OpenBSD support
Hi, Here are some patches to get libvirt building on OpenBSD. I tested basic functionality with virt-manager though there appear to be some rough edges to fix/polish (could also be in virt-manager). At least basic libvirt usage should work on OpenBSD now. Jasper Lievisse Adriaanse (4): Define DYNLIB_NAME on OpenBSD. Fixup some headers in virnetdevbridge.c, needed for OpenBSD. Check for sys/ucred.h and adjust virnetsocket.c for OpenBSD compilation. There's no librt on OpenBSD. configure.ac | 10 -- src/rpc/virnetsocket.c | 10 ++ src/util/virnetdevbridge.c |9 + src/vbox/vbox_XPCOMCGlue.c |2 +- 4 files changed, 28 insertions(+), 3 deletions(-) -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Define DYNLIB_NAME on OpenBSD.
--- src/vbox/vbox_XPCOMCGlue.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/vbox/vbox_XPCOMCGlue.c b/src/vbox/vbox_XPCOMCGlue.c index e7e9c37..63470ae 100644 --- a/src/vbox/vbox_XPCOMCGlue.c +++ b/src/vbox/vbox_XPCOMCGlue.c @@ -48,7 +48,7 @@ /*** * Defined Constants And Macros * ***/ -#if defined(__linux__) || defined(__linux_gnu__) || defined(__sun__) || defined(__FreeBSD__) +#if defined(__linux__) || defined(__linux_gnu__) || defined(__sun__) || defined(__FreeBSD__) || defined(__OpenBSD__) # define DYNLIB_NAMEVBoxXPCOMC.so #elif defined(__APPLE__) # define DYNLIB_NAMEVBoxXPCOMC.dylib -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] Check for sys/ucred.h and adjust virnetsocket.c for OpenBSD compilation.
--- configure.ac |2 +- src/rpc/virnetsocket.c | 10 ++ 2 files changed, 11 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index e0d00d5..7ba1e9c 100644 --- a/configure.ac +++ b/configure.ac @@ -197,7 +197,7 @@ dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ sys/un.h sys/syscall.h netinet/tcp.h ifaddrs.h libtasn1.h \ - net/if.h execinfo.h]) + net/if.h execinfo.h sys/ucred.h]) dnl We need to decide at configure time if libvirt will use real atomic dnl operations (lock free) or emulated ones with a mutex. diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 5a48300..0457710 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -31,6 +31,10 @@ #include fcntl.h #include netdb.h +#ifdef HAVE_SYS_UCRED_H +# include sys/ucred.h +#endif + #ifdef HAVE_NETINET_TCP_H # include netinet/tcp.h #endif @@ -999,9 +1003,15 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, return -1; } +#if defined(__OpenBSD__) +*pid = getpid(); +*uid = cr.cr_uid; +*gid = cr.cr_gid; +#else *pid = cr.pid; *uid = cr.uid; *gid = cr.gid; +#endif virMutexUnlock(sock-lock); return 0; -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] There's no librt on OpenBSD.
--- configure.ac |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index 7ba1e9c..701ab50 100644 --- a/configure.ac +++ b/configure.ac @@ -188,7 +188,13 @@ RT_LIBS= LIBS=$LIBS $LIB_PTHREAD -lrt AC_CHECK_FUNC([clock_gettime],[ AC_DEFINE([HAVE_CLOCK_GETTIME],[],[Defined if clock_gettime() exists in librt.so]) - RT_LIBS=-lrt + case $host in + *-*-openbsd*) + ;; + *) + RT_LIBS=-lrt +;; + esac ]) LIBS=$old_libs AC_SUBST(RT_LIBS) -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list