* Re: [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil
2019-01-17 17:14 [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil Thomas Huth
@ 2019-01-17 17:34 ` Philippe Mathieu-Daudé
2019-01-17 17:39 ` Eric Blake
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-01-17 17:34 UTC (permalink / raw)
To: Thomas Huth, qemu-devel; +Cc: Peter Maydell, Paolo Bonzini
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)))
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil
2019-01-17 17:14 [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil Thomas Huth
2019-01-17 17:34 ` Philippe Mathieu-Daudé
@ 2019-01-17 17:39 ` Eric Blake
2019-01-17 17:53 ` Paolo Bonzini
2019-01-17 18:07 ` Peter Maydell
3 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2019-01-17 17:39 UTC (permalink / raw)
To: Thomas Huth, qemu-devel; +Cc: Peter Maydell, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil
2019-01-17 17:14 [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil Thomas Huth
2019-01-17 17:34 ` Philippe Mathieu-Daudé
2019-01-17 17:39 ` Eric Blake
@ 2019-01-17 17:53 ` Paolo Bonzini
2019-01-17 18:07 ` Peter Maydell
3 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2019-01-17 17:53 UTC (permalink / raw)
To: Thomas Huth, qemu-devel; +Cc: Peter Maydell, Eric Blake
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil
2019-01-17 17:14 [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil Thomas Huth
` (2 preceding siblings ...)
2019-01-17 17:53 ` Paolo Bonzini
@ 2019-01-17 18:07 ` Peter Maydell
2019-01-17 18:15 ` Eric Blake
3 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2019-01-17 18:07 UTC (permalink / raw)
To: Thomas Huth; +Cc: QEMU Developers, Paolo Bonzini, Eric Blake
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] configure: Add a proper check for openpty() in libutil
2019-01-17 18:07 ` Peter Maydell
@ 2019-01-17 18:15 ` Eric Blake
0 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2019-01-17 18:15 UTC (permalink / raw)
To: Peter Maydell, Thomas Huth; +Cc: QEMU Developers, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread