[v1] add ptmx support to test-container

On Thu, Mar 20, 2025 at 12:39 PM DJ Delorie <dj@redhat.com> wrote:
>
>
>
> diff --git a/support/Makefile b/support/Makefile
> index 59a9974539..2887cc7fa9 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -325,6 +325,7 @@ tests = \
>    README-testing \
>    tst-support-namespace \
>    tst-support-open-dev-null-range \
> +  tst-support-openpty \
>    tst-support-process_state \
>    tst-support_blob_repeat \
>    tst-support_capture_subprocess \
> @@ -346,6 +347,10 @@ tests = \
>    tst-xsigstack \
>    # tests
>
> +tests-container = \
> +  tst-support-openpty-c \
> +  # tests-container
> +
>  ifeq ($(run-built-tests),yes)
>  tests-special = \
>    $(objpfx)tst-support_record_failure-2.out
> diff --git a/support/test-container.c b/support/test-container.c
> index 79d3189e2f..a641250079 100644
> --- a/support/test-container.c
> +++ b/support/test-container.c
> @@ -1151,6 +1151,9 @@ main (int argc, char **argv)
>    devmount (new_root_path, "null");
>    devmount (new_root_path, "zero");
>    devmount (new_root_path, "urandom");
> +#ifdef __linux__
> +  devmount (new_root_path, "ptmx");
> +#endif

I think the ptmx test should be done with header files in
sysdeps/generic and sysdeps/unix/sysv/linux to avoid
"#ifdef __linux__".

>    /* We're done with the "old" root, switch to the new one.  */
>    if (chroot (new_root_path) < 0)
> @@ -1217,6 +1220,14 @@ main (int argc, char **argv)
>
>    maybe_xmkdir ("/tmp", 0755);
>
> +#ifdef __linux__
> +  maybe_xmkdir ("/dev/pts", 0777);
> +  if (mount ("/dev/pts", "/dev/pts", "devpts", 0, "newinstance,ptmxmode=0666,mode=0666") < 0)
> +    FAIL_EXIT1 ("can't mount /dev/pts: %m\n");
> +  if (mount ("/dev/pts/ptmx", "/dev/ptmx", "", MS_BIND | MS_REC, NULL) < 0)
> +    FAIL_EXIT1 ("can't mount /dev/ptmx\n");
> +#endif
> +
>    if (require_pidns)
>      {
>        /* Now that we're pid 1 (effectively "root") we can mount /proc  */
> diff --git a/support/tst-support-openpty-c.c b/support/tst-support-openpty-c.c
> new file mode 100644
> index 0000000000..0a6a428fc3
> --- /dev/null
> +++ b/support/tst-support-openpty-c.c
> @@ -0,0 +1,2 @@
> +/* Same test, but in a test-container.  */
> +#include "tst-support-openpty.c"
> diff --git a/support/tst-support-openpty.c b/support/tst-support-openpty.c
> new file mode 100644
> index 0000000000..b0e3448fa1
> --- /dev/null
> +++ b/support/tst-support-openpty.c
> @@ -0,0 +1,49 @@
> +/* Basic test for support_openpty support in test-container.
> +   Copyright (C) 2025 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <termios.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +
> +#include <support/tty.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +
> +/* Note: the purpose of this test isn't to test if ptys function
> +   correctly, but only to verify that test-container's support for
> +   them is correct.  The many checks in support_openpty.c are
> +   sufficient for this.  */
> +
> +int
> +do_test(void)
> +{
> +  int outer, inner;
> +  char *name;
> +  struct termios term;
> +  struct winsize win;
> +
> +  cfmakeraw (&term);
> +  win.ws_row = 24;
> +  win.ws_col = 80;
> +
> +  support_openpty (&outer, &inner, &name, &term, &win);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
>
  

* H. J. Lu:

> I think the ptmx test should be done with header files in
> sysdeps/generic and sysdeps/unix/sysv/linux to avoid
> "#ifdef __linux__".

We can move the whole file to sysdeps/unix/sysv/linux and replace it
with a stub that just calls FAIL_UNSUPPORTED.

Thanks,
Florian
  

"H.J. Lu" <hjl.tools@gmail.com> writes:
>> +#ifdef __linux__
>> +  devmount (new_root_path, "ptmx");
>> +#endif
>
> I think the ptmx test should be done with header files in
> sysdeps/generic and sysdeps/unix/sysv/linux to avoid
> "#ifdef __linux__".

It's not the only __linux__ test in that file, though.

test-container is a neccessarily os-aware program, I don't see making it
portable across the two kernels we normally support (Linux and HURD) to
be information other programs would benefit from.  Who would benefit
from a "#define TEST_CONTAINER_NEEDS_DEV_PTMX" ?  And keep in mind it
would need two macros if we macroized the commands; one for the
pre-namespace commands and one for the post-namespace ones.

As for existing portability macros, posix_openpt() is generally
implemented by separate files with their own kernel-specific paths
within, so there's nothing existing to check for.
  

On Thu, Mar 20, 2025 at 4:59 PM DJ Delorie <dj@redhat.com> wrote:
>
> "H.J. Lu" <hjl.tools@gmail.com> writes:
> >> +#ifdef __linux__
> >> +  devmount (new_root_path, "ptmx");
> >> +#endif
> >
> > I think the ptmx test should be done with header files in
> > sysdeps/generic and sysdeps/unix/sysv/linux to avoid
> > "#ifdef __linux__".
>
> It's not the only __linux__ test in that file, though.
>
> test-container is a neccessarily os-aware program, I don't see making it
> portable across the two kernels we normally support (Linux and HURD) to
> be information other programs would benefit from.  Who would benefit
> from a "#define TEST_CONTAINER_NEEDS_DEV_PTMX" ?  And keep in mind it

No need for macros.  We can add tst-ptmx.h to define inline functions,
devmount_ptmx and mount_ptmx.  Then we just do

...
#include <tst-ptmx.h>
...
  devmount_ptmx (new_root_path);
...
  mount_ptmx ();
...

> would need two macros if we macroized the commands; one for the
> pre-namespace commands and one for the post-namespace ones.
>
> As for existing portability macros, posix_openpt() is generally
> implemented by separate files with their own kernel-specific paths
> within, so there's nothing existing to check for.
>
  

"H.J. Lu" <hjl.tools@gmail.com> writes:
> No need for macros.  We can add tst-ptmx.h to define inline functions,
> devmount_ptmx and mount_ptmx.  Then we just do

Four macros, four inline functions, same thing.  I just don't see the
point.  We either have a lot of kernel-specific information on setting
up a container in test-container.c (the only place that uses it), or we
spread that information around where it's harder to find.