diff mbox series

[v2] Discover openpty(3) dynamically in configure

Message ID 20170911171639.22005-1-n54@gmx.com
State New
Headers show
Series [v2] Discover openpty(3) dynamically in configure | expand

Commit Message

Kamil Rytarowski Sept. 11, 2017, 5:16 p.m. UTC 
openpty(3) might:
 - exists in libc (OSX)
 - exists in libutil (GNU, BSD)
 - does not exist (SmartOS)

Add a function to discover this function in the ./configure script.
Add new config types: CONFIG_OPENPTY_LIBC and CONFIG_OPENPTY_LIBUTIL,
respectively defined when openpts(3) links with -lc or -lutil.

Replace the condition adding -lutil in tests (for openpty(3)) from
CONFIG_POSIX to CONFIG_OPENPTY_LIBUTIL.

Replace the fallback openpty(3) impelementation comment from Solaris
to SmartOS. Solaris is EOL'ed and it's confirmed that it does not work
on Oracle Solaris.
---
 configure              | 25 +++++++++++++++++++++++++
 tests/Makefile.include |  2 +-
 util/qemu-openpty.c    |  4 ++--
 3 files changed, 28 insertions(+), 3 deletions(-)

Comments

Kamil Rytarowski Sept. 11, 2017, 8:09 p.m. UTC | #1 
On 11.09.2017 19:16, Kamil Rytarowski wrote:
> openpty(3) might:
>  - exists in libc (OSX)
>  - exists in libutil (GNU, BSD)
>  - does not exist (SmartOS)
> 
> Add a function to discover this function in the ./configure script.
> Add new config types: CONFIG_OPENPTY_LIBC and CONFIG_OPENPTY_LIBUTIL,
> respectively defined when openpts(3) links with -lc or -lutil.
> 
> Replace the condition adding -lutil in tests (for openpty(3)) from
> CONFIG_POSIX to CONFIG_OPENPTY_LIBUTIL.
> 
> Replace the fallback openpty(3) impelementation comment from Solaris
> to SmartOS. Solaris is EOL'ed and it's confirmed that it does not work
> on Oracle Solaris.

Signed-off-by: Kamil Rytarowski <n54@gmx.com>

> ---
>  configure              | 25 +++++++++++++++++++++++++
>  tests/Makefile.include |  2 +-
>  util/qemu-openpty.c    |  4 ++--
>  3 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index fd7e3a5e81..a614adcd29 100755
> --- a/configure
> +++ b/configure
> @@ -3819,6 +3819,25 @@ EOF
>    fi
>  fi
>  
> +##########################################
> +# openpty probe
> +openpty_libc=no
> +openpty_libutil=no
> +cat > $TMPC << EOF
> +extern int openpty(int *amaster, int *aslave, char *name, void *termp, void *winp);
> +
> +int main(void)
> +{
> +    int master_fd, slave_fd;
> +    return openpty(&master_fd, &slave_fd, 0, 0, 0) == 0;
> +}
> +EOF
> +if compile_prog "" "" ; then
> +  openpty_libc=yes
> +elif compile_prog "" "-lutil" ; then
> +  openpty_libutil=yes
> +fi
> +
>  ##########################################
>  # signalfd probe
>  signalfd="no"
> @@ -5788,6 +5807,12 @@ fi
>  if test "$fdt" = "yes" ; then
>    echo "CONFIG_FDT=y" >> $config_host_mak
>  fi
> +if test "$openpty_libc" = "yes" ; then
> +  echo "CONFIG_OPENPTY_LIBC=y" >> $config_host_mak
> +fi
> +if test "$openpty_libutil" = "yes" ; then
> +  echo "CONFIG_OPENPTY_LIBUTIL=y" >> $config_host_mak
> +fi
>  if test "$signalfd" = "yes" ; then
>    echo "CONFIG_SIGNALFD=y" >> $config_host_mak
>  fi
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index fae5715e9c..e7e0bc2724 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -814,7 +814,7 @@ tests/migration/initrd-stress.img: tests/migration/stress$(EXESUF)
>  	rm $(INITRD_WORK_DIR)/init
>  	rmdir $(INITRD_WORK_DIR)
>  
> -ifeq ($(CONFIG_POSIX),y)
> +ifeq ($(CONFIG_OPENPTY_LIBUTIL),y)
>  LIBS += -lutil
>  endif
>  
> diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
> index 2e8b43bdf5..62c87e5563 100644
> --- a/util/qemu-openpty.c
> +++ b/util/qemu-openpty.c
> @@ -51,8 +51,8 @@
>  # include <termios.h>
>  #endif
>  
> -#ifdef __sun__
> -/* Once Solaris has openpty(), this is going to be removed. */
> +/* The fallback implementation is needed at least on SmartOS. */
> +#if !defined(CONFIG_OPENPTY_LIBC) && !defined(CONFIG_OPENPTY_LIBUTIL)
>  static int openpty(int *amaster, int *aslave, char *name,
>                     struct termios *termp, struct winsize *winp)
>  {
>
Peter Maydell Sept. 14, 2017, 1:52 p.m. UTC | #2 
On 11 September 2017 at 18:16, Kamil Rytarowski <n54@gmx.com> wrote:
> openpty(3) might:
>  - exists in libc (OSX)
>  - exists in libutil (GNU, BSD)
>  - does not exist (SmartOS)
>
> Add a function to discover this function in the ./configure script.
> Add new config types: CONFIG_OPENPTY_LIBC and CONFIG_OPENPTY_LIBUTIL,
> respectively defined when openpts(3) links with -lc or -lutil.
>
> Replace the condition adding -lutil in tests (for openpty(3)) from
> CONFIG_POSIX to CONFIG_OPENPTY_LIBUTIL.
>
> Replace the fallback openpty(3) impelementation comment from Solaris
> to SmartOS. Solaris is EOL'ed and it's confirmed that it does not work
> on Oracle Solaris.
> ---
>  configure              | 25 +++++++++++++++++++++++++
>  tests/Makefile.include |  2 +-
>  util/qemu-openpty.c    |  4 ++--
>  3 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index fd7e3a5e81..a614adcd29 100755
> --- a/configure
> +++ b/configure
> @@ -3819,6 +3819,25 @@ EOF
>    fi
>  fi
>
> +##########################################
> +# openpty probe
> +openpty_libc=no
> +openpty_libutil=no
> +cat > $TMPC << EOF
> +extern int openpty(int *amaster, int *aslave, char *name, void *termp, void *winp);

I think the need to provide a prototype here rather than
using the system header to define it is revealing that we
also need a configure test for
 * openpty() in <pty.h>
 * openpty() in <libutil.h>
 * openpty() in <util.h>

Something like this untested code ought to do:

cat > $TMPC << EOF
#if defined(CONFIG_OPENPTY_IN_PTY_H)
#include <pty.h>
#elif defined(CONFIG_OPENPTY_IN_LIBUTIL_H)
#include <libutil.h>
#elif defined(CONFIG_OPENPTY_IN_UTIL_H)
#include <util.h>
#endif
int main(void) { return openpty(0, 0, 0, 0, 0); }
EOF

# Different platforms put openpty() in different headers,
# and may or may not need us to link against -lutil
if compile_prog -DCONFIG_OPENPTY_IN_PTY_H ""; then
  openpty_in_pty_h=yes
elif compile_prog -D_CONFIG_OPENPTY_IN_PTY_H -lutil; then
  openpty_in_pty_h=yes
  openpty_libutil=yes
elif compile_object -DCONFIG_OPENPTY_IN_LIBUTIL_H; then
  openpty_in_libutil_h=yes
elif compile_prog -D_CONFIG_OPENPTY_IN_LIBUTIL_H -lutil; then
  openpty_in_libutil_h=yes
  openpty_libutil=yes
elif compile_object -DCONFIG_OPENPTY_IN_UTIL_H; then
  openpty_in_util_h=yes
elif compile_prog -D_CONFIG_OPENPTY_IN_UTIL_H -lutil; then
  openpty_in_util_h=yes
  openpty_libutil=yes
fi

Then in qemu-openpty.c we can do

#include <termios.h>

#if defined(CONFIG_OPENPTY_IN_PTY_H)
#include <pty.h>
#elif defined(CONFIG_OPENPTY_IN_LIBUTIL_H)
#include <libutil.h>
#elif defined(CONFIG_OPENPTY_IN_UTIL_H)
#include <util.h>
#else
/* sunos fallback code */
#endif


> +if test "$openpty_libc" = "yes" ; then
> +  echo "CONFIG_OPENPTY_LIBC=y" >> $config_host_mak
> +fi


If we use the CONFIG_OPENPTY_IN_*_H constants as above
we don't need CONFIG_OPENPTY_LIBC any more.

thanks
-- PMM
Kamil Rytarowski Oct. 28, 2017, 5:07 p.m. UTC | #3 
On 14.09.2017 15:52, Peter Maydell wrote:
> On 11 September 2017 at 18:16, Kamil Rytarowski <n54@gmx.com> wrote:
>> openpty(3) might:
>>  - exists in libc (OSX)
>>  - exists in libutil (GNU, BSD)
>>  - does not exist (SmartOS)
>>
>> Add a function to discover this function in the ./configure script.
>> Add new config types: CONFIG_OPENPTY_LIBC and CONFIG_OPENPTY_LIBUTIL,
>> respectively defined when openpts(3) links with -lc or -lutil.
>>
>> Replace the condition adding -lutil in tests (for openpty(3)) from
>> CONFIG_POSIX to CONFIG_OPENPTY_LIBUTIL.
>>
>> Replace the fallback openpty(3) impelementation comment from Solaris
>> to SmartOS. Solaris is EOL'ed and it's confirmed that it does not work
>> on Oracle Solaris.
>> ---
>>  configure              | 25 +++++++++++++++++++++++++
>>  tests/Makefile.include |  2 +-
>>  util/qemu-openpty.c    |  4 ++--
>>  3 files changed, 28 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure b/configure
>> index fd7e3a5e81..a614adcd29 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3819,6 +3819,25 @@ EOF
>>    fi
>>  fi
>>
>> +##########################################
>> +# openpty probe
>> +openpty_libc=no
>> +openpty_libutil=no
>> +cat > $TMPC << EOF
>> +extern int openpty(int *amaster, int *aslave, char *name, void *termp, void *winp);
> 
> I think the need to provide a prototype here rather than
> using the system header to define it is revealing that we
> also need a configure test for
>  * openpty() in <pty.h>
>  * openpty() in <libutil.h>
>  * openpty() in <util.h>
> 
> Something like this untested code ought to do:
> 
> cat > $TMPC << EOF
> #if defined(CONFIG_OPENPTY_IN_PTY_H)
> #include <pty.h>
> #elif defined(CONFIG_OPENPTY_IN_LIBUTIL_H)
> #include <libutil.h>
> #elif defined(CONFIG_OPENPTY_IN_UTIL_H)
> #include <util.h>
> #endif
> int main(void) { return openpty(0, 0, 0, 0, 0); }
> EOF
> 
> # Different platforms put openpty() in different headers,
> # and may or may not need us to link against -lutil
> if compile_prog -DCONFIG_OPENPTY_IN_PTY_H ""; then
>   openpty_in_pty_h=yes
> elif compile_prog -D_CONFIG_OPENPTY_IN_PTY_H -lutil; then
>   openpty_in_pty_h=yes
>   openpty_libutil=yes
> elif compile_object -DCONFIG_OPENPTY_IN_LIBUTIL_H; then
>   openpty_in_libutil_h=yes
> elif compile_prog -D_CONFIG_OPENPTY_IN_LIBUTIL_H -lutil; then
>   openpty_in_libutil_h=yes
>   openpty_libutil=yes
> elif compile_object -DCONFIG_OPENPTY_IN_UTIL_H; then
>   openpty_in_util_h=yes
> elif compile_prog -D_CONFIG_OPENPTY_IN_UTIL_H -lutil; then
>   openpty_in_util_h=yes
>   openpty_libutil=yes
> fi
> 
> Then in qemu-openpty.c we can do
> 
> #include <termios.h>
> 
> #if defined(CONFIG_OPENPTY_IN_PTY_H)
> #include <pty.h>
> #elif defined(CONFIG_OPENPTY_IN_LIBUTIL_H)
> #include <libutil.h>
> #elif defined(CONFIG_OPENPTY_IN_UTIL_H)
> #include <util.h>
> #else
> /* sunos fallback code */
> #endif
> 
> 
>> +if test "$openpty_libc" = "yes" ; then
>> +  echo "CONFIG_OPENPTY_LIBC=y" >> $config_host_mak
>> +fi
> 
> 
> If we use the CONFIG_OPENPTY_IN_*_H constants as above
> we don't need CONFIG_OPENPTY_LIBC any more.
> 
> thanks
> -- PMM
> 
Hello,

I've naively expected that once a build on a certain OS will be fixed,
it will be functional for longer period (at least few weeks). In one
month qemu has been broken on DragonFly and SmartOS so they are not just
broken in tests, but also in the basic build.

SmartOS has new compatibility problems with grep(1) invocations and tpm
emulator fatal build failure. Few old problems with conflicts with
system headers like conflicts with symbols CS and REG_SP.

There are certainly other small incompatibility problems, after the
breakage. Just a remind that SmartOS was not capable to execute tests
because of incompatible diff(1) invocations and unclear gmake(1)
misbehavior. And this was just the beginning of the problems.

DragonFlyBSD has problems with PAGE_SIZE, PAGE_MASK clash with headers
in s390x code. PAGE_MASK, PAGE_SHIFT and PAGE_SIZE in nand. No single
GTESTER invocation passes.

Because there is need to fix the SmartOS build in order to improve this
openpty(3) patch, I'm resigning from it. I also resign from helping
DragonFly port.

I will focus entirely on the bits that I registered as a maintainer.

I leave the decisions and steps about support of these platforms to
other volunteers and qemu core maintainers.

+ CC Antonio and Peter
Peter Maydell Nov. 2, 2017, 3:14 p.m. UTC | #4 
On 28 October 2017 at 18:07, Kamil Rytarowski <n54@gmx.com> wrote:
> I've naively expected that once a build on a certain OS will be fixed,
> it will be functional for longer period (at least few weeks). In one
> month qemu has been broken on DragonFly and SmartOS so they are not just
> broken in tests, but also in the basic build.

Yep; things do break if they're not tested, which is why I think
we have to have a host system being in our regular build/test
process in order to be able to call it supported.

> Because there is need to fix the SmartOS build in order to improve this
> openpty(3) patch, I'm resigning from it. I also resign from helping
> DragonFly port.
>
> I will focus entirely on the bits that I registered as a maintainer.
>
> I leave the decisions and steps about support of these platforms to
> other volunteers and qemu core maintainers.

Thanks for your efforts with the BSD porting work; we appreciate
it.

thanks
-- PMM
diff mbox series

Patch

diff --git a/configure b/configure
index fd7e3a5e81..a614adcd29 100755
--- a/configure
+++ b/configure
@@ -3819,6 +3819,25 @@  EOF
   fi
 fi
 
+##########################################
+# openpty probe
+openpty_libc=no
+openpty_libutil=no
+cat > $TMPC << EOF
+extern int openpty(int *amaster, int *aslave, char *name, void *termp, void *winp);
+
+int main(void)
+{
+    int master_fd, slave_fd;
+    return openpty(&master_fd, &slave_fd, 0, 0, 0) == 0;
+}
+EOF
+if compile_prog "" "" ; then
+  openpty_libc=yes
+elif compile_prog "" "-lutil" ; then
+  openpty_libutil=yes
+fi
+
 ##########################################
 # signalfd probe
 signalfd="no"
@@ -5788,6 +5807,12 @@  fi
 if test "$fdt" = "yes" ; then
   echo "CONFIG_FDT=y" >> $config_host_mak
 fi
+if test "$openpty_libc" = "yes" ; then
+  echo "CONFIG_OPENPTY_LIBC=y" >> $config_host_mak
+fi
+if test "$openpty_libutil" = "yes" ; then
+  echo "CONFIG_OPENPTY_LIBUTIL=y" >> $config_host_mak
+fi
 if test "$signalfd" = "yes" ; then
   echo "CONFIG_SIGNALFD=y" >> $config_host_mak
 fi
diff --git a/tests/Makefile.include b/tests/Makefile.include
index fae5715e9c..e7e0bc2724 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -814,7 +814,7 @@  tests/migration/initrd-stress.img: tests/migration/stress$(EXESUF)
 	rm $(INITRD_WORK_DIR)/init
 	rmdir $(INITRD_WORK_DIR)
 
-ifeq ($(CONFIG_POSIX),y)
+ifeq ($(CONFIG_OPENPTY_LIBUTIL),y)
 LIBS += -lutil
 endif
 
diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
index 2e8b43bdf5..62c87e5563 100644
--- a/util/qemu-openpty.c
+++ b/util/qemu-openpty.c
@@ -51,8 +51,8 @@ 
 # include <termios.h>
 #endif
 
-#ifdef __sun__
-/* Once Solaris has openpty(), this is going to be removed. */
+/* The fallback implementation is needed at least on SmartOS. */
+#if !defined(CONFIG_OPENPTY_LIBC) && !defined(CONFIG_OPENPTY_LIBUTIL)
 static int openpty(int *amaster, int *aslave, char *name,
                    struct termios *termp, struct winsize *winp)
 {