[OE-core] [PATCH 3/3] ptest-runner: Add several logging fixes
Randy MacLeod
randy.macleod at windriver.com
Thu Jun 6 20:26:33 UTC 2019
On 4/8/19 7:59 AM, Richard Purdie wrote:
> This change adds three patches to improve the handling of stdout/stderr and child
> processes to try and improve logging reliability in ptest-runner.
>
> Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
> ---
> ...ils-Ensure-stdout-stderr-are-flushed.patch | 45 +++++++++++
> ...002-use-process-groups-when-spawning.patch | 35 +++++++++
> ...ils-Ensure-pipes-are-read-after-exit.patch | 76 +++++++++++++++++++
Sakib and I found another problem with ptest-runner and job control.
When bash runs some ptests, there's a warning:
< bash: cannot set terminal process group (2980): Inappropriate ioctl
for device
< bash: no job control in this shell
This only happens when being run under ptest-runner instead of say
under /usr/lib/bash/ptest/run-ptest. There may also be some problems
with su'ing to the bash_user account but we'll see.
So we're working on this and we'd like to know if the three patches here
are considered stable and should therefore be merged to ptest-runner?
Sakib is testing a patch to detach in the parent and hand the
controlling terminal to the child using a couple of
ioctl(0, TIOCSCTTY, NULL) type of calls.
../Randy
> .../ptest-runner/ptest-runner_2.3.1.bb | 6 +-
> 4 files changed, 161 insertions(+), 1 deletion(-)
> create mode 100644 meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch
> create mode 100644 meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch
> create mode 100644 meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch
>
> diff --git a/meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch b/meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch
> new file mode 100644
> index 00000000000..c9a9dd7cf49
> --- /dev/null
> +++ b/meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch
> @@ -0,0 +1,45 @@
> +From 9b36993794c1de733c521b2477370c874c07b617 Mon Sep 17 00:00:00 2001
> +From: Richard Purdie <richard.purdie at linuxfoundation.org>
> +Date: Thu, 4 Apr 2019 14:18:55 +0100
> +Subject: [PATCH 1/3] utils: Ensure stdout/stderr are flushed
> +
> +There is no guarantee that the data written with fwrite will be flushed to the
> +buffer. If stdout and stderr are the same thing, this could lead to interleaved
> +writes. The common case is stdout output so flush the output pipes when writing to
> +stderr. Also flush stdout before the function returns.
> +
> +Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
> +Upstream-Status: Pending [code being tested]
> +---
> + utils.c | 7 +++++--
> + 1 file changed, 5 insertions(+), 2 deletions(-)
> +
> +diff --git a/utils.c b/utils.c
> +index 504df0b..3ceb342 100644
> +--- a/utils.c
> ++++ b/utils.c
> +@@ -295,8 +295,11 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
> + }
> +
> + if (pfds[1].revents != 0) {
> +- while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0)
> ++ while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0) {
> ++ fflush(fps[0]);
> + fwrite(buf, n, 1, fps[1]);
> ++ fflush(fps[1]);
> ++ }
> + }
> +
> + clock_gettime(clock, &sentinel);
> +@@ -315,7 +318,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
> + break;
> + }
> +
> +-
> ++ fflush(fps[0]);
> + return status;
> + }
> +
> +--
> +2.17.1
> +
> diff --git a/meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch b/meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch
> new file mode 100644
> index 00000000000..5436a3340c4
> --- /dev/null
> +++ b/meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch
> @@ -0,0 +1,35 @@
> +From f0c42a65633341ad048718c7a6dbd035818e9eaf Mon Sep 17 00:00:00 2001
> +From: Richard Purdie <richard.purdie at linuxfoundation.org>
> +Date: Thu, 4 Apr 2019 14:20:31 +0100
> +Subject: [PATCH 2/3] use process groups when spawning
> +
> +Rather than just killing the process we've swawned, set the process group
> +for spawned children and then kill the group of processes.
> +
> +Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
> +Upstream-Status: Pending [code being tested]
> +---
> + utils.c | 9 +++++----
> + 1 file changed, 5 insertions(+), 4 deletions(-)
> +
> +diff --git a/utils.c b/utils.c
> +index 3ceb342..c5b3b8d 100644
> +--- a/utils.c
> ++++ b/utils.c
> +@@ -309,7 +309,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
> + clock_gettime(clock, &time);
> + if ((time.tv_sec - sentinel.tv_sec) > timeout) {
> + *timeouted = 1;
> +- kill(pid, SIGKILL);
> ++ kill(-pid, SIGKILL);
> + waitflags = 0;
> + }
> + }
> +@@ -371,6 +371,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
> + rc = -1;
> + break;
> + } else if (child == 0) {
> ++ setsid();
> + run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);
> + } else {
> + int status;
> diff --git a/meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch b/meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch
> new file mode 100644
> index 00000000000..f7c3ebe6f2c
> --- /dev/null
> +++ b/meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch
> @@ -0,0 +1,76 @@
> +From e58e4e1a7f854953f823dc5135d35f728f253f31 Mon Sep 17 00:00:00 2001
> +From: Richard Purdie <richard.purdie at linuxfoundation.org>
> +Date: Thu, 4 Apr 2019 14:24:14 +0100
> +Subject: [PATCH 3/3] utils: Ensure pipes are read after exit
> +
> +There was a race in the code where the pipes may not be read after the process has exited
> +and data may be left behind in them. This change to ordering ensures the pipes are read
> +after the exit code has been read meaning no data can be left behind and the logs should
> +be complete.
> +
> +Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
> +Upstream-Status: Pending [code being tested]
> +---
> + utils.c | 29 ++++++++++++++++-------------
> + 1 file changed, 16 insertions(+), 13 deletions(-)
> +
> +diff --git a/utils.c b/utils.c
> +index c5b3b8d..37e88ab 100644
> +--- a/utils.c
> ++++ b/utils.c
> +@@ -264,6 +264,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group,
> + struct pollfd pfds[2];
> + struct timespec sentinel;
> + clockid_t clock = CLOCK_MONOTONIC;
> ++ int looping = 1;
> + int r;
> +
> + int status;
> +@@ -281,9 +282,23 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group,
> +
> + *timeouted = 0;
> +
> +- while (1) {
> ++ while (looping) {
> + waitflags = WNOHANG;
> +
> ++ if (timeout >= 0) {
> ++ struct timespec time;
> ++
> ++ clock_gettime(clock, &time);
> ++ if ((time.tv_sec - sentinel.tv_sec) > timeout) {
> ++ *timeouted = 1;
> ++ kill(-pid, SIGKILL);
> ++ waitflags = 0;
> ++ }
> ++ }
> ++
> ++ if (waitpid(pid, &status, waitflags) == pid)
> ++ looping = 0;
> ++
> + r = poll(pfds, 2, WAIT_CHILD_POLL_TIMEOUT_MS);
> + if (r > 0) {
> + char buf[WAIT_CHILD_BUF_MAX_SIZE];
> +@@ -303,19 +318,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group,
> + }
> +
> + clock_gettime(clock, &sentinel);
> +- } else if (timeout >= 0) {
> +- struct timespec time;
> +-
> +- clock_gettime(clock, &time);
> +- if ((time.tv_sec - sentinel.tv_sec) > timeout) {
> +- *timeouted = 1;
> +- kill(-pid, SIGKILL);
> +- waitflags = 0;
> +- }
> + }
> +-
> +- if (waitpid(pid, &status, waitflags) == pid)
> +- break;
> + }
> +
> + fflush(fps[0]);
> +--
> +2.17.1
> +
> diff --git a/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb b/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
> index 4b7992bf2fd..e2eb258d0b0 100644
> --- a/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
> +++ b/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
> @@ -10,7 +10,11 @@ LIC_FILES_CHKSUM = "file://LICENSE;md5=751419260aa954499f7abaabaa882bbe"
> SRCREV = "05b112bda7ac2adba8e9b0f088d6e5843b148a38"
> PV = "2.3.1+git${SRCPV}"
>
> -SRC_URI = "git://git.yoctoproject.org/ptest-runner2"
> +SRC_URI = "git://git.yoctoproject.org/ptest-runner2 \
> + file://0001-utils-Ensure-stdout-stderr-are-flushed.patch \
> + file://0002-use-process-groups-when-spawning.patch \
> + file://0003-utils-Ensure-pipes-are-read-after-exit.patch"
> +
> S = "${WORKDIR}/git"
>
> FILES_${PN} = "${bindir}/ptest-runner"
>
--
# Randy MacLeod
# Wind River Linux
More information about the Openembedded-core
mailing list