[libvirt] portability issue on Centos 6.0

I just tred compiling 0.9.4 and git head on Centos 6.0, for the possible libvirt.org server replacement and it fails in both cases with: CC libvirtd-remote.o remote.c: In function 'remoteDispatchAuthPolkit': remote.c:2161: error: invalid initializer This correspond to the following macro PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s", virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident); I'm a bit lost how the expansion could lead to this and why, unless the DTrace support in Centos 6 is slightly different than on RHEL, I will look but if someone has an idea :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 08/16/2011 03:57 AM, Daniel Veillard wrote:
I just tred compiling 0.9.4 and git head on Centos 6.0, for the possible libvirt.org server replacement and it fails in both cases with:
CC libvirtd-remote.o remote.c: In function 'remoteDispatchAuthPolkit': remote.c:2161: error: invalid initializer
What version of systemtap headers are installed?
This correspond to the following macro
PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s", virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident);
I'm a bit lost how the expansion could lead to this and why, unless the DTrace support in Centos 6 is slightly different than on RHEL, I will look but if someone has an idea :-)
This came up previously, and I wrote this RFC patch: https://www.redhat.com/archives/libvir-list/2011-July/msg00053.html but no one replied whether we really want it in the tree. commit c6793e397d0e62e2d00bfa1beb910fd09b254337 Author: Eric Blake <eblake@redhat.com> Date: Fri Jul 1 12:33:50 2011 -0600 build: work around older systemtap header Systemtap 1.2 <sys/sdt.h> tried to expand STAP_PROBE3 into an initialization: volatile __typeof__(arg) foo = arg; but that fails if arg was declared as 'char arg[100]'. Rather than make all callers to PROBE deal with the stupidity of <sys/sdt.h>, we instead make PROBE cast away the problem. Some of this preprocessor abuse copies ideas in src/libvirt.c. * daemon/libvirtd.h (PROBE): Add casts to all arguments, using... (VIR_ADD_CASTS, VIR_ADD_CAST, VIR_ADD_CAST2, VIR_ADD_CAST3) (VIR_ADD_CAST_EXPAND, VIR_ADD_CAST_PASTE, VIR_COUNT_ARGS) (VIR_ARG5, PROBE_EXPAND): New macros. Reported by Wen Congyang. diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 6c9b9c3..6d6460e 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -44,11 +44,41 @@ # define LIBVIRTD_PROBES_H # include "probes.h" # endif /* LIBVIRTD_PROBES_H */ + +/* Systemtap 1.2 headers have a bug where they cannot handle a + * variable declared with array type. Work around this by casting all + * arguments. This is some gross use of the preprocessor because + * PROBE is a var-arg macro, but it is better than the alternative of + * making all callers to PROBE have to be aware of the issues. And + * hopefully, if we ever add a call to PROBE with other than 2 or 3 + * end arguments, you can figure out the pattern to extend this hack. + */ +# define VIR_COUNT_ARGS(...) VIR_ARG5(__VA_ARGS__, 4, 3, 2, 1) +# define VIR_ARG5(_1, _2, _3, _4, _5, ...) _5 +# define VIR_ADD_CAST_EXPAND(a, b, ...) VIR_ADD_CAST_PASTE(a, b, __VA_ARGS__) +# define VIR_ADD_CAST_PASTE(a, b, ...) a##b(__VA_ARGS__) + +/* The double cast is necessary to silence gcc warnings; any pointer + * can safely go to intptr_t and back to void *, which collapses + * arrays into pointers; while any integer can be widened to intptr_t + * then cast to void *. */ +# define VIR_ADD_CAST(a) ((void *)(intptr_t)(a)) +# define VIR_ADD_CAST2(a, b) \ + VIR_ADD_CAST(a), VIR_ADD_CAST(b) +# define VIR_ADD_CAST3(a, b, c) \ + VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c) + +# define VIR_ADD_CASTS(...) \ + VIR_ADD_CAST_EXPAND(VIR_ADD_CAST, VIR_COUNT_ARGS(__VA_ARGS__), \ + __VA_ARGS__) + +# define PROBE_EXPAND(NAME, ARGS) NAME(ARGS) # define PROBE(NAME, FMT, ...) \ VIR_DEBUG_INT("trace." __FILE__ , __func__, __LINE__, \ #NAME ": " FMT, __VA_ARGS__); \ if (LIBVIRTD_ ## NAME ## _ENABLED()) { \ - LIBVIRTD_ ## NAME(__VA_ARGS__); \ + PROBE_EXPAND(LIBVIRTD_ ## NAME, \ + VIR_ADD_CASTS(__VA_ARGS__)); \ } # else # define PROBE(NAME, FMT, ...) \ -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Tue, Aug 16, 2011 at 08:26:02AM -0600, Eric Blake wrote:
On 08/16/2011 03:57 AM, Daniel Veillard wrote:
I just tred compiling 0.9.4 and git head on Centos 6.0, for the possible libvirt.org server replacement and it fails in both cases with:
CC libvirtd-remote.o remote.c: In function 'remoteDispatchAuthPolkit': remote.c:2161: error: invalid initializer
What version of systemtap headers are installed?
Sorry for reporting late, I had forgotten about the issue, but let's try to fix it :-) libvirt:~ -> rpm -qil systemtap-sdt-devel-1.2-9.el6.x86_64 ... /usr/bin/dtrace /usr/include/sys/sdt.h libvirt:~ ->
This correspond to the following macro
PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s", virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident);
I'm a bit lost how the expansion could lead to this and why, unless the DTrace support in Centos 6 is slightly different than on RHEL, I will look but if someone has an idea :-)
This came up previously, and I wrote this RFC patch: https://www.redhat.com/archives/libvir-list/2011-July/msg00053.html
but no one replied whether we really want it in the tree.
commit c6793e397d0e62e2d00bfa1beb910fd09b254337 Author: Eric Blake <eblake@redhat.com> Date: Fri Jul 1 12:33:50 2011 -0600
build: work around older systemtap header
Systemtap 1.2 <sys/sdt.h> tried to expand STAP_PROBE3 into an initialization: volatile __typeof__(arg) foo = arg; but that fails if arg was declared as 'char arg[100]'. Rather than make all callers to PROBE deal with the stupidity of <sys/sdt.h>, we instead make PROBE cast away the problem. Some of this preprocessor abuse copies ideas in src/libvirt.c.
* daemon/libvirtd.h (PROBE): Add casts to all arguments, using... (VIR_ADD_CASTS, VIR_ADD_CAST, VIR_ADD_CAST2, VIR_ADD_CAST3) (VIR_ADD_CAST_EXPAND, VIR_ADD_CAST_PASTE, VIR_COUNT_ARGS) (VIR_ARG5, PROBE_EXPAND): New macros. Reported by Wen Congyang.
diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 6c9b9c3..6d6460e 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -44,11 +44,41 @@ # define LIBVIRTD_PROBES_H # include "probes.h" # endif /* LIBVIRTD_PROBES_H */ + +/* Systemtap 1.2 headers have a bug where they cannot handle a + * variable declared with array type. Work around this by casting all + * arguments. This is some gross use of the preprocessor because + * PROBE is a var-arg macro, but it is better than the alternative of + * making all callers to PROBE have to be aware of the issues. And + * hopefully, if we ever add a call to PROBE with other than 2 or 3 + * end arguments, you can figure out the pattern to extend this hack. + */ +# define VIR_COUNT_ARGS(...) VIR_ARG5(__VA_ARGS__, 4, 3, 2, 1) +# define VIR_ARG5(_1, _2, _3, _4, _5, ...) _5 +# define VIR_ADD_CAST_EXPAND(a, b, ...) VIR_ADD_CAST_PASTE(a, b, __VA_ARGS__) +# define VIR_ADD_CAST_PASTE(a, b, ...) a##b(__VA_ARGS__) + +/* The double cast is necessary to silence gcc warnings; any pointer + * can safely go to intptr_t and back to void *, which collapses + * arrays into pointers; while any integer can be widened to intptr_t + * then cast to void *. */ +# define VIR_ADD_CAST(a) ((void *)(intptr_t)(a)) +# define VIR_ADD_CAST2(a, b) \ + VIR_ADD_CAST(a), VIR_ADD_CAST(b) +# define VIR_ADD_CAST3(a, b, c) \ + VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c) + +# define VIR_ADD_CASTS(...) \ + VIR_ADD_CAST_EXPAND(VIR_ADD_CAST, VIR_COUNT_ARGS(__VA_ARGS__), \ + __VA_ARGS__) + +# define PROBE_EXPAND(NAME, ARGS) NAME(ARGS) # define PROBE(NAME, FMT, ...) \ VIR_DEBUG_INT("trace." __FILE__ , __func__, __LINE__, \ #NAME ": " FMT, __VA_ARGS__); \ if (LIBVIRTD_ ## NAME ## _ENABLED()) { \ - LIBVIRTD_ ## NAME(__VA_ARGS__); \ + PROBE_EXPAND(LIBVIRTD_ ## NAME, \ + VIR_ADD_CASTS(__VA_ARGS__)); \ } # else # define PROBE(NAME, FMT, ...) \
Yes that fixes it for me (but I didn't try to do much more than make and make check), I had to rebase the diff slightly as follow, thanks, ACK ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 08/22/2011 12:30 AM, Daniel Veillard wrote:
On Tue, Aug 16, 2011 at 08:26:02AM -0600, Eric Blake wrote:
On 08/16/2011 03:57 AM, Daniel Veillard wrote:
I just tred compiling 0.9.4 and git head on Centos 6.0, for the possible libvirt.org server replacement and it fails in both cases with:
CC libvirtd-remote.o remote.c: In function 'remoteDispatchAuthPolkit': remote.c:2161: error: invalid initializer
What version of systemtap headers are installed?
Sorry for reporting late, I had forgotten about the issue, but let's try to fix it :-)
libvirt:~ -> rpm -qil systemtap-sdt-devel-1.2-9.el6.x86_64
Yes that fixes it for me (but I didn't try to do much more than make and make check), I had to rebase the diff slightly as follow,
thanks, ACK !
I've pushed the patch, including your rebase touchups. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel Veillard
-
Eric Blake