diff options
| author | Richard Purdie <richard.purdie@linuxfoundation.org> | 2019-04-04 14:27:09 +0100 |
|---|---|---|
| committer | Richard Purdie <richard.purdie@linuxfoundation.org> | 2019-04-09 13:44:39 +0100 |
| commit | 65b37734c7350443957dcb7d8f299d384d5aaa8c (patch) | |
| tree | 93446beec4cb7f22cb784eaa6fc0f2dc1c30adae | |
| parent | 57933bd958cc06722ab7924ec9dc2c72fadc9693 (diff) | |
| download | poky-65b37734c7350443957dcb7d8f299d384d5aaa8c.tar.gz | |
ptest-runner: Add several logging fixes
This change adds three patches to improve the handling of stdout/stderr and child
processes to try and improve logging reliability in ptest-runner.
(From OE-Core rev: 1c0fffc401cdb581a93d16d225f53c83359ff209)
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
4 files changed, 161 insertions, 1 deletions
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 0000000000..c9a9dd7cf4 --- /dev/null +++ b/meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch | |||
| @@ -0,0 +1,45 @@ | |||
| 1 | From 9b36993794c1de733c521b2477370c874c07b617 Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Richard Purdie <richard.purdie@linuxfoundation.org> | ||
| 3 | Date: Thu, 4 Apr 2019 14:18:55 +0100 | ||
| 4 | Subject: [PATCH 1/3] utils: Ensure stdout/stderr are flushed | ||
| 5 | |||
| 6 | There is no guarantee that the data written with fwrite will be flushed to the | ||
| 7 | buffer. If stdout and stderr are the same thing, this could lead to interleaved | ||
| 8 | writes. The common case is stdout output so flush the output pipes when writing to | ||
| 9 | stderr. Also flush stdout before the function returns. | ||
| 10 | |||
| 11 | Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> | ||
| 12 | Upstream-Status: Pending [code being tested] | ||
| 13 | --- | ||
| 14 | utils.c | 7 +++++-- | ||
| 15 | 1 file changed, 5 insertions(+), 2 deletions(-) | ||
| 16 | |||
| 17 | diff --git a/utils.c b/utils.c | ||
| 18 | index 504df0b..3ceb342 100644 | ||
| 19 | --- a/utils.c | ||
| 20 | +++ b/utils.c | ||
| 21 | @@ -295,8 +295,11 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, | ||
| 22 | } | ||
| 23 | |||
| 24 | if (pfds[1].revents != 0) { | ||
| 25 | - while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0) | ||
| 26 | + while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0) { | ||
| 27 | + fflush(fps[0]); | ||
| 28 | fwrite(buf, n, 1, fps[1]); | ||
| 29 | + fflush(fps[1]); | ||
| 30 | + } | ||
| 31 | } | ||
| 32 | |||
| 33 | clock_gettime(clock, &sentinel); | ||
| 34 | @@ -315,7 +318,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, | ||
| 35 | break; | ||
| 36 | } | ||
| 37 | |||
| 38 | - | ||
| 39 | + fflush(fps[0]); | ||
| 40 | return status; | ||
| 41 | } | ||
| 42 | |||
| 43 | -- | ||
| 44 | 2.17.1 | ||
| 45 | |||
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 0000000000..5436a3340c --- /dev/null +++ b/meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch | |||
| @@ -0,0 +1,35 @@ | |||
| 1 | From f0c42a65633341ad048718c7a6dbd035818e9eaf Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Richard Purdie <richard.purdie@linuxfoundation.org> | ||
| 3 | Date: Thu, 4 Apr 2019 14:20:31 +0100 | ||
| 4 | Subject: [PATCH 2/3] use process groups when spawning | ||
| 5 | |||
| 6 | Rather than just killing the process we've swawned, set the process group | ||
| 7 | for spawned children and then kill the group of processes. | ||
| 8 | |||
| 9 | Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> | ||
| 10 | Upstream-Status: Pending [code being tested] | ||
| 11 | --- | ||
| 12 | utils.c | 9 +++++---- | ||
| 13 | 1 file changed, 5 insertions(+), 4 deletions(-) | ||
| 14 | |||
| 15 | diff --git a/utils.c b/utils.c | ||
| 16 | index 3ceb342..c5b3b8d 100644 | ||
| 17 | --- a/utils.c | ||
| 18 | +++ b/utils.c | ||
| 19 | @@ -309,7 +309,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, | ||
| 20 | clock_gettime(clock, &time); | ||
| 21 | if ((time.tv_sec - sentinel.tv_sec) > timeout) { | ||
| 22 | *timeouted = 1; | ||
| 23 | - kill(pid, SIGKILL); | ||
| 24 | + kill(-pid, SIGKILL); | ||
| 25 | waitflags = 0; | ||
| 26 | } | ||
| 27 | } | ||
| 28 | @@ -371,6 +371,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts, | ||
| 29 | rc = -1; | ||
| 30 | break; | ||
| 31 | } else if (child == 0) { | ||
| 32 | + setsid(); | ||
| 33 | run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]); | ||
| 34 | } else { | ||
| 35 | 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 0000000000..f7c3ebe6f2 --- /dev/null +++ b/meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch | |||
| @@ -0,0 +1,76 @@ | |||
| 1 | From e58e4e1a7f854953f823dc5135d35f728f253f31 Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Richard Purdie <richard.purdie@linuxfoundation.org> | ||
| 3 | Date: Thu, 4 Apr 2019 14:24:14 +0100 | ||
| 4 | Subject: [PATCH 3/3] utils: Ensure pipes are read after exit | ||
| 5 | |||
| 6 | There was a race in the code where the pipes may not be read after the process has exited | ||
| 7 | and data may be left behind in them. This change to ordering ensures the pipes are read | ||
| 8 | after the exit code has been read meaning no data can be left behind and the logs should | ||
| 9 | be complete. | ||
| 10 | |||
| 11 | Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> | ||
| 12 | Upstream-Status: Pending [code being tested] | ||
| 13 | --- | ||
| 14 | utils.c | 29 ++++++++++++++++------------- | ||
| 15 | 1 file changed, 16 insertions(+), 13 deletions(-) | ||
| 16 | |||
| 17 | diff --git a/utils.c b/utils.c | ||
| 18 | index c5b3b8d..37e88ab 100644 | ||
| 19 | --- a/utils.c | ||
| 20 | +++ b/utils.c | ||
| 21 | @@ -264,6 +264,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group, | ||
| 22 | struct pollfd pfds[2]; | ||
| 23 | struct timespec sentinel; | ||
| 24 | clockid_t clock = CLOCK_MONOTONIC; | ||
| 25 | + int looping = 1; | ||
| 26 | int r; | ||
| 27 | |||
| 28 | int status; | ||
| 29 | @@ -281,9 +282,23 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group, | ||
| 30 | |||
| 31 | *timeouted = 0; | ||
| 32 | |||
| 33 | - while (1) { | ||
| 34 | + while (looping) { | ||
| 35 | waitflags = WNOHANG; | ||
| 36 | |||
| 37 | + if (timeout >= 0) { | ||
| 38 | + struct timespec time; | ||
| 39 | + | ||
| 40 | + clock_gettime(clock, &time); | ||
| 41 | + if ((time.tv_sec - sentinel.tv_sec) > timeout) { | ||
| 42 | + *timeouted = 1; | ||
| 43 | + kill(-pid, SIGKILL); | ||
| 44 | + waitflags = 0; | ||
| 45 | + } | ||
| 46 | + } | ||
| 47 | + | ||
| 48 | + if (waitpid(pid, &status, waitflags) == pid) | ||
| 49 | + looping = 0; | ||
| 50 | + | ||
| 51 | r = poll(pfds, 2, WAIT_CHILD_POLL_TIMEOUT_MS); | ||
| 52 | if (r > 0) { | ||
| 53 | char buf[WAIT_CHILD_BUF_MAX_SIZE]; | ||
| 54 | @@ -303,19 +318,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group, | ||
| 55 | } | ||
| 56 | |||
| 57 | clock_gettime(clock, &sentinel); | ||
| 58 | - } else if (timeout >= 0) { | ||
| 59 | - struct timespec time; | ||
| 60 | - | ||
| 61 | - clock_gettime(clock, &time); | ||
| 62 | - if ((time.tv_sec - sentinel.tv_sec) > timeout) { | ||
| 63 | - *timeouted = 1; | ||
| 64 | - kill(-pid, SIGKILL); | ||
| 65 | - waitflags = 0; | ||
| 66 | - } | ||
| 67 | } | ||
| 68 | - | ||
| 69 | - if (waitpid(pid, &status, waitflags) == pid) | ||
| 70 | - break; | ||
| 71 | } | ||
| 72 | |||
| 73 | fflush(fps[0]); | ||
| 74 | -- | ||
| 75 | 2.17.1 | ||
| 76 | |||
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 4b7992bf2f..e2eb258d0b 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" | |||
| 10 | SRCREV = "05b112bda7ac2adba8e9b0f088d6e5843b148a38" | 10 | SRCREV = "05b112bda7ac2adba8e9b0f088d6e5843b148a38" |
| 11 | PV = "2.3.1+git${SRCPV}" | 11 | PV = "2.3.1+git${SRCPV}" |
| 12 | 12 | ||
| 13 | SRC_URI = "git://git.yoctoproject.org/ptest-runner2" | 13 | SRC_URI = "git://git.yoctoproject.org/ptest-runner2 \ |
| 14 | file://0001-utils-Ensure-stdout-stderr-are-flushed.patch \ | ||
| 15 | file://0002-use-process-groups-when-spawning.patch \ | ||
| 16 | file://0003-utils-Ensure-pipes-are-read-after-exit.patch" | ||
| 17 | |||
| 14 | S = "${WORKDIR}/git" | 18 | S = "${WORKDIR}/git" |
| 15 | 19 | ||
| 16 | FILES_${PN} = "${bindir}/ptest-runner" | 20 | FILES_${PN} = "${bindir}/ptest-runner" |
