[libvirt] [PATCH] build: force libnl1 if netcf also used libnl1
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible. By making configure a little bit smarter, we can avoid this situation. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine which library netcf linked against. * configure.ac (LIBNL): Don't probe libnl3 if netcf doesn't use it. --- Does this patch look safe enough to use? It was sufficient to let me resume self-tests on my F17 box, where I had intentionally installed libnl3-devel. configure.ac | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 47a72b9..7528894 100644 --- a/configure.ac +++ b/configure.ac @@ -2902,14 +2902,24 @@ LIBNL_LIBS="" have_libnl=no if test "$with_linux" = "yes"; then - PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ - have_libnl=yes - AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) - AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) - PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) - LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" - LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" - ], [PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ + # When linking with netcf, we must ensure that we pick the same version + # of libnl that netcf picked. Prefer libnl-3 unless we can prove + # netcf linked against libnl-1. + ncftool=`which ncftool` + case `(ldd "$ncftool") 2>&1` in + *libnl.so.1*) ;; + *) + PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ + have_libnl=yes + AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) + AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) + PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) + LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" + LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" + ], []) ;; + esac + if test "$have_libnl" = no; then + PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ have_libnl=yes AC_DEFINE_UNQUOTED([HAVE_LIBNL], [1], [whether the netlink library is available]) @@ -2920,7 +2930,7 @@ if test "$with_linux" = "yes"; then AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for macvtap support]) fi ]) - ]) + fi fi AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"]) -- 1.7.11.4
On 09/07/2012 06:42 PM, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this situation. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine which library netcf linked against.
* configure.ac (LIBNL): Don't probe libnl3 if netcf doesn't use it. ---
Does this patch look safe enough to use? It was sufficient to let me resume self-tests on my F17 box, where I had intentionally installed libnl3-devel.
After thinking about it for a bit, I only have two concerns: 1) is it possible to force either libnl1 or libnl3 from the configure commandline, and fail if the requested version isn't available? I think it's nice to have it default to the libnl version used by the installed netcf, but may not always be what's wanted. 2) ncftool isn't installed unless the package "netcf" is installed, but running libvirt only requires netcf-libs, and building only requires netcf-devel + netcf-libs. How about checking the version of libnl that libnetcf.so is linked against instead? Of course this is a bit more complex, because you can't just look in $PATH, but if there's a simple way to locate that file, it's more likely to be on the system. (If not, checking ncftool is better than no check at all.)
configure.ac | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac index 47a72b9..7528894 100644 --- a/configure.ac +++ b/configure.ac @@ -2902,14 +2902,24 @@ LIBNL_LIBS="" have_libnl=no
if test "$with_linux" = "yes"; then - PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ - have_libnl=yes - AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) - AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) - PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) - LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" - LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" - ], [PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ + # When linking with netcf, we must ensure that we pick the same version + # of libnl that netcf picked. Prefer libnl-3 unless we can prove + # netcf linked against libnl-1. + ncftool=`which ncftool` + case `(ldd "$ncftool") 2>&1` in + *libnl.so.1*) ;; + *) + PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ + have_libnl=yes + AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) + AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) + PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) + LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" + LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" + ], []) ;; + esac + if test "$have_libnl" = no; then + PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ have_libnl=yes AC_DEFINE_UNQUOTED([HAVE_LIBNL], [1], [whether the netlink library is available]) @@ -2920,7 +2930,7 @@ if test "$with_linux" = "yes"; then AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for macvtap support]) fi ]) - ]) + fi fi AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"])
On 09/08/2012 11:19 AM, Laine Stump wrote:
On 09/07/2012 06:42 PM, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this situation. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine which library netcf linked against.
* configure.ac (LIBNL): Don't probe libnl3 if netcf doesn't use it. ---
Does this patch look safe enough to use? It was sufficient to let me resume self-tests on my F17 box, where I had intentionally installed libnl3-devel.
After thinking about it for a bit, I only have two concerns:
1) is it possible to force either libnl1 or libnl3 from the configure commandline, and fail if the requested version isn't available? I think it's nice to have it default to the libnl version used by the installed netcf, but may not always be what's wanted.
Forcing libnl1 is definitely possible (by priming the cache for the *_cv_* variables set while doing the probe); I'm not yet sure if it is possible to force libnl3. What I may do is a v2 patch, where if $LIBNL or the appropriate *_cv_* variables are set, we skip out on the ldd probe; leaving the ldd probe only for the case of a default build. After all, it is the default build where people don't know what variables to override in the first place where it makes the most sense; but if a user has already requested $LIBNL, then they know which library they are using. I'll look into it a bit more.
2) ncftool isn't installed unless the package "netcf" is installed, but running libvirt only requires netcf-libs, and building only requires netcf-devel + netcf-libs. How about checking the version of libnl that libnetcf.so is linked against instead? Of course this is a bit more complex, because you can't just look in $PATH, but if there's a simple way to locate that file, it's more likely to be on the system. (If not, checking ncftool is better than no check at all.)
You're right there, as well. Maybe it is sufficient to just do: for dir in /usr/lib /usr/lib64; do if test -f $dir/libnetcf.so; then ldd $dir/libnetcf.so fi done when computing the ldd output. And remember, the ldd output is _only_ used to skip the libnl3 probe; if we fail to find libnetcf.so, we merely end up probing both libnl libraries, and may pick the wrong one; but that goes back to question 1, where as long as you are able to supply explicit configure arguments to override the defaults, then we are safe. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/10/2012 11:50 AM, Eric Blake wrote:
On 09/07/2012 06:42 PM, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this situation. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine which library netcf linked against.
* configure.ac (LIBNL): Don't probe libnl3 if netcf doesn't use it. ---
Does this patch look safe enough to use? It was sufficient to let me resume self-tests on my F17 box, where I had intentionally installed libnl3-devel. After thinking about it for a bit, I only have two concerns:
1) is it possible to force either libnl1 or libnl3 from the configure commandline, and fail if the requested version isn't available? I think it's nice to have it default to the libnl version used by the installed netcf, but may not always be what's wanted. Forcing libnl1 is definitely possible (by priming the cache for the *_cv_* variables set while doing the probe); I'm not yet sure if it is
On 09/08/2012 11:19 AM, Laine Stump wrote: possible to force libnl3. What I may do is a v2 patch, where if $LIBNL or the appropriate *_cv_* variables are set, we skip out on the ldd probe; leaving the ldd probe only for the case of a default build. After all, it is the default build where people don't know what variables to override in the first place where it makes the most sense; but if a user has already requested $LIBNL, then they know which library they are using. I'll look into it a bit more.
2) ncftool isn't installed unless the package "netcf" is installed, but running libvirt only requires netcf-libs, and building only requires netcf-devel + netcf-libs. How about checking the version of libnl that libnetcf.so is linked against instead? Of course this is a bit more complex, because you can't just look in $PATH, but if there's a simple way to locate that file, it's more likely to be on the system. (If not, checking ncftool is better than no check at all.) You're right there, as well. Maybe it is sufficient to just do:
for dir in /usr/lib /usr/lib64; do if test -f $dir/libnetcf.so; then ldd $dir/libnetcf.so fi done
when computing the ldd output. And remember, the ldd output is _only_ used to skip the libnl3 probe; if we fail to find libnetcf.so, we merely end up probing both libnl libraries, and may pick the wrong one; but that goes back to question 1, where as long as you are able to supply explicit configure arguments to override the defaults, then we are safe.
Okay. Regardless of the outcome, as long as those two issues are considered, then I'll be happy to ACK whatever you subsequently deem as most appropriate.
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible. By making configure a little bit smarter, we can avoid this situation - we merely skip the probe of libnl-3 if we can prove that netcf is still using libnl-1. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine things. Defaults being what they are, someone will invariably complain that our smarts were wrong. Never fear - in that case, just run ./configure LIBNL_CFLAGS=..., where the fact that you set LIBNL_CFLAGS (even to the empty string) will go back to probing for libnl-3, regardless of netcf's choice. * configure.ac (LIBNL): Don't probe libnl3 if netcf doesn't use it. --- v2: check for libnetcf.so, in usual suspect locations; and document a way to override things if the guess is wrong. configure.ac | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 47a72b9..6a96bf3 100644 --- a/configure.ac +++ b/configure.ac @@ -2902,14 +2902,30 @@ LIBNL_LIBS="" have_libnl=no if test "$with_linux" = "yes"; then - PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ - have_libnl=yes - AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) - AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) - PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) - LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" - LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" - ], [PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ + # When linking with netcf, we must ensure that we pick the same version + # of libnl that netcf picked. Prefer libnl-3 unless we can prove + # netcf linked against libnl-1, or unless the user set LIBNL_CFLAGS. + libnl_ldd= + for dir in /usr/lib64 /usr/lib; do + if test -f $dir/libnetcf.so; then + libnl_ldd=`(ldd $dir/libnetcf.so) 2>&1` + break + fi + done + case $libnl_ldd:${LIBNL_CFLAGS+set} in + *libnl.so.1*:) ;; + *) + PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ + have_libnl=yes + AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) + AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) + PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) + LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" + LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" + ], []) ;; + esac + if test "$have_libnl" = no; then + PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ have_libnl=yes AC_DEFINE_UNQUOTED([HAVE_LIBNL], [1], [whether the netlink library is available]) @@ -2920,7 +2936,7 @@ if test "$with_linux" = "yes"; then AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for macvtap support]) fi ]) - ]) + fi fi AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"]) -- 1.7.11.4
On 09/10/2012 06:14 PM, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this situation - we merely skip the probe of libnl-3 if we can prove that netcf is still using libnl-1. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine things.
Defaults being what they are, someone will invariably complain that our smarts were wrong. Never fear - in that case, just run ./configure LIBNL_CFLAGS=..., where the fact that you set LIBNL_CFLAGS (even to the empty string) will go back to probing for libnl-3, regardless of netcf's choice.
This method of forcing libnl-3 is a bit obscure, but it will be used infrequently enough (possibly never? :-P) that I'm okay doing it that way, as long as you add a comment to configure.ac briefly explaining it (as you've done in the commit message). ACK with that added.
* configure.ac (LIBNL): Don't probe libnl3 if netcf doesn't use it. ---
v2: check for libnetcf.so, in usual suspect locations; and document a way to override things if the guess is wrong.
configure.ac | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac index 47a72b9..6a96bf3 100644 --- a/configure.ac +++ b/configure.ac @@ -2902,14 +2902,30 @@ LIBNL_LIBS="" have_libnl=no
if test "$with_linux" = "yes"; then - PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ - have_libnl=yes - AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) - AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) - PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) - LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" - LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" - ], [PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ + # When linking with netcf, we must ensure that we pick the same version + # of libnl that netcf picked. Prefer libnl-3 unless we can prove + # netcf linked against libnl-1, or unless the user set LIBNL_CFLAGS. + libnl_ldd= + for dir in /usr/lib64 /usr/lib; do + if test -f $dir/libnetcf.so; then + libnl_ldd=`(ldd $dir/libnetcf.so) 2>&1` + break + fi + done + case $libnl_ldd:${LIBNL_CFLAGS+set} in + *libnl.so.1*:) ;; + *) + PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ + have_libnl=yes + AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) + AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) + PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) + LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" + LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" + ], []) ;; + esac + if test "$have_libnl" = no; then + PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ have_libnl=yes AC_DEFINE_UNQUOTED([HAVE_LIBNL], [1], [whether the netlink library is available]) @@ -2920,7 +2936,7 @@ if test "$with_linux" = "yes"; then AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for macvtap support]) fi ]) - ]) + fi fi AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"])
On 09/12/2012 01:14 PM, Laine Stump wrote:
On 09/10/2012 06:14 PM, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this situation - we merely skip the probe of libnl-3 if we can prove that netcf is still using libnl-1. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine things.
Defaults being what they are, someone will invariably complain that our smarts were wrong. Never fear - in that case, just run ./configure LIBNL_CFLAGS=..., where the fact that you set LIBNL_CFLAGS (even to the empty string) will go back to probing for libnl-3, regardless of netcf's choice.
This method of forcing libnl-3 is a bit obscure, but it will be used infrequently enough (possibly never? :-P) that I'm okay doing it that way, as long as you add a comment to configure.ac briefly explaining it (as you've done in the commit message). ACK with that added.
Done and pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Wed, Sep 12, 2012 at 03:40:15PM -0600, Eric Blake wrote:
On 09/12/2012 01:14 PM, Laine Stump wrote:
On 09/10/2012 06:14 PM, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this situation - we merely skip the probe of libnl-3 if we can prove that netcf is still using libnl-1. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine things.
Defaults being what they are, someone will invariably complain that our smarts were wrong. Never fear - in that case, just run ./configure LIBNL_CFLAGS=..., where the fact that you set LIBNL_CFLAGS (even to the empty string) will go back to probing for libnl-3, regardless of netcf's choice.
This method of forcing libnl-3 is a bit obscure, but it will be used infrequently enough (possibly never? :-P) that I'm okay doing it that way, as long as you add a comment to configure.ac briefly explaining it (as you've done in the commit message). ACK with that added.
Done and pushed.
Since this went in ./configure fails on Debian with a netcf linked against libnl1: configure:66379: $PKG_CONFIG --exists --print-errors "libnl-3.0" Package libnl-3.0 was not found in the pkg-config search path. Perhaps you should add the directory containing `libnl-3.0.pc' to the PKG_CONFIG_PATH environment variable No package 'libnl-3.0' found configure:66382: $? = 1 configure:66396: $PKG_CONFIG --exists --print-errors "libnl-3.0" Package libnl-3.0 was not found in the pkg-config search path. Perhaps you should add the directory containing `libnl-3.0.pc' to the PKG_CONFIG_PATH environment variable No package 'libnl-3.0' found configure:66399: $? = 1 configure:66413: result: no No package 'libnl-3.0' found configure:66429: error: Package requirements (libnl-3.0) were not met: Full log is at: http://honk.sigxcpu.org:8001/job/libvirt-build/60/console I didn't have a chance to have a closer look yet. Cheers, -- Guido
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 2012年09月13日 15:15, Guido Günther wrote:
On Wed, Sep 12, 2012 at 03:40:15PM -0600, Eric Blake wrote:
On 09/12/2012 01:14 PM, Laine Stump wrote:
On 09/10/2012 06:14 PM, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this situation - we merely skip the probe of libnl-3 if we can prove that netcf is still using libnl-1. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine things.
Defaults being what they are, someone will invariably complain that our smarts were wrong. Never fear - in that case, just run ./configure LIBNL_CFLAGS=..., where the fact that you set LIBNL_CFLAGS (even to the empty string) will go back to probing for libnl-3, regardless of netcf's choice.
This method of forcing libnl-3 is a bit obscure, but it will be used infrequently enough (possibly never? :-P) that I'm okay doing it that way, as long as you add a comment to configure.ac briefly explaining it (as you've done in the commit message). ACK with that added.
Done and pushed.
Since this went in ./configure fails on Debian with a netcf linked against libnl1:
configure:66379: $PKG_CONFIG --exists --print-errors "libnl-3.0" Package libnl-3.0 was not found in the pkg-config search path. Perhaps you should add the directory containing `libnl-3.0.pc' to the PKG_CONFIG_PATH environment variable No package 'libnl-3.0' found configure:66382: $? = 1 configure:66396: $PKG_CONFIG --exists --print-errors "libnl-3.0" Package libnl-3.0 was not found in the pkg-config search path. Perhaps you should add the directory containing `libnl-3.0.pc' to the PKG_CONFIG_PATH environment variable No package 'libnl-3.0' found configure:66399: $? = 1 configure:66413: result: no No package 'libnl-3.0' found configure:66429: error: Package requirements (libnl-3.0) were not met:
Full log is at:
http://honk.sigxcpu.org:8001/job/libvirt-build/60/console
I didn't have a chance to have a closer look yet. Cheers,
Should be fixed by commit 51b708c63ee. Regards, Osier
On Thu, Sep 13, 2012 at 03:19:40PM +0800, Osier Yang wrote:
On 2012年09月13日 15:15, Guido Günther wrote:
On Wed, Sep 12, 2012 at 03:40:15PM -0600, Eric Blake wrote:
On 09/12/2012 01:14 PM, Laine Stump wrote:
On 09/10/2012 06:14 PM, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this situation - we merely skip the probe of libnl-3 if we can prove that netcf is still using libnl-1. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine things.
Defaults being what they are, someone will invariably complain that our smarts were wrong. Never fear - in that case, just run ./configure LIBNL_CFLAGS=..., where the fact that you set LIBNL_CFLAGS (even to the empty string) will go back to probing for libnl-3, regardless of netcf's choice.
This method of forcing libnl-3 is a bit obscure, but it will be used infrequently enough (possibly never? :-P) that I'm okay doing it that way, as long as you add a comment to configure.ac briefly explaining it (as you've done in the commit message). ACK with that added.
Done and pushed.
Since this went in ./configure fails on Debian with a netcf linked against libnl1:
configure:66379: $PKG_CONFIG --exists --print-errors "libnl-3.0" Package libnl-3.0 was not found in the pkg-config search path. Perhaps you should add the directory containing `libnl-3.0.pc' to the PKG_CONFIG_PATH environment variable No package 'libnl-3.0' found configure:66382: $? = 1 configure:66396: $PKG_CONFIG --exists --print-errors "libnl-3.0" Package libnl-3.0 was not found in the pkg-config search path. Perhaps you should add the directory containing `libnl-3.0.pc' to the PKG_CONFIG_PATH environment variable No package 'libnl-3.0' found configure:66399: $? = 1 configure:66413: result: no No package 'libnl-3.0' found configure:66429: error: Package requirements (libnl-3.0) were not met:
Full log is at:
http://honk.sigxcpu.org:8001/job/libvirt-build/60/console
I didn't have a chance to have a closer look yet. Cheers,
Should be fixed by commit 51b708c63ee. Unfortunately it still fails: http://honk.sigxcpu.org:8001/job/libvirt-build/62/console
Cheers, -- Guido
On 09/13/2012 01:19 AM, Osier Yang wrote:
configure:66413: result: no No package 'libnl-3.0' found configure:66429: error: Package requirements (libnl-3.0) were not met:
Full log is at:
http://honk.sigxcpu.org:8001/job/libvirt-build/60/console
I didn't have a chance to have a closer look yet. Cheers,
Should be fixed by commit 51b708c63ee.
Actually, commit 51b708c63 needs to be reverted. It breaks the case when LIBNL_CFLAGS is explicitly set. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/13/2012 01:15 AM, Guido Günther wrote:
Since this went in ./configure fails on Debian with a netcf linked against libnl1:
Where does libnl.so live on your platform? Is it just a matter of expanding the for loop to find the library to run ldd on?
configure:66413: result: no No package 'libnl-3.0' found configure:66429: error: Package requirements (libnl-3.0) were not met:
Ah, I think I know how to fix this one. PKG_CHECK_MODULES defaults to a fatal error unless we supply a non-empty fourth argument. Does this patch do it? diff --git i/configure.ac w/configure.ac index 690de2a..fd89bfc 100644 --- i/configure.ac +++ w/configure.ac @@ -2924,7 +2924,7 @@ if test "$with_linux" = "yes"; then PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" - ], []) ;; + ], [:]) ;; esac if test "$have_libnl" = no; then PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
to fix libnl detection --- Thanks Eric! On Thu, Sep 13, 2012 at 06:18:39AM -0600, Eric Blake wrote:
On 09/13/2012 01:15 AM, Guido Günther wrote:
Since this went in ./configure fails on Debian with a netcf linked against libnl1:
Where does libnl.so live on your platform? Is it just a matter of expanding the for loop to find the library to run ldd on?
configure:66413: result: no No package 'libnl-3.0' found configure:66429: error: Package requirements (libnl-3.0) were not met:
Ah, I think I know how to fix this one. PKG_CHECK_MODULES defaults to a fatal error unless we supply a non-empty fourth argument. Does this patch do it?
diff --git i/configure.ac w/configure.ac index 690de2a..fd89bfc 100644 --- i/configure.ac +++ w/configure.ac @@ -2924,7 +2924,7 @@ if test "$with_linux" = "yes"; then PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" - ], []) ;; + ], [:]) ;; esac if test "$have_libnl" = no; then PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [
I checked for libnetcf.so which is in /usr/lib/ but not for libnl1 which is already in a multiarch directory (`/usr/lib/i386-linux-gnu/libnl.so). So this fixes it: configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index b5666a5..dcbe14f 100644 --- a/configure.ac +++ b/configure.ac @@ -2908,7 +2908,7 @@ if test "$with_linux" = "yes"; then # (Setting LIBNL_CFLAGS is already used by PKG_CHECK_MODULES to # override any probing, so if it set, you know which libnl is in use.) libnl_ldd= - for dir in /usr/lib64 /usr/lib; do + for dir in /usr/lib64 /usr/lib /usr/lib/*/libnl.so; do if test -f $dir/libnetcf.so; then libnl_ldd=`(ldd $dir/libnetcf.so) 2>&1` break -- 1.7.10.4
On Thu, Sep 13, 2012 at 02:35:03PM +0200, =?UTF-8?q?Guido=20G=C3=BCnther?= wrote:
I checked for libnetcf.so which is in /usr/lib/ but not for libnl1 which is already in a multiarch directory (`/usr/lib/i386-linux-gnu/libnl.so). So this fixes it:
configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac index b5666a5..dcbe14f 100644 --- a/configure.ac +++ b/configure.ac @@ -2908,7 +2908,7 @@ if test "$with_linux" = "yes"; then # (Setting LIBNL_CFLAGS is already used by PKG_CHECK_MODULES to # override any probing, so if it set, you know which libnl is in use.) libnl_ldd= - for dir in /usr/lib64 /usr/lib; do + for dir in /usr/lib64 /usr/lib /usr/lib/*/libnl.so; do
In fact, what I send in is crap since I need to put a directory here not the library itself. It turned out that a "git clean -dfx" fixed the problem too - although I don't know why. Nevertheless the attched patch would make things more robust since Debian is moving to multiarch [1] and more and more libs move to /usr/lib/<multiarch-specifier> so we'd be safe in the future. O.k. to apply the attached version? Cheers, -- Guido
if test -f $dir/libnetcf.so; then libnl_ldd=`(ldd $dir/libnetcf.so) 2>&1` break -- 1.7.10.4
On 09/13/2012 09:36 AM, Guido Günther wrote:
On Thu, Sep 13, 2012 at 02:35:03PM +0200, =?UTF-8?q?Guido=20G=C3=BCnther?= wrote:
I checked for libnetcf.so which is in /usr/lib/ but not for libnl1 which is already in a multiarch directory (`/usr/lib/i386-linux-gnu/libnl.so). So this fixes it:
+ for dir in /usr/lib64 /usr/lib /usr/lib/*/libnl.so; do
In fact, what I send in is crap since I need to put a directory here not the library itself.
Yeah, I was about to point that out, too, until I saw your next email. At any rate, it looks like you are acking my patch, and I am now acking yours :)
It turned out that a "git clean -dfx" fixed the problem too - although I don't know why.
I do - because either of our patches in isolation is sufficient to avoid aborting configure; but my patch in isolation still costs you the time to probe libnl-3. Both patches are useful.
Nevertheless the attched patch would make things more robust since Debian is moving to multiarch [1] and more and more libs move to /usr/lib/<multiarch-specifier> so we'd be safe in the future. O.k. to apply the attached version?
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Sep 13, 2012 at 10:46:30AM -0600, Eric Blake wrote:
On 09/13/2012 09:36 AM, Guido Günther wrote:
On Thu, Sep 13, 2012 at 02:35:03PM +0200, =?UTF-8?q?Guido=20G=C3=BCnther?= wrote:
I checked for libnetcf.so which is in /usr/lib/ but not for libnl1 which is already in a multiarch directory (`/usr/lib/i386-linux-gnu/libnl.so). So this fixes it:
+ for dir in /usr/lib64 /usr/lib /usr/lib/*/libnl.so; do
In fact, what I send in is crap since I need to put a directory here not the library itself.
Yeah, I was about to point that out, too, until I saw your next email.
At any rate, it looks like you are acking my patch, and I am now acking yours :)
Yes. ACK.
It turned out that a "git clean -dfx" fixed the problem too - although I don't know why.
I do - because either of our patches in isolation is sufficient to avoid aborting configure; but my patch in isolation still costs you the time to probe libnl-3. Both patches are useful.
Nevertheless the attched patch would make things more robust since Debian is moving to multiarch [1] and more and more libs move to /usr/lib/<multiarch-specifier> so we'd be safe in the future. O.k. to apply the attached version?
ACK.
Pushed. Thanks. -- Guido
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2012年09月11日 06:14, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this situation - we merely skip the probe of libnl-3 if we can prove that netcf is still using libnl-1. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine things.
Defaults being what they are, someone will invariably complain that our smarts were wrong. Never fear - in that case, just run ./configure LIBNL_CFLAGS=..., where the fact that you set LIBNL_CFLAGS (even to the empty string) will go back to probing for libnl-3, regardless of netcf's choice.
* configure.ac (LIBNL): Don't probe libnl3 if netcf doesn't use it. ---
v2: check for libnetcf.so, in usual suspect locations; and document a way to override things if the guess is wrong.
configure.ac | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac index 47a72b9..6a96bf3 100644 --- a/configure.ac +++ b/configure.ac @@ -2902,14 +2902,30 @@ LIBNL_LIBS="" have_libnl=no
if test "$with_linux" = "yes"; then - PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ - have_libnl=yes - AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) - AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) - PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) - LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" - LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" - ], [PKG_CHECK_MODULES([LIBNL], [libnl-1>= $LIBNL_REQUIRED], [ + # When linking with netcf, we must ensure that we pick the same version + # of libnl that netcf picked. Prefer libnl-3 unless we can prove + # netcf linked against libnl-1, or unless the user set LIBNL_CFLAGS. + libnl_ldd= + for dir in /usr/lib64 /usr/lib; do + if test -f $dir/libnetcf.so; then + libnl_ldd=`(ldd $dir/libnetcf.so) 2>&1` + break + fi + done + case $libnl_ldd:${LIBNL_CFLAGS+set} in + *libnl.so.1*:) ;;
The typo ':', as the end of the pattern, causes build failure.
On 09/12/2012 10:46 PM, Osier Yang wrote:
On 2012年09月11日 06:14, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
+ case $libnl_ldd:${LIBNL_CFLAGS+set} in + *libnl.so.1*:) ;;
The typo ':', as the end of the pattern, causes build failure.
That is NOT a typo. The case statement is looking at: contents of $libnl_ldd (which might be empty), followed by a literal :, followed by the contenst of ${LIBNL_CFLAGS+set} (which will either be empty or the literal string 'set'). We insist that $libnl_ldd contain libnl.so.1 and that LIBNL_CFLAGS not be set before we hit this statement. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Hey, On Thu, Sep 13, 2012 at 06:14:05AM -0600, Eric Blake wrote:
On 09/12/2012 10:46 PM, Osier Yang wrote:
On 2012年09月11日 06:14, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
+ case $libnl_ldd:${LIBNL_CFLAGS+set} in + *libnl.so.1*:) ;;
The typo ':', as the end of the pattern, causes build failure.
That is NOT a typo. The case statement is looking at: contents of $libnl_ldd (which might be empty), followed by a literal :, followed by the contenst of ${LIBNL_CFLAGS+set} (which will either be empty or the literal string 'set'). We insist that $libnl_ldd contain libnl.so.1 and that LIBNL_CFLAGS not be set before we hit this statement.
Unfortunately this bit of magic is currently not working as right above this block LIBNL_CFLAGS is unconditionally set: dnl netlink library LIBNL_ROUTE3_CFLAGS="" LIBNL_ROUTE3_LIBS="" LIBNL_CFLAGS="" LIBNL_LIBS="" have_libnl=no if test "$with_linux" = "yes"; then # When linking with netcf, we must ensure that we pick the same version # of libnl that netcf picked. Prefer libnl-3 unless we can prove # netcf linked against libnl-1, or unless the user set LIBNL_CFLAGS. # (Setting LIBNL_CFLAGS is already used by PKG_CHECK_MODULES to # override any probing, so if it set, you know which libnl is in use.) This causes libnl3 to be used on my f17 system where libnetcf is linked with libnl1. If I'm not mistaken, the 4 lines setting LIBNL_* to "" can be safely removed. I've checked that after this change my f17 box picks libnl1. Christophe
Pushed under build-breaker rule. --- configure.ac | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index 690de2a..b5666a5 100644 --- a/configure.ac +++ b/configure.ac @@ -2915,7 +2915,7 @@ if test "$with_linux" = "yes"; then fi done case $libnl_ldd:${LIBNL_CFLAGS+set} in - *libnl.so.1*:) ;; + *libnl.so.1*) ;; *) PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ have_libnl=yes -- 1.7.7.3
On 09/12/2012 10:47 PM, Osier Yang wrote:
Pushed under build-breaker rule.
What was the break? This is the wrong fix, and should be reverted so that we can apply the correct fix.
--- configure.ac | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/configure.ac b/configure.ac index 690de2a..b5666a5 100644 --- a/configure.ac +++ b/configure.ac @@ -2915,7 +2915,7 @@ if test "$with_linux" = "yes"; then fi done case $libnl_ldd:${LIBNL_CFLAGS+set} in - *libnl.so.1*:) ;; + *libnl.so.1*) ;; *) PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ have_libnl=yes
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2012年09月13日 20:14, Eric Blake wrote:
On 09/12/2012 10:47 PM, Osier Yang wrote:
Pushed under build-breaker rule.
What was the break? This is the wrong fix, and should be reverted so that we can apply the correct fix.
same error as Guido. My machine is x86_64 + fc16, which netcf is linked to libnl.so.1.
--- configure.ac | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/configure.ac b/configure.ac index 690de2a..b5666a5 100644 --- a/configure.ac +++ b/configure.ac @@ -2915,7 +2915,7 @@ if test "$with_linux" = "yes"; then fi done case $libnl_ldd:${LIBNL_CFLAGS+set} in - *libnl.so.1*:) ;; + *libnl.so.1*) ;; *) PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ have_libnl=yes
On 09/13/2012 08:53 AM, Osier Yang wrote:
On 2012年09月13日 20:14, Eric Blake wrote:
On 09/12/2012 10:47 PM, Osier Yang wrote:
Pushed under build-breaker rule.
What was the break? This is the wrong fix, and should be reverted so that we can apply the correct fix.
same error as Guido. My machine is x86_64 + fc16, which netcf is linked to libnl.so.1.
and where no libnl-3 headers are present. That's the difference; I tested on a machine with libnl-3, to prove that the libnl-3 detection was skipped after a successful ldd run; but on your machine, the ldd run found nothing (not sure why for F16, although for Debian it is because we weren't looking in enough directories), and then went ahead and probed for libnl-3 anyways where it became fatal because I didn't supply a non-empty argument to the package check. At any rate, glad it's solved now. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/07/2012 06:42 PM, Eric Blake wrote:
Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this situation. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine which library netcf linked against.
* configure.ac (LIBNL): Don't probe libnl3 if netcf doesn't use it. ---
Does this patch look safe enough to use? It was sufficient to let me resume self-tests on my F17 box, where I had intentionally installed libnl3-devel.
The problem that this patch is attempting to fix is both real and takes a little digging to find ... as happened to me. While is might be nice to have the option of using libnl-3 on a Fedora 17 system, it should then require the correct netcf. Gene
configure.ac | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac index 47a72b9..7528894 100644 --- a/configure.ac +++ b/configure.ac @@ -2902,14 +2902,24 @@ LIBNL_LIBS="" have_libnl=no
if test "$with_linux" = "yes"; then - PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ - have_libnl=yes - AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) - AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) - PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) - LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" - LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" - ], [PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ + # When linking with netcf, we must ensure that we pick the same version + # of libnl that netcf picked. Prefer libnl-3 unless we can prove + # netcf linked against libnl-1. + ncftool=`which ncftool` + case `(ldd "$ncftool") 2>&1` in + *libnl.so.1*) ;; + *) + PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ + have_libnl=yes + AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) + AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) + PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) + LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" + LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" + ], []) ;; + esac + if test "$have_libnl" = no; then + PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ have_libnl=yes AC_DEFINE_UNQUOTED([HAVE_LIBNL], [1], [whether the netlink library is available]) @@ -2920,7 +2930,7 @@ if test "$with_linux" = "yes"; then AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for macvtap support]) fi ]) - ]) + fi fi AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"])
participants (7)
-
=?UTF-8?q?Guido=20G=C3=BCnther?= -
Christophe Fergeau -
Eric Blake -
Gene Czarcinski -
Guido Günther
-
Laine Stump -
Osier Yang