[libvirt] [PATCH] Make sure sys/types.h is included after sys/sysmacros.h

In the latest glibc, major() and minor() functions are marked as deprecated (glibc commit dbab6577): CC util/libvirt_util_la-vircgroup.lo util/vircgroup.c: In function 'virCgroupGetBlockDevString': util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated: In the GNU C Library, `major' is defined by <sys/sysmacros.h>. For historical compatibility, it is currently defined by <sys/types.h> as well, but we plan to remove this soon. To use `major', include <sys/sysmacros.h> directly. If you did not intend to use a system-defined macro `major', you should #undef it after including <sys/types.h>. [-Werror=deprecated-declarations] if (virAsprintf(&ret, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 0) ^~ In file included from /usr/include/features.h:397:0, from /usr/include/bits/libc-header-start.h:33, from /usr/include/stdio.h:28, from ../gnulib/lib/stdio.h:43, from util/vircgroup.c:26: /usr/include/sys/sysmacros.h:87:1: note: declared here __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL) ^ Moreover, in the glibc commit, there's suggestion to keep ordering of including of header files as implemented here. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- This still won't solve the build issue completely as AC_HEADER_MAJOR still reports that major() is defined by sys/types.h instead of sys/sysmacros.h. But once they fix it, we are good too. Or we can use the following workaround in configure.ac: +old_CFLAGS=$CFLAGS +CFLAGS+=" -Werror " AC_HEADER_MAJOR +CFLAGS=$old_CFLAGS src/conf/domain_audit.c | 3 ++- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/util/vircgroup.c | 2 +- src/util/virutil.c | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 53a58ac..52dea02 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -24,7 +24,6 @@ #include <config.h> #include <sys/stat.h> -#include <sys/types.h> #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -32,6 +31,8 @@ # include <sys/sysmacros.h> #endif +#include <sys/types.h> + #include "domain_audit.h" #include "viraudit.h" #include "viruuid.h" diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 825b4d4..8c581df 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -27,7 +27,6 @@ #include <sys/epoll.h> #include <sys/wait.h> #include <sys/socket.h> -#include <sys/types.h> #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -35,6 +34,7 @@ # include <sys/sysmacros.h> #endif +#include <sys/types.h> #include <sys/un.h> #include <sys/personality.h> #include <unistd.h> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index da98b38..24025d1 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -28,7 +28,6 @@ #include <sched.h> #include <sys/utsname.h> #include <string.h> -#include <sys/types.h> #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -36,6 +35,7 @@ # include <sys/sysmacros.h> #endif +#include <sys/types.h> #include <sys/socket.h> #include <sys/stat.h> #include <sys/un.h> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index f2477d5..8b52816 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -34,7 +34,6 @@ #include <errno.h> #include <stdlib.h> #include <sys/stat.h> -#include <sys/types.h> #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -42,6 +41,7 @@ # include <sys/sysmacros.h> #endif +#include <sys/types.h> #include <signal.h> #include <dirent.h> #include <unistd.h> diff --git a/src/util/virutil.c b/src/util/virutil.c index 170dd59..b57a195 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -35,7 +35,6 @@ #include <errno.h> #include <poll.h> #include <sys/stat.h> -#include <sys/types.h> #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -43,6 +42,7 @@ # include <sys/sysmacros.h> #endif +#include <sys/types.h> #include <sys/ioctl.h> #include <string.h> #include <termios.h> -- 2.8.4

On Tue, Sep 06, 2016 at 03:46:59PM +0200, Michal Privoznik wrote:
In the latest glibc, major() and minor() functions are marked as deprecated (glibc commit dbab6577):
CC util/libvirt_util_la-vircgroup.lo util/vircgroup.c: In function 'virCgroupGetBlockDevString': util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated: In the GNU C Library, `major' is defined by <sys/sysmacros.h>. For historical compatibility, it is currently defined by <sys/types.h> as well, but we plan to remove this soon. To use `major', include <sys/sysmacros.h> directly. If you did not intend to use a system-defined macro `major', you should #undef it after including <sys/types.h>. [-Werror=deprecated-declarations] if (virAsprintf(&ret, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 0) ^~ In file included from /usr/include/features.h:397:0, from /usr/include/bits/libc-header-start.h:33, from /usr/include/stdio.h:28, from ../gnulib/lib/stdio.h:43, from util/vircgroup.c:26: /usr/include/sys/sysmacros.h:87:1: note: declared here __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL) ^
Moreover, in the glibc commit, there's suggestion to keep ordering of including of header files as implemented here.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
This still won't solve the build issue completely as AC_HEADER_MAJOR still reports that major() is defined by sys/types.h instead of sys/sysmacros.h. But once they fix it, we are good too. Or we can use the following workaround in configure.ac:
+old_CFLAGS=$CFLAGS +CFLAGS+=" -Werror " AC_HEADER_MAJOR +CFLAGS=$old_CFLAGS
It isn't clear that AC_HEADER_MAJOR is actually doing anything useful for us. Could we not simply remove the use of AC_HEADER_MAJOR in configure.ac ? If not, then just re-implement the functionality ourselves, so that we prioritize checking sys/sysmacros.h in configure. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 09/06/2016 08:46 AM, Michal Privoznik wrote:
In the latest glibc, major() and minor() functions are marked as deprecated (glibc commit dbab6577):
Not only that, but there's a thread on the autoconf list about how to fix AC_HEADER_MAJOR to work. I may be releasing autoconf 2.70 in the near future because of this glibc change, although there is still the issue that it will only benefit programs that are configured with new-enough autoconf with the fixed macro. Gnulib, of course, will backport any autoconf fix, and we'll at least pick it up via gnulib if we update the submodule, once everything settles upstream in autoconf, but it may be a few days before I know how best to tackle it among all the related projects.
This still won't solve the build issue completely as AC_HEADER_MAJOR still reports that major() is defined by sys/types.h instead of sys/sysmacros.h. But once they fix it, we are good too. Or we can use the following workaround in configure.ac:
+old_CFLAGS=$CFLAGS +CFLAGS+=" -Werror " AC_HEADER_MAJOR +CFLAGS=$old_CFLAGS
I don't think we need to worry about working around it in our configure.ac; once upstream autoconf and gnulib figure out what they want to do, we can inherit that.
+++ b/src/conf/domain_audit.c @@ -24,7 +24,6 @@ #include <config.h>
#include <sys/stat.h> -#include <sys/types.h>
#ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> @@ -32,6 +31,8 @@ # include <sys/sysmacros.h> #endif
+#include <sys/types.h> +
For the purposes of immediate compilation, this looks correct to me, so you have my ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, Sep 06, 2016 at 03:46:59PM +0200, Michal Privoznik wrote:
In the latest glibc, major() and minor() functions are marked as deprecated (glibc commit dbab6577):
CC util/libvirt_util_la-vircgroup.lo util/vircgroup.c: In function 'virCgroupGetBlockDevString': util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated: In the GNU C Library, `major' is defined by <sys/sysmacros.h>. For historical compatibility, it is currently defined by <sys/types.h> as well, but we plan to remove this soon. To use `major', include <sys/sysmacros.h> directly. If you did not intend to use a system-defined macro `major', you should #undef it after including <sys/types.h>. [-Werror=deprecated-declarations] if (virAsprintf(&ret, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 0) ^~ In file included from /usr/include/features.h:397:0, from /usr/include/bits/libc-header-start.h:33, from /usr/include/stdio.h:28, from ../gnulib/lib/stdio.h:43, from util/vircgroup.c:26: /usr/include/sys/sysmacros.h:87:1: note: declared here __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL) ^
Moreover, in the glibc commit, there's suggestion to keep ordering of including of header files as implemented here.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
This still won't solve the build issue completely as AC_HEADER_MAJOR still reports that major() is defined by sys/types.h instead of sys/sysmacros.h.
AFAICT, it doesn't appear to have even fixed the vircgroups.c file compile failure - our CI is still broken with the exact same error message you have above https://ci.centos.org/view/libvirt-project/job/libvirt-master-build/systems=... Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 12.09.2016 16:54, Daniel P. Berrange wrote:
On Tue, Sep 06, 2016 at 03:46:59PM +0200, Michal Privoznik wrote:
In the latest glibc, major() and minor() functions are marked as deprecated (glibc commit dbab6577):
CC util/libvirt_util_la-vircgroup.lo util/vircgroup.c: In function 'virCgroupGetBlockDevString': util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated: In the GNU C Library, `major' is defined by <sys/sysmacros.h>. For historical compatibility, it is currently defined by <sys/types.h> as well, but we plan to remove this soon. To use `major', include <sys/sysmacros.h> directly. If you did not intend to use a system-defined macro `major', you should #undef it after including <sys/types.h>. [-Werror=deprecated-declarations] if (virAsprintf(&ret, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 0) ^~ In file included from /usr/include/features.h:397:0, from /usr/include/bits/libc-header-start.h:33, from /usr/include/stdio.h:28, from ../gnulib/lib/stdio.h:43, from util/vircgroup.c:26: /usr/include/sys/sysmacros.h:87:1: note: declared here __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL) ^
Moreover, in the glibc commit, there's suggestion to keep ordering of including of header files as implemented here.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
This still won't solve the build issue completely as AC_HEADER_MAJOR still reports that major() is defined by sys/types.h instead of sys/sysmacros.h.
AFAICT, it doesn't appear to have even fixed the vircgroups.c file compile failure - our CI is still broken with the exact same error message you have above
Yes, that's because autoconf macro checking where does major() come from hasn't been fixed yet. As soon as it is fixed the problem will go away. Eric, any update on this? Michal

On Mon, Sep 12, 2016 at 05:15:53PM +0200, Michal Privoznik wrote:
On 12.09.2016 16:54, Daniel P. Berrange wrote:
On Tue, Sep 06, 2016 at 03:46:59PM +0200, Michal Privoznik wrote:
In the latest glibc, major() and minor() functions are marked as deprecated (glibc commit dbab6577):
CC util/libvirt_util_la-vircgroup.lo util/vircgroup.c: In function 'virCgroupGetBlockDevString': util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated: In the GNU C Library, `major' is defined by <sys/sysmacros.h>. For historical compatibility, it is currently defined by <sys/types.h> as well, but we plan to remove this soon. To use `major', include <sys/sysmacros.h> directly. If you did not intend to use a system-defined macro `major', you should #undef it after including <sys/types.h>. [-Werror=deprecated-declarations] if (virAsprintf(&ret, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 0) ^~ In file included from /usr/include/features.h:397:0, from /usr/include/bits/libc-header-start.h:33, from /usr/include/stdio.h:28, from ../gnulib/lib/stdio.h:43, from util/vircgroup.c:26: /usr/include/sys/sysmacros.h:87:1: note: declared here __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL) ^
Moreover, in the glibc commit, there's suggestion to keep ordering of including of header files as implemented here.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
This still won't solve the build issue completely as AC_HEADER_MAJOR still reports that major() is defined by sys/types.h instead of sys/sysmacros.h.
AFAICT, it doesn't appear to have even fixed the vircgroups.c file compile failure - our CI is still broken with the exact same error message you have above
Yes, that's because autoconf macro checking where does major() come from hasn't been fixed yet. As soon as it is fixed the problem will go away. Eric, any update on this?
Oh I misunderstood then - i thought your meant the configure problem would merely cause a warning message to appear in configure output, not that it would still cause us to have a broken build. We've have rawhide builds of libvirt broken in CI for weeks now. I think we need to fix out configure.ac script in libvirt to deal with this now. As & when autoconf/gnulib get a workaround we can revert our local fix. The priority has to be getting out CI passing again asap. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Michal Privoznik