configure: Add a proper check for openpty() in libutil

[Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil

Posted by Thomas Huth 6 years, 9 months ago

On Linux (and maybe some BSDs), we require libutil for the openpty()
function. However, this library is not available on some other systems, so
we currently use a fragile if-statement in the configure script to check
whether we need the library or not. Unfortunately, we also hard-coded a
"-lutil" in the tests/Makefile.include file, so this breaks the build on
Solaris, for example (see buglink below). To fix the issue, add the "-lutil"
to "libs_tools" in the configure script instead, then this gets properly
propagated to the tests, too.
And while we're at it, also replace the fragile if-statement in the confi-
gure script with a proper link-check for the availablity of this function.

Buglink: https://bugs.launchpad.net/qemu/+bug/1777252
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure              | 12 ++++++++++--
 tests/Makefile.include |  4 ----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index 3c28bae..0f4e42a 100755
--- a/configure
+++ b/configure
@@ -4595,9 +4595,17 @@ elif compile_prog "" "$pthread_lib -lrt" ; then
   libs_qga="$libs_qga -lrt"
 fi
 
-if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
-        "$haiku" != "yes" ; then
+# Check whether we need to link libutil for openpty()
+cat > $TMPC << EOF
+extern int openpty(int *am, int *as, char *name, void *termp, void *winp);
+int main(void) { return openpty(0, 0, 0, 0, 0); }
+EOF
+
+if ! compile_prog "" "" ; then
+  if compile_prog "" "-lutil" ; then
     libs_softmmu="-lutil $libs_softmmu"
+    libs_tools="-lutil $libs_tools"
+  fi
 fi
 
 ##########################################
diff --git a/tests/Makefile.include b/tests/Makefile.include
index f403a65..8f8e3b4 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -793,10 +793,6 @@ tests/migration/initrd-stress.img: tests/migration/stress$(EXESUF)
 	rm $(INITRD_WORK_DIR)/init
 	rmdir $(INITRD_WORK_DIR)
 
-ifeq ($(CONFIG_POSIX),y)
-LIBS += -lutil
-endif
-
 # QTest rules
 
 TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS)))
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil

Posted by Philippe Mathieu-Daudé 6 years, 9 months ago

On 1/17/19 6:14 PM, Thomas Huth wrote:
> On Linux (and maybe some BSDs), we require libutil for the openpty()
> function. However, this library is not available on some other systems, so
> we currently use a fragile if-statement in the configure script to check
> whether we need the library or not. Unfortunately, we also hard-coded a
> "-lutil" in the tests/Makefile.include file, so this breaks the build on
> Solaris, for example (see buglink below). To fix the issue, add the "-lutil"
> to "libs_tools" in the configure script instead, then this gets properly
> propagated to the tests, too.
> And while we're at it, also replace the fragile if-statement in the confi-
> gure script with a proper link-check for the availablity of this function.

Clean.

> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1777252
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  configure              | 12 ++++++++++--
>  tests/Makefile.include |  4 ----
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/configure b/configure
> index 3c28bae..0f4e42a 100755
> --- a/configure
> +++ b/configure
> @@ -4595,9 +4595,17 @@ elif compile_prog "" "$pthread_lib -lrt" ; then
>    libs_qga="$libs_qga -lrt"
>  fi
>  
> -if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
> -        "$haiku" != "yes" ; then
> +# Check whether we need to link libutil for openpty()
> +cat > $TMPC << EOF
> +extern int openpty(int *am, int *as, char *name, void *termp, void *winp);
> +int main(void) { return openpty(0, 0, 0, 0, 0); }
> +EOF
> +
> +if ! compile_prog "" "" ; then
> +  if compile_prog "" "-lutil" ; then
>      libs_softmmu="-lutil $libs_softmmu"
> +    libs_tools="-lutil $libs_tools"

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +  fi
>  fi
>  
>  ##########################################
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index f403a65..8f8e3b4 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -793,10 +793,6 @@ tests/migration/initrd-stress.img: tests/migration/stress$(EXESUF)
>  	rm $(INITRD_WORK_DIR)/init
>  	rmdir $(INITRD_WORK_DIR)
>  
> -ifeq ($(CONFIG_POSIX),y)
> -LIBS += -lutil
> -endif
> -
>  # QTest rules
>  
>  TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS)))
>

Re: [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil

Posted by Eric Blake 6 years, 9 months ago

On 1/17/19 11:14 AM, Thomas Huth wrote:
> On Linux (and maybe some BSDs), we require libutil for the openpty()
> function. However, this library is not available on some other systems, so
> we currently use a fragile if-statement in the configure script to check
> whether we need the library or not. Unfortunately, we also hard-coded a
> "-lutil" in the tests/Makefile.include file, so this breaks the build on
> Solaris, for example (see buglink below). To fix the issue, add the "-lutil"
> to "libs_tools" in the configure script instead, then this gets properly
> propagated to the tests, too.
> And while we're at it, also replace the fragile if-statement in the confi-
> gure script with a proper link-check for the availablity of this function.

Interesting that you split "confi-" and "gure" across lines, I guess to
appear less ragged.

s/availablity/availability/

> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1777252
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  configure              | 12 ++++++++++--
>  tests/Makefile.include |  4 ----
>  2 files changed, 10 insertions(+), 6 deletions(-)

Feature-based testing is always better than hard-coded knowledge ;)

> -if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
> -        "$haiku" != "yes" ; then
> +# Check whether we need to link libutil for openpty()
> +cat > $TMPC << EOF
> +extern int openpty(int *am, int *as, char *name, void *termp, void *winp);
> +int main(void) { return openpty(0, 0, 0, 0, 0); }
> +EOF
> +
> +if ! compile_prog "" "" ; then
> +  if compile_prog "" "-lutil" ; then
>      libs_softmmu="-lutil $libs_softmmu"
> +    libs_tools="-lutil $libs_tools"
> +  fi
>  fi

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil

Posted by Paolo Bonzini 6 years, 9 months ago

On 17/01/19 18:14, Thomas Huth wrote:
> On Linux (and maybe some BSDs), we require libutil for the openpty()
> function. However, this library is not available on some other systems, so
> we currently use a fragile if-statement in the configure script to check
> whether we need the library or not. Unfortunately, we also hard-coded a
> "-lutil" in the tests/Makefile.include file, so this breaks the build on
> Solaris, for example (see buglink below). To fix the issue, add the "-lutil"
> to "libs_tools" in the configure script instead, then this gets properly
> propagated to the tests, too.
> And while we're at it, also replace the fragile if-statement in the confi-
> gure script with a proper link-check for the availablity of this function.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1777252
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  configure              | 12 ++++++++++--
>  tests/Makefile.include |  4 ----
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/configure b/configure
> index 3c28bae..0f4e42a 100755
> --- a/configure
> +++ b/configure
> @@ -4595,9 +4595,17 @@ elif compile_prog "" "$pthread_lib -lrt" ; then
>    libs_qga="$libs_qga -lrt"
>  fi
>  
> -if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
> -        "$haiku" != "yes" ; then
> +# Check whether we need to link libutil for openpty()
> +cat > $TMPC << EOF
> +extern int openpty(int *am, int *as, char *name, void *termp, void *winp);
> +int main(void) { return openpty(0, 0, 0, 0, 0); }
> +EOF
> +
> +if ! compile_prog "" "" ; then
> +  if compile_prog "" "-lutil" ; then
>      libs_softmmu="-lutil $libs_softmmu"
> +    libs_tools="-lutil $libs_tools"
> +  fi
>  fi
>  
>  ##########################################
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index f403a65..8f8e3b4 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -793,10 +793,6 @@ tests/migration/initrd-stress.img: tests/migration/stress$(EXESUF)
>  	rm $(INITRD_WORK_DIR)/init
>  	rmdir $(INITRD_WORK_DIR)
>  
> -ifeq ($(CONFIG_POSIX),y)
> -LIBS += -lutil
> -endif
> -
>  # QTest rules
>  
>  TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS)))
> 

Queued, thanks.

Paolo

Re: [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil

Posted by Peter Maydell 6 years, 9 months ago

On Thu, 17 Jan 2019 at 17:14, Thomas Huth <thuth@redhat.com> wrote:
>
> On Linux (and maybe some BSDs), we require libutil for the openpty()
> function. However, this library is not available on some other systems, so
> we currently use a fragile if-statement in the configure script to check
> whether we need the library or not. Unfortunately, we also hard-coded a
> "-lutil" in the tests/Makefile.include file, so this breaks the build on
> Solaris, for example (see buglink below). To fix the issue, add the "-lutil"
> to "libs_tools" in the configure script instead, then this gets properly
> propagated to the tests, too.

It also gets propagated to every tool binary we build even if
it doesn't use openpty(), but I guess that's not a big deal.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil

Posted by Eric Blake 6 years, 9 months ago

On 1/17/19 12:07 PM, Peter Maydell wrote:
> On Thu, 17 Jan 2019 at 17:14, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On Linux (and maybe some BSDs), we require libutil for the openpty()
>> function. However, this library is not available on some other systems, so
>> we currently use a fragile if-statement in the configure script to check
>> whether we need the library or not. Unfortunately, we also hard-coded a
>> "-lutil" in the tests/Makefile.include file, so this breaks the build on
>> Solaris, for example (see buglink below). To fix the issue, add the "-lutil"
>> to "libs_tools" in the configure script instead, then this gets properly
>> propagated to the tests, too.
> 
> It also gets propagated to every tool binary we build even if
> it doesn't use openpty(), but I guess that's not a big deal.

Statically linking against libraries only pulls in symbols that the
library provides but which have not been satisfied earlier in the link
line. For programs which do not use openpty(), and where no other
unsatisfied symbols remain which -lutil would satisfy in a shadowing
effect on symbols normally provided by remaining libraries on the
command line, then yeah, it shouldn't change the compiled size of those
binaries.  Dynamic linking might change the binary slightly because of
tracking that the library was linked, but again the behavior shouldn't
change.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org