[PATCH v4 1/3] tst: Extend cross-test-ssh.sh to specify if target date can be altered

[PATCH v4 1/3] tst: Extend cross-test-ssh.sh to specify if target date can be altered

[Lukasz Majewski]

This code adds new flag - '--allow-time-setting' to cross-test-ssh.sh
script to indicate if it is allowed to alter the date on the system
on which tests are executed. This change is supposed to be used with
test systems, which use virtual machines for testing.

The GLIBC_TEST_ALLOW_TIME_SETTING env variable is exported to the
remote environment on which the eligible test is run and brings no
functional change when it is not.

Changes for v2:
- Utilize flock to provide serialization of cross-test-ssh.sh script
  execution.
- Add entry to manual/install.texi about --allow-time-setting flag
  usage.

Changes for v3:
- The install.texi manual has been augmented to explain two distinct
  use cases for setting the time on target system.

Changes for v4:
- Add -w switch to flock (specify timeout)
- Check return value of flock - exit ssh shell script
- Allow user override of FLOCK_* variables
- Rewrite the script's information/help text
- Rewrite the install.texi manual entry for this
  script usage
---
 manual/install.texi       | 17 +++++++++++++++++
 scripts/cross-test-ssh.sh | 26 +++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/manual/install.texi b/manual/install.texi
index 419576f49c..5b32401805 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -380,6 +380,23 @@ the newly built binaries of @theglibc{}.  The source and build
 directories must be visible at the same locations on both the build
 system and @var{hostname}.
 
+It is also possible to execute tests, which require setting date on
+the target machine. Following use cases are supported:
+@itemize @bullet
+@item
+@code{GLIBC_TEST_ALLOW_TIME_SETTING} is set in the environment in
+which eligible tests are executed and have priviledges to run
+@code{clock_settime}. In this case nothing prevents those tests from
+running in parallel, so the caller shall assure that those tests
+are serialized or provide a proper wrapper script for it.
+
+@item
+The @code{cross-test-ssh.sh} script is used and one passes the
+@option{--allow-time-setting} flag. In this case both setting
+@code{GLIBC_TEST_ALLOW_TIME_SETTING} and serialization of tests
+execution are assured automatically.
+@end itemize
+
 In general, when testing @theglibc{}, @samp{test-wrapper} may be set
 to the name and arguments of any program to run newly built binaries.
 This program must preserve the arguments to the binary being run, its
diff --git a/scripts/cross-test-ssh.sh b/scripts/cross-test-ssh.sh
index 6d8fbcdfd2..e73802974d 100755
--- a/scripts/cross-test-ssh.sh
+++ b/scripts/cross-test-ssh.sh
@@ -22,7 +22,7 @@
 
 progname="$(basename $0)"
 
-usage="usage: ${progname} [--ssh SSH] HOST COMMAND ..."
+usage="usage: ${progname} [--ssh SSH] [--allow-time-setting] HOST COMMAND ..."
 help="Run a glibc test COMMAND on the remote machine HOST, via ssh,
 preserving the current working directory, and respecting quoting.
 
@@ -32,6 +32,11 @@ instead of ordinary 'ssh'.
 If the '--timeoutfactor FACTOR' flag is present, set TIMEOUTFACTOR on
 the remote machine to the specified FACTOR.
 
+If the '--allow-time-setting' flag is present, set
+GLIBC_TEST_ALLOW_TIME_SETTING on the remote machine to indicate that
+time can be safely adjusted when e.g. tests are run in a virtual
+machine.
+
 To use this to run glibc tests, invoke the tests as follows:
 
   $ make test-wrapper='ABSPATH/cross-test-ssh.sh HOST' tests
@@ -81,6 +86,10 @@ while [ $# -gt 0 ]; do
       timeoutfactor="$1"
       ;;
 
+    "--allow-time-setting")
+      settimeallowed="1"
+      ;;
+
     "--help")
       echo "$usage"
       echo "$help"
@@ -127,6 +136,21 @@ if [ "$timeoutfactor" ]; then
 ${command}"
 fi
 
+# Add command to set the info that time on target can be adjusted,
+# if required.
+# Serialize execution of this script on target to prevent from unintended
+# change of target time.
+FLOCK_PATH="${FLOCK_PATH:-/var/lock/clock_settime}"
+FLOCK_TIMEOUT="${FLOCK_TIMEOUT:-20}"
+FLOCK_FD="${FLOCK_FD:-99}"
+if [ "$settimeallowed" ]; then
+  command="exec ${FLOCK_FD}<>${FLOCK_PATH}
+flock -w ${FLOCK_TIMEOUT} ${FLOCK_FD}
+if [ $? -ne 0 ]; then exit 1; fi
+export GLIBC_TEST_ALLOW_TIME_SETTING=1
+${command}"
+fi
+
 # HOST's sshd simply concatenates its arguments with spaces and
 # passes them to some shell.  We want to force the use of /bin/sh,
 # so we need to re-quote the whole command to ensure it appears as
-- 
2.20.1

[PATCH v4 2/3] support: Provide xclock_settime test helper function

[Lukasz Majewski]

The xclock_settime is a wrapper function on the clock_settime syscall
to be used in the test code.

It checks if the GLIBC_TEST_ALLOW_TIME_SETTING env variable is defined
in the environment in which test is executed. If it is not - the test
ends as unsupported. Otherwise, the clock-settime is executed and return
value is assessed.

---
Changes for v4:
- Remove not needed #include <support/xthread.h>
---
 support/Makefile         |  1 +
 support/xclock_settime.c | 34 ++++++++++++++++++++++++++++++++++
 support/xtime.h          |  5 +++++
 3 files changed, 40 insertions(+)
 create mode 100644 support/xclock_settime.c

diff --git a/support/Makefile b/support/Makefile
index bb9889efb4..8d63fbd5da 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -90,6 +90,7 @@ libsupport-routines = \
   xchdir \
   xchroot \
   xclock_gettime \
+  xclock_settime \
   xclose \
   xchmod \
   xconnect \
diff --git a/support/xclock_settime.c b/support/xclock_settime.c
new file mode 100644
index 0000000000..183aba4876
--- /dev/null
+++ b/support/xclock_settime.c
@@ -0,0 +1,34 @@
+/* clock_settime with error checking.
+   Copyright (C) 2021 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 <stdlib.h>
+#include <support/check.h>
+#include <support/xtime.h>
+
+void
+xclock_settime (clockid_t clockid,
+                const struct timespec *ts)
+{
+  if (getenv (SETTIME_ENV_NAME) == NULL)
+    FAIL_UNSUPPORTED ("clock_settime is executed only when "\
+                      SETTIME_ENV_NAME" is set\n");
+
+  const int ret = clock_settime (clockid, ts);
+  if (ret < 0)
+    FAIL_EXIT1 ("clock_settime (%d): %m", clockid);
+}
diff --git a/support/xtime.h b/support/xtime.h
index 2482837dee..b4ac3b59e2 100644
--- a/support/xtime.h
+++ b/support/xtime.h
@@ -23,10 +23,15 @@
 
 __BEGIN_DECLS
 
+/* Name of the env variable, which indicates if it is possible to
+   adjust time on target machine.  */
+#define SETTIME_ENV_NAME "GLIBC_TEST_ALLOW_TIME_SETTING"
+
 /* The following functions call the corresponding libc functions and
    terminate the process on error.  */
 
 void xclock_gettime (clockid_t clock, struct timespec *ts);
+void xclock_settime (clockid_t clock, const struct timespec *ts);
 
 /* This helper can often simplify tests by avoiding an explicit
    variable declaration or allowing that declaration to be const. */
-- 
2.20.1

Re: [PATCH v4 2/3] support: Provide xclock_settime test helper function

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 3690 bytes --]

Dear Community,

> The xclock_settime is a wrapper function on the clock_settime syscall
> to be used in the test code.
> 
> It checks if the GLIBC_TEST_ALLOW_TIME_SETTING env variable is defined
> in the environment in which test is executed. If it is not - the test
> ends as unsupported. Otherwise, the clock-settime is executed and
> return value is assessed.
> 

Do you have any more comments regarding this patch?

> ---
> Changes for v4:
> - Remove not needed #include <support/xthread.h>
> ---
>  support/Makefile         |  1 +
>  support/xclock_settime.c | 34 ++++++++++++++++++++++++++++++++++
>  support/xtime.h          |  5 +++++
>  3 files changed, 40 insertions(+)
>  create mode 100644 support/xclock_settime.c
> 
> diff --git a/support/Makefile b/support/Makefile
> index bb9889efb4..8d63fbd5da 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -90,6 +90,7 @@ libsupport-routines = \
>    xchdir \
>    xchroot \
>    xclock_gettime \
> +  xclock_settime \
>    xclose \
>    xchmod \
>    xconnect \
> diff --git a/support/xclock_settime.c b/support/xclock_settime.c
> new file mode 100644
> index 0000000000..183aba4876
> --- /dev/null
> +++ b/support/xclock_settime.c
> @@ -0,0 +1,34 @@
> +/* clock_settime with error checking.
> +   Copyright (C) 2021 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 <stdlib.h>
> +#include <support/check.h>
> +#include <support/xtime.h>
> +
> +void
> +xclock_settime (clockid_t clockid,
> +                const struct timespec *ts)
> +{
> +  if (getenv (SETTIME_ENV_NAME) == NULL)
> +    FAIL_UNSUPPORTED ("clock_settime is executed only when "\
> +                      SETTIME_ENV_NAME" is set\n");
> +
> +  const int ret = clock_settime (clockid, ts);
> +  if (ret < 0)
> +    FAIL_EXIT1 ("clock_settime (%d): %m", clockid);
> +}
> diff --git a/support/xtime.h b/support/xtime.h
> index 2482837dee..b4ac3b59e2 100644
> --- a/support/xtime.h
> +++ b/support/xtime.h
> @@ -23,10 +23,15 @@
>  
>  __BEGIN_DECLS
>  
> +/* Name of the env variable, which indicates if it is possible to
> +   adjust time on target machine.  */
> +#define SETTIME_ENV_NAME "GLIBC_TEST_ALLOW_TIME_SETTING"
> +
>  /* The following functions call the corresponding libc functions and
>     terminate the process on error.  */
>  
>  void xclock_gettime (clockid_t clock, struct timespec *ts);
> +void xclock_settime (clockid_t clock, const struct timespec *ts);
>  
>  /* This helper can often simplify tests by avoiding an explicit
>     variable declaration or allowing that declaration to be const. */




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 2/3] support: Provide xclock_settime test helper function

[Adhemerval Zanella]

On 17/02/2021 19:31, Lukasz Majewski wrote:
> The xclock_settime is a wrapper function on the clock_settime syscall
> to be used in the test code.
> 
> It checks if the GLIBC_TEST_ALLOW_TIME_SETTING env variable is defined
> in the environment in which test is executed. If it is not - the test
> ends as unsupported. Otherwise, the clock-settime is executed and return
> value is assessed.
> 
> ---
> Changes for v4:
> - Remove not needed #include <support/xthread.h>

LGTM with a small nit below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  support/Makefile         |  1 +
>  support/xclock_settime.c | 34 ++++++++++++++++++++++++++++++++++
>  support/xtime.h          |  5 +++++
>  3 files changed, 40 insertions(+)
>  create mode 100644 support/xclock_settime.c
> 
> diff --git a/support/Makefile b/support/Makefile
> index bb9889efb4..8d63fbd5da 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -90,6 +90,7 @@ libsupport-routines = \
>    xchdir \
>    xchroot \
>    xclock_gettime \
> +  xclock_settime \
>    xclose \
>    xchmod \
>    xconnect \

Ok.

> diff --git a/support/xclock_settime.c b/support/xclock_settime.c
> new file mode 100644
> index 0000000000..183aba4876
> --- /dev/null
> +++ b/support/xclock_settime.c
> @@ -0,0 +1,34 @@
> +/* clock_settime with error checking.
> +   Copyright (C) 2021 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 <stdlib.h>
> +#include <support/check.h>
> +#include <support/xtime.h>
> +
> +void
> +xclock_settime (clockid_t clockid,
> +                const struct timespec *ts)
> +{
> +  if (getenv (SETTIME_ENV_NAME) == NULL)
> +    FAIL_UNSUPPORTED ("clock_settime is executed only when "\
> +                      SETTIME_ENV_NAME" is set\n");
> +
> +  const int ret = clock_settime (clockid, ts);

No need of 'const' here.

> +  if (ret < 0)
> +    FAIL_EXIT1 ("clock_settime (%d): %m", clockid);
> +}

Ok.

> diff --git a/support/xtime.h b/support/xtime.h
> index 2482837dee..b4ac3b59e2 100644
> --- a/support/xtime.h
> +++ b/support/xtime.h
> @@ -23,10 +23,15 @@
>  
>  __BEGIN_DECLS
>  
> +/* Name of the env variable, which indicates if it is possible to
> +   adjust time on target machine.  */
> +#define SETTIME_ENV_NAME "GLIBC_TEST_ALLOW_TIME_SETTING"
> +
>  /* The following functions call the corresponding libc functions and
>     terminate the process on error.  */
>  
>  void xclock_gettime (clockid_t clock, struct timespec *ts);
> +void xclock_settime (clockid_t clock, const struct timespec *ts);
>  
>  /* This helper can often simplify tests by avoiding an explicit
>     variable declaration or allowing that declaration to be const. */
> 

Ok.

[PATCH v4 3/3] tst: Add test for clock_settime

[Lukasz Majewski]

This code brings test to check if time on target machine is properly set.
To avoid any issues with altering the time:

- The time, which was set before the test was executed is restored.

- The time is altered only when cross-test-ssh.sh is executed with
  --allow-time-setting flag

Reviewed-by: DJ Delorie <dj@redhat.com>

---
Changes for v4:
- Remove "(in VM)" from test description
---
 time/Makefile            |  3 ++-
 time/tst-clock_settime.c | 45 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 time/tst-clock_settime.c

diff --git a/time/Makefile b/time/Makefile
index a8748480d6..b6f0969f3d 100644
--- a/time/Makefile
+++ b/time/Makefile
@@ -50,7 +50,8 @@ tests	:= test_time clocktest tst-posixtz tst-strptime tst_wcsftime \
 	   tst-tzname tst-y2039 bug-mktime4 tst-strftime2 tst-strftime3 \
 	   tst-clock tst-clock2 tst-clock_nanosleep tst-cpuclock1 \
 	   tst-adjtime tst-ctime tst-difftime tst-mktime4 tst-clock-y2038 \
-	   tst-clock2-y2038 tst-cpuclock1-y2038 tst-clock_nanosleep-y2038
+	   tst-clock2-y2038 tst-cpuclock1-y2038 tst-clock_nanosleep-y2038 \
+	   tst-clock_settime
 
 include ../Rules
 
diff --git a/time/tst-clock_settime.c b/time/tst-clock_settime.c
new file mode 100644
index 0000000000..90a16561ed
--- /dev/null
+++ b/time/tst-clock_settime.c
@@ -0,0 +1,45 @@
+/* Test for clock_settime
+   Copyright (C) 2021 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 <time.h>
+#include <support/check.h>
+#include <support/xtime.h>
+
+#define TIMESPEC_SEC_Y2038_OV 0x7FFFFFFF
+#define FUTURE_TIME (TIMESPEC_SEC_Y2038_OV - 10)
+
+static int
+do_test (void)
+{
+  const struct timespec tv = { FUTURE_TIME, 0};
+  struct timespec tv_future, tv_now;
+
+  tv_now = xclock_now(CLOCK_REALTIME);
+  xclock_settime(CLOCK_REALTIME, &tv);
+  tv_future = xclock_now(CLOCK_REALTIME);
+
+  /* Restore old time value on target machine.  */
+  xclock_settime(CLOCK_REALTIME, (const struct timespec*) &tv_now);
+
+  if (tv_future.tv_sec < tv.tv_sec)
+    FAIL_EXIT1 ("clock_settime set wrong time!\n");
+
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.20.1

Re: [PATCH v4 1/3] tst: Extend cross-test-ssh.sh to specify if target date can be altered

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 5095 bytes --]

Dear Community,

> This code adds new flag - '--allow-time-setting' to cross-test-ssh.sh
> script to indicate if it is allowed to alter the date on the system
> on which tests are executed. This change is supposed to be used with
> test systems, which use virtual machines for testing.
> 
> The GLIBC_TEST_ALLOW_TIME_SETTING env variable is exported to the
> remote environment on which the eligible test is run and brings no
> functional change when it is not.

Do you have any more comments regarding this patch? 

> 
> Changes for v2:
> - Utilize flock to provide serialization of cross-test-ssh.sh script
>   execution.
> - Add entry to manual/install.texi about --allow-time-setting flag
>   usage.
> 
> Changes for v3:
> - The install.texi manual has been augmented to explain two distinct
>   use cases for setting the time on target system.
> 
> Changes for v4:
> - Add -w switch to flock (specify timeout)
> - Check return value of flock - exit ssh shell script
> - Allow user override of FLOCK_* variables
> - Rewrite the script's information/help text
> - Rewrite the install.texi manual entry for this
>   script usage
> ---
>  manual/install.texi       | 17 +++++++++++++++++
>  scripts/cross-test-ssh.sh | 26 +++++++++++++++++++++++++-
>  2 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/manual/install.texi b/manual/install.texi
> index 419576f49c..5b32401805 100644
> --- a/manual/install.texi
> +++ b/manual/install.texi
> @@ -380,6 +380,23 @@ the newly built binaries of @theglibc{}.  The
> source and build directories must be visible at the same locations on
> both the build system and @var{hostname}.
>  
> +It is also possible to execute tests, which require setting date on
> +the target machine. Following use cases are supported:
> +@itemize @bullet
> +@item
> +@code{GLIBC_TEST_ALLOW_TIME_SETTING} is set in the environment in
> +which eligible tests are executed and have priviledges to run
> +@code{clock_settime}. In this case nothing prevents those tests from
> +running in parallel, so the caller shall assure that those tests
> +are serialized or provide a proper wrapper script for it.
> +
> +@item
> +The @code{cross-test-ssh.sh} script is used and one passes the
> +@option{--allow-time-setting} flag. In this case both setting
> +@code{GLIBC_TEST_ALLOW_TIME_SETTING} and serialization of tests
> +execution are assured automatically.
> +@end itemize
> +
>  In general, when testing @theglibc{}, @samp{test-wrapper} may be set
>  to the name and arguments of any program to run newly built binaries.
>  This program must preserve the arguments to the binary being run, its
> diff --git a/scripts/cross-test-ssh.sh b/scripts/cross-test-ssh.sh
> index 6d8fbcdfd2..e73802974d 100755
> --- a/scripts/cross-test-ssh.sh
> +++ b/scripts/cross-test-ssh.sh
> @@ -22,7 +22,7 @@
>  
>  progname="$(basename $0)"
>  
> -usage="usage: ${progname} [--ssh SSH] HOST COMMAND ..."
> +usage="usage: ${progname} [--ssh SSH] [--allow-time-setting] HOST
> COMMAND ..." help="Run a glibc test COMMAND on the remote machine
> HOST, via ssh, preserving the current working directory, and
> respecting quoting. 
> @@ -32,6 +32,11 @@ instead of ordinary 'ssh'.
>  If the '--timeoutfactor FACTOR' flag is present, set TIMEOUTFACTOR on
>  the remote machine to the specified FACTOR.
>  
> +If the '--allow-time-setting' flag is present, set
> +GLIBC_TEST_ALLOW_TIME_SETTING on the remote machine to indicate that
> +time can be safely adjusted when e.g. tests are run in a virtual
> +machine.
> +
>  To use this to run glibc tests, invoke the tests as follows:
>  
>    $ make test-wrapper='ABSPATH/cross-test-ssh.sh HOST' tests
> @@ -81,6 +86,10 @@ while [ $# -gt 0 ]; do
>        timeoutfactor="$1"
>        ;;
>  
> +    "--allow-time-setting")
> +      settimeallowed="1"
> +      ;;
> +
>      "--help")
>        echo "$usage"
>        echo "$help"
> @@ -127,6 +136,21 @@ if [ "$timeoutfactor" ]; then
>  ${command}"
>  fi
>  
> +# Add command to set the info that time on target can be adjusted,
> +# if required.
> +# Serialize execution of this script on target to prevent from
> unintended +# change of target time.
> +FLOCK_PATH="${FLOCK_PATH:-/var/lock/clock_settime}"
> +FLOCK_TIMEOUT="${FLOCK_TIMEOUT:-20}"
> +FLOCK_FD="${FLOCK_FD:-99}"
> +if [ "$settimeallowed" ]; then
> +  command="exec ${FLOCK_FD}<>${FLOCK_PATH}
> +flock -w ${FLOCK_TIMEOUT} ${FLOCK_FD}
> +if [ $? -ne 0 ]; then exit 1; fi
> +export GLIBC_TEST_ALLOW_TIME_SETTING=1
> +${command}"
> +fi
> +
>  # HOST's sshd simply concatenates its arguments with spaces and
>  # passes them to some shell.  We want to force the use of /bin/sh,
>  # so we need to re-quote the whole command to ensure it appears as




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 1/3] tst: Extend cross-test-ssh.sh to specify if target date can be altered

[Adhemerval Zanella]

On 17/02/2021 19:31, Lukasz Majewski wrote:
> This code adds new flag - '--allow-time-setting' to cross-test-ssh.sh
> script to indicate if it is allowed to alter the date on the system
> on which tests are executed. This change is supposed to be used with
> test systems, which use virtual machines for testing.
> 
> The GLIBC_TEST_ALLOW_TIME_SETTING env variable is exported to the
> remote environment on which the eligible test is run and brings no
> functional change when it is not.
> 
> Changes for v2:
> - Utilize flock to provide serialization of cross-test-ssh.sh script
>   execution.
> - Add entry to manual/install.texi about --allow-time-setting flag
>   usage.
> 
> Changes for v3:
> - The install.texi manual has been augmented to explain two distinct
>   use cases for setting the time on target system.
> 
> Changes for v4:
> - Add -w switch to flock (specify timeout)
> - Check return value of flock - exit ssh shell script
> - Allow user override of FLOCK_* variables
> - Rewrite the script's information/help text
> - Rewrite the install.texi manual entry for this
>   script usage

I think this would require a v5: if you decide to use the flock command
it should be documented on the manual, otherwise it should be done with
the .NOTPARALLEL special rule (as below). 

> ---
>  manual/install.texi       | 17 +++++++++++++++++
>  scripts/cross-test-ssh.sh | 26 +++++++++++++++++++++++++-
>  2 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/manual/install.texi b/manual/install.texi
> index 419576f49c..5b32401805 100644
> --- a/manual/install.texi
> +++ b/manual/install.texi
> @@ -380,6 +380,23 @@ the newly built binaries of @theglibc{}.  The source and build
>  directories must be visible at the same locations on both the build
>  system and @var{hostname}.
>  
> +It is also possible to execute tests, which require setting date on
> +the target machine. Following use cases are supported:

Two space after period.  There are other occurrences that need to be
fixed on this patch as well.

> +@itemize @bullet
> +@item
> +@code{GLIBC_TEST_ALLOW_TIME_SETTING} is set in the environment in
> +which eligible tests are executed and have priviledges to run

s/priviledges/privilege

> +@code{clock_settime}. In this case nothing prevents those tests from
> +running in parallel, so the caller shall assure that those tests
> +are serialized or provide a proper wrapper script for it.
> +
> +@item
> +The @code{cross-test-ssh.sh} script is used and one passes the
> +@option{--allow-time-setting} flag. In this case both setting
> +@code{GLIBC_TEST_ALLOW_TIME_SETTING} and serialization of tests
> +execution are assured automatically.
> +@end itemize
> +
>  In general, when testing @theglibc{}, @samp{test-wrapper} may be set
>  to the name and arguments of any program to run newly built binaries.
>  This program must preserve the arguments to the binary being run, its
> diff --git a/scripts/cross-test-ssh.sh b/scripts/cross-test-ssh.sh
> index 6d8fbcdfd2..e73802974d 100755
> --- a/scripts/cross-test-ssh.sh
> +++ b/scripts/cross-test-ssh.sh
> @@ -22,7 +22,7 @@
>  
>  progname="$(basename $0)"
>  
> -usage="usage: ${progname} [--ssh SSH] HOST COMMAND ..."
> +usage="usage: ${progname} [--ssh SSH] [--allow-time-setting] HOST COMMAND ..."
>  help="Run a glibc test COMMAND on the remote machine HOST, via ssh,
>  preserving the current working directory, and respecting quoting.
>  
> @@ -32,6 +32,11 @@ instead of ordinary 'ssh'.
>  If the '--timeoutfactor FACTOR' flag is present, set TIMEOUTFACTOR on
>  the remote machine to the specified FACTOR.
>  
> +If the '--allow-time-setting' flag is present, set
> +GLIBC_TEST_ALLOW_TIME_SETTING on the remote machine to indicate that
> +time can be safely adjusted when e.g. tests are run in a virtual
> +machine.

Maybe:

  time can be safely adjusted (e.g. on a virtual machine).

> +
>  To use this to run glibc tests, invoke the tests as follows:
>  
>    $ make test-wrapper='ABSPATH/cross-test-ssh.sh HOST' tests
> @@ -81,6 +86,10 @@ while [ $# -gt 0 ]; do
>        timeoutfactor="$1"
>        ;;
>  
> +    "--allow-time-setting")
> +      settimeallowed="1"
> +      ;;
> +
>      "--help")
>        echo "$usage"
>        echo "$help"

Ok.

> @@ -127,6 +136,21 @@ if [ "$timeoutfactor" ]; then
>  ${command}"
>  fi
>  
> +# Add command to set the info that time on target can be adjusted,
> +# if required.
> +# Serialize execution of this script on target to prevent from unintended
> +# change of target time.
> +FLOCK_PATH="${FLOCK_PATH:-/var/lock/clock_settime}"
> +FLOCK_TIMEOUT="${FLOCK_TIMEOUT:-20}"
> +FLOCK_FD="${FLOCK_FD:-99}"
> +if [ "$settimeallowed" ]; then
> +  command="exec ${FLOCK_FD}<>${FLOCK_PATH}
> +flock -w ${FLOCK_TIMEOUT} ${FLOCK_FD}
> +if [ $? -ne 0 ]; then exit 1; fi
> +export GLIBC_TEST_ALLOW_TIME_SETTING=1
> +${command}"
> +fi
> +
>  # HOST's sshd simply concatenates its arguments with spaces and
>  # passes them to some shell.  We want to force the use of /bin/sh,
>  # so we need to re-quote the whole command to ensure it appears as
> 

This requires the util-linux to be installed on the system (flock).
I think this should ok, but it need a note on the manual specifying
that if '--allow-time-setting' is used the 'flock' command should be
present on the target system.

The usual way to force non-parallel testing (as done for nptl and
benchtests) is through the special target .NOTPARALLEL.  Not sure
how easy would be to check , but one option would be to check
for '--allow-time-setting' on test-wrapper command and add the
.NOTPARALLEL rules as done in nptl/Makefile.

Re: [PATCH v4 1/3] tst: Extend cross-test-ssh.sh to specify if target date can be altered

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 7385 bytes --]

Hi Adhemerval,

> On 17/02/2021 19:31, Lukasz Majewski wrote:
> > This code adds new flag - '--allow-time-setting' to
> > cross-test-ssh.sh script to indicate if it is allowed to alter the
> > date on the system on which tests are executed. This change is
> > supposed to be used with test systems, which use virtual machines
> > for testing.
> > 
> > The GLIBC_TEST_ALLOW_TIME_SETTING env variable is exported to the
> > remote environment on which the eligible test is run and brings no
> > functional change when it is not.
> > 
> > Changes for v2:
> > - Utilize flock to provide serialization of cross-test-ssh.sh script
> >   execution.
> > - Add entry to manual/install.texi about --allow-time-setting flag
> >   usage.
> > 
> > Changes for v3:
> > - The install.texi manual has been augmented to explain two distinct
> >   use cases for setting the time on target system.
> > 
> > Changes for v4:
> > - Add -w switch to flock (specify timeout)
> > - Check return value of flock - exit ssh shell script
> > - Allow user override of FLOCK_* variables
> > - Rewrite the script's information/help text
> > - Rewrite the install.texi manual entry for this
> >   script usage  
> 
> I think this would require a v5: if you decide to use the flock
> command it should be documented on the manual, otherwise it should be
> done with the .NOTPARALLEL special rule (as below). 
> 
> > ---
> >  manual/install.texi       | 17 +++++++++++++++++
> >  scripts/cross-test-ssh.sh | 26 +++++++++++++++++++++++++-
> >  2 files changed, 42 insertions(+), 1 deletion(-)
> > 
> > diff --git a/manual/install.texi b/manual/install.texi
> > index 419576f49c..5b32401805 100644
> > --- a/manual/install.texi
> > +++ b/manual/install.texi
> > @@ -380,6 +380,23 @@ the newly built binaries of @theglibc{}.  The
> > source and build directories must be visible at the same locations
> > on both the build system and @var{hostname}.
> >  
> > +It is also possible to execute tests, which require setting date on
> > +the target machine. Following use cases are supported:  
> 
> Two space after period.  There are other occurrences that need to be
> fixed on this patch as well.
> 
> > +@itemize @bullet
> > +@item
> > +@code{GLIBC_TEST_ALLOW_TIME_SETTING} is set in the environment in
> > +which eligible tests are executed and have priviledges to run  
> 
> s/priviledges/privilege
> 
> > +@code{clock_settime}. In this case nothing prevents those tests
> > from +running in parallel, so the caller shall assure that those
> > tests +are serialized or provide a proper wrapper script for it.
> > +
> > +@item
> > +The @code{cross-test-ssh.sh} script is used and one passes the
> > +@option{--allow-time-setting} flag. In this case both setting
> > +@code{GLIBC_TEST_ALLOW_TIME_SETTING} and serialization of tests
> > +execution are assured automatically.
> > +@end itemize
> > +
> >  In general, when testing @theglibc{}, @samp{test-wrapper} may be
> > set to the name and arguments of any program to run newly built
> > binaries. This program must preserve the arguments to the binary
> > being run, its diff --git a/scripts/cross-test-ssh.sh
> > b/scripts/cross-test-ssh.sh index 6d8fbcdfd2..e73802974d 100755
> > --- a/scripts/cross-test-ssh.sh
> > +++ b/scripts/cross-test-ssh.sh
> > @@ -22,7 +22,7 @@
> >  
> >  progname="$(basename $0)"
> >  
> > -usage="usage: ${progname} [--ssh SSH] HOST COMMAND ..."
> > +usage="usage: ${progname} [--ssh SSH] [--allow-time-setting] HOST
> > COMMAND ..." help="Run a glibc test COMMAND on the remote machine
> > HOST, via ssh, preserving the current working directory, and
> > respecting quoting. 
> > @@ -32,6 +32,11 @@ instead of ordinary 'ssh'.
> >  If the '--timeoutfactor FACTOR' flag is present, set TIMEOUTFACTOR
> > on the remote machine to the specified FACTOR.
> >  
> > +If the '--allow-time-setting' flag is present, set
> > +GLIBC_TEST_ALLOW_TIME_SETTING on the remote machine to indicate
> > that +time can be safely adjusted when e.g. tests are run in a
> > virtual +machine.  
> 
> Maybe:
> 
>   time can be safely adjusted (e.g. on a virtual machine).
> 
> > +
> >  To use this to run glibc tests, invoke the tests as follows:
> >  
> >    $ make test-wrapper='ABSPATH/cross-test-ssh.sh HOST' tests
> > @@ -81,6 +86,10 @@ while [ $# -gt 0 ]; do
> >        timeoutfactor="$1"
> >        ;;
> >  
> > +    "--allow-time-setting")
> > +      settimeallowed="1"
> > +      ;;
> > +
> >      "--help")
> >        echo "$usage"
> >        echo "$help"  
> 
> Ok.
> 
> > @@ -127,6 +136,21 @@ if [ "$timeoutfactor" ]; then
> >  ${command}"
> >  fi
> >  
> > +# Add command to set the info that time on target can be adjusted,
> > +# if required.
> > +# Serialize execution of this script on target to prevent from
> > unintended +# change of target time.
> > +FLOCK_PATH="${FLOCK_PATH:-/var/lock/clock_settime}"
> > +FLOCK_TIMEOUT="${FLOCK_TIMEOUT:-20}"
> > +FLOCK_FD="${FLOCK_FD:-99}"
> > +if [ "$settimeallowed" ]; then
> > +  command="exec ${FLOCK_FD}<>${FLOCK_PATH}
> > +flock -w ${FLOCK_TIMEOUT} ${FLOCK_FD}
> > +if [ $? -ne 0 ]; then exit 1; fi
> > +export GLIBC_TEST_ALLOW_TIME_SETTING=1
> > +${command}"
> > +fi
> > +
> >  # HOST's sshd simply concatenates its arguments with spaces and
> >  # passes them to some shell.  We want to force the use of /bin/sh,
> >  # so we need to re-quote the whole command to ensure it appears as
> >   
> 
> This requires the util-linux to be installed on the system (flock).
> I think this should ok, but it need a note on the manual specifying
> that if '--allow-time-setting' is used the 'flock' command should be
> present on the target system.
> 

The 'flock' solution has on big advantage - it is easy to add to the
cross-test-ssh.sh script. It just extends this particular wrapper (a
few lines of code), as it is forbidden (for now) to adjust time on HOST
(i.e. non VM) system.

It works well for running Y2038 VM tests.

> The usual way to force non-parallel testing (as done for nptl and
> benchtests) is through the special target .NOTPARALLEL. 

The approach with .NOTPARALLEL seems to be more generic, but would
require some Makefiles (and Rules) modification, which may be time
consuming.

> Not sure
> how easy would be to check , but one option would be to check
> for '--allow-time-setting' on test-wrapper command and add the
> .NOTPARALLEL rules as done in nptl/Makefile.

I also think that the implementation would require checking arguments
passed to 'test-wrapper' env variable and define .NOTPARALLEL: target
for several Makefiles (in ./time , ./misc , ./io , etc.).

I do have a feeling that for our purpose (to add tests) it would be
easier for maintenance to use 'flock' and then (if required) move to
.NOTPARALLEL. In the end of the day both approaches do their job, but
IMHO 'flock' is easier to maintain and implement.


Off topic:
----------
What is the purpose of MAKECMDGOALS variable in nptl/Makefile? It
is only used in this place.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 1/3] tst: Extend cross-test-ssh.sh to specify if target date can be altered

[Adhemerval Zanella]

On 28/02/2021 20:06, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 17/02/2021 19:31, Lukasz Majewski wrote:
>>> This code adds new flag - '--allow-time-setting' to
>>> cross-test-ssh.sh script to indicate if it is allowed to alter the
>>> date on the system on which tests are executed. This change is
>>> supposed to be used with test systems, which use virtual machines
>>> for testing.
>>>
>>> The GLIBC_TEST_ALLOW_TIME_SETTING env variable is exported to the
>>> remote environment on which the eligible test is run and brings no
>>> functional change when it is not.
>>>
>>> Changes for v2:
>>> - Utilize flock to provide serialization of cross-test-ssh.sh script
>>>   execution.
>>> - Add entry to manual/install.texi about --allow-time-setting flag
>>>   usage.
>>>
>>> Changes for v3:
>>> - The install.texi manual has been augmented to explain two distinct
>>>   use cases for setting the time on target system.
>>>
>>> Changes for v4:
>>> - Add -w switch to flock (specify timeout)
>>> - Check return value of flock - exit ssh shell script
>>> - Allow user override of FLOCK_* variables
>>> - Rewrite the script's information/help text
>>> - Rewrite the install.texi manual entry for this
>>>   script usage  
>>
>> I think this would require a v5: if you decide to use the flock
>> command it should be documented on the manual, otherwise it should be
>> done with the .NOTPARALLEL special rule (as below). 
>>
>>> ---
>>>  manual/install.texi       | 17 +++++++++++++++++
>>>  scripts/cross-test-ssh.sh | 26 +++++++++++++++++++++++++-
>>>  2 files changed, 42 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/manual/install.texi b/manual/install.texi
>>> index 419576f49c..5b32401805 100644
>>> --- a/manual/install.texi
>>> +++ b/manual/install.texi
>>> @@ -380,6 +380,23 @@ the newly built binaries of @theglibc{}.  The
>>> source and build directories must be visible at the same locations
>>> on both the build system and @var{hostname}.
>>>  
>>> +It is also possible to execute tests, which require setting date on
>>> +the target machine. Following use cases are supported:  
>>
>> Two space after period.  There are other occurrences that need to be
>> fixed on this patch as well.
>>
>>> +@itemize @bullet
>>> +@item
>>> +@code{GLIBC_TEST_ALLOW_TIME_SETTING} is set in the environment in
>>> +which eligible tests are executed and have priviledges to run  
>>
>> s/priviledges/privilege
>>
>>> +@code{clock_settime}. In this case nothing prevents those tests
>>> from +running in parallel, so the caller shall assure that those
>>> tests +are serialized or provide a proper wrapper script for it.
>>> +
>>> +@item
>>> +The @code{cross-test-ssh.sh} script is used and one passes the
>>> +@option{--allow-time-setting} flag. In this case both setting
>>> +@code{GLIBC_TEST_ALLOW_TIME_SETTING} and serialization of tests
>>> +execution are assured automatically.
>>> +@end itemize
>>> +
>>>  In general, when testing @theglibc{}, @samp{test-wrapper} may be
>>> set to the name and arguments of any program to run newly built
>>> binaries. This program must preserve the arguments to the binary
>>> being run, its diff --git a/scripts/cross-test-ssh.sh
>>> b/scripts/cross-test-ssh.sh index 6d8fbcdfd2..e73802974d 100755
>>> --- a/scripts/cross-test-ssh.sh
>>> +++ b/scripts/cross-test-ssh.sh
>>> @@ -22,7 +22,7 @@
>>>  
>>>  progname="$(basename $0)"
>>>  
>>> -usage="usage: ${progname} [--ssh SSH] HOST COMMAND ..."
>>> +usage="usage: ${progname} [--ssh SSH] [--allow-time-setting] HOST
>>> COMMAND ..." help="Run a glibc test COMMAND on the remote machine
>>> HOST, via ssh, preserving the current working directory, and
>>> respecting quoting. 
>>> @@ -32,6 +32,11 @@ instead of ordinary 'ssh'.
>>>  If the '--timeoutfactor FACTOR' flag is present, set TIMEOUTFACTOR
>>> on the remote machine to the specified FACTOR.
>>>  
>>> +If the '--allow-time-setting' flag is present, set
>>> +GLIBC_TEST_ALLOW_TIME_SETTING on the remote machine to indicate
>>> that +time can be safely adjusted when e.g. tests are run in a
>>> virtual +machine.  
>>
>> Maybe:
>>
>>   time can be safely adjusted (e.g. on a virtual machine).
>>
>>> +
>>>  To use this to run glibc tests, invoke the tests as follows:
>>>  
>>>    $ make test-wrapper='ABSPATH/cross-test-ssh.sh HOST' tests
>>> @@ -81,6 +86,10 @@ while [ $# -gt 0 ]; do
>>>        timeoutfactor="$1"
>>>        ;;
>>>  
>>> +    "--allow-time-setting")
>>> +      settimeallowed="1"
>>> +      ;;
>>> +
>>>      "--help")
>>>        echo "$usage"
>>>        echo "$help"  
>>
>> Ok.
>>
>>> @@ -127,6 +136,21 @@ if [ "$timeoutfactor" ]; then
>>>  ${command}"
>>>  fi
>>>  
>>> +# Add command to set the info that time on target can be adjusted,
>>> +# if required.
>>> +# Serialize execution of this script on target to prevent from
>>> unintended +# change of target time.
>>> +FLOCK_PATH="${FLOCK_PATH:-/var/lock/clock_settime}"
>>> +FLOCK_TIMEOUT="${FLOCK_TIMEOUT:-20}"
>>> +FLOCK_FD="${FLOCK_FD:-99}"
>>> +if [ "$settimeallowed" ]; then
>>> +  command="exec ${FLOCK_FD}<>${FLOCK_PATH}
>>> +flock -w ${FLOCK_TIMEOUT} ${FLOCK_FD}
>>> +if [ $? -ne 0 ]; then exit 1; fi
>>> +export GLIBC_TEST_ALLOW_TIME_SETTING=1
>>> +${command}"
>>> +fi
>>> +
>>>  # HOST's sshd simply concatenates its arguments with spaces and
>>>  # passes them to some shell.  We want to force the use of /bin/sh,
>>>  # so we need to re-quote the whole command to ensure it appears as
>>>   
>>
>> This requires the util-linux to be installed on the system (flock).
>> I think this should ok, but it need a note on the manual specifying
>> that if '--allow-time-setting' is used the 'flock' command should be
>> present on the target system.
>>
> 
> The 'flock' solution has on big advantage - it is easy to add to the
> cross-test-ssh.sh script. It just extends this particular wrapper (a
> few lines of code), as it is forbidden (for now) to adjust time on HOST
> (i.e. non VM) system.
> 
> It works well for running Y2038 VM tests.
> 
>> The usual way to force non-parallel testing (as done for nptl and
>> benchtests) is through the special target .NOTPARALLEL. 
> 
> The approach with .NOTPARALLEL seems to be more generic, but would
> require some Makefiles (and Rules) modification, which may be time
> consuming.
> 
>> Not sure
>> how easy would be to check , but one option would be to check
>> for '--allow-time-setting' on test-wrapper command and add the
>> .NOTPARALLEL rules as done in nptl/Makefile.
> 
> I also think that the implementation would require checking arguments
> passed to 'test-wrapper' env variable and define .NOTPARALLEL: target
> for several Makefiles (in ./time , ./misc , ./io , etc.).
> 
> I do have a feeling that for our purpose (to add tests) it would be
> easier for maintenance to use 'flock' and then (if required) move to
> .NOTPARALLEL. In the end of the day both approaches do their job, but
> IMHO 'flock' is easier to maintain and implement.

Fair enough, flock seems ok. Please add a note on INSTALL that it requires
flock from util-linux to work with GLIBC_TEST_ALLOW_TIME_SETTING

> 
> 
> Off topic:
> ----------
> What is the purpose of MAKECMDGOALS variable in nptl/Makefile? It
> is only used in this place.
> 

This is set by GNU make itself [1] and my understanding it runs the
nptl tests in non-parallel manner unless you specify the test itself
(make test t=nptl/xxxxx).

[1] https://www.gnu.org/software/make/manual/html_node/Goals.html

> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
>