From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x72c.google.com (mail-qk1-x72c.google.com [IPv6:2607:f8b0:4864:20::72c]) by sourceware.org (Postfix) with ESMTPS id 96C4A3874C1D for ; Tue, 2 Mar 2021 13:58:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 96C4A3874C1D Received: by mail-qk1-x72c.google.com with SMTP id z128so20177544qkc.12 for ; Tue, 02 Mar 2021 05:58:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:cc:references:from:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=EUFChJXBluKGW/tEtOWCrlBm53KR2a78QhRf4AKA/7w=; b=FzUHxozu3nujsSK8MpZBVxXO3INvlwy8464s68K4xTZyUPL81PE2SJ1dDAsjXILJrb CI2xRxhOfik+Fsw2kMJ0D+Y93sHfTQNHeDvQVJ8GlUxM2pLNYACxbMPXNrRuKiKhCVtY 6lAvw1BMlCUKUXBiTgEwJyQkpCAg8op9B/bhNyPH1trRBO26Nnpe/ztfb9isbDSo7Yfw kgkEnQmBNlKcEQbj7WbIQFK4EEa6la73kgyuuo+COoJzXStKua0oQhPNXepjsMko2mty cbMcotk2ikAMR1SthXbEXc5vtVkUUOe/nS6Ez/S9SgGVy8gyEe50Cwu+Kcnqb5RLFA/b H49g== X-Gm-Message-State: AOAM532Jyk9VieB5naof4MYsYch6bNiCrsRLW5wZ6K5m5r848V5Dy/bz YERnR/nM1HzmZSKwCHoufnntJQ== X-Google-Smtp-Source: ABdhPJxIkMnU+qjdz9fH5u4I1+iim7IRHqcBt83McrmfH3wJI2tSuTVeJTBiUqHb3kvCNnCzGMATnw== X-Received: by 2002:a37:ba03:: with SMTP id k3mr12647088qkf.336.1614693510862; Tue, 02 Mar 2021 05:58:30 -0800 (PST) Received: from [192.168.1.4] ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id q6sm14839427qkd.41.2021.03.02.05.58.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 02 Mar 2021 05:58:30 -0800 (PST) To: Lukasz Majewski Cc: Joseph Myers , Florian Weimer , GNU C Library , Carlos O'Donell References: <20210217223158.16441-1-lukma@denx.de> <20210301000623.076245ef@jawa> From: Adhemerval Zanella Subject: Re: [PATCH v4 1/3] tst: Extend cross-test-ssh.sh to specify if target date can be altered Message-ID: Date: Tue, 2 Mar 2021 10:58:27 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210301000623.076245ef@jawa> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Mar 2021 13:58:33 -0000 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 >