From e7476647385a9b613ec0e14bd2b4a4e57d848c64 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 6 May 2024 14:04:38 -0400 Subject: [PATCH 1/2] Revert "cmdlib.sh: go back to using `tail -F` for command output" This reverts commit 79b15c89d57a4b97334c70881b036aee3462f1e4. I found a workaround for the issue that prompted that commit. See next commit. --- src/cmdlib.sh | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/cmdlib.sh b/src/cmdlib.sh index e477a628ad..123d560cd6 100755 --- a/src/cmdlib.sh +++ b/src/cmdlib.sh @@ -733,9 +733,6 @@ runvm() { # and include all GPG keys find /etc/pki/rpm-gpg/ -type f >> "${vmpreparedir}/hostfiles" - local cmdoutput - cmdoutput=${tmp_builddir}/cmd-output.txt - # the reason we do a heredoc here is so that the var substition takes # place immediately instead of having to proxy them through to the VM cat > "${vmpreparedir}/init" <> "${tmp_builddir}"/cmd.sh done - touch "${runvm_console}" "${cmdoutput}" - setpriv --pdeathsig SIGTERM -- tail -qF "${cmdoutput}" --pid $$ & - local tail_pid=$! + touch "${runvm_console}" # There seems to be some false positives in shellcheck # https://github.com/koalaman/shellcheck/issues/2217 @@ -826,6 +821,8 @@ EOF if [ -z "${RUNVM_SHELL:-}" ]; then if ! "${kola_args[@]}" -- "${base_qemu_args[@]}" \ + -device virtserialport,chardev=virtioserial0,name=cosa-cmdout \ + -chardev stdio,id=virtioserial0 \ "${qemu_args[@]}"; then cat "${runvm_console}" fatal "Failed to run 'kola qemuexec'" @@ -845,13 +842,6 @@ EOF fi rc="$(cat "${rc_file}")" - # XXX: this is for debugging temporarily - if [ -n "${TAIL_SLEEP:-}" ]; then - sleep "${TAIL_SLEEP}" - fi - # cleanup tail before nuking dir containing file it's following - kill "$tail_pid" - if [ -n "${cleanup_tmpdir:-}" ]; then rm -rf "${tmp_builddir}" unset tmp_builddir From c5adc99aff48e0d55db89e898cd5366bf8ed91e4 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 7 May 2024 16:12:24 -0400 Subject: [PATCH 2/2] cmdlib.sh: feed `/dev/zero` as qemu stdin This is a follow-up to 79b15c89d ("cmdlib.sh: go back to using `tail -F` for command output") which was subsequently reverted. To summarize, it seems like in QEMU v8.2 (in f40), the guest sometimes would hang when writing over virtio-serial if the device is hooked up to the QEMU's stdio. In testing, removing the `<&-` hack to close QEMU's stdin fixed it for CoreOS CI but not Prow: https://github.com/coreos/coreos-assembler/pull/3785#issuecomment-2087342748 I think I've narrowed it down to CoreOS CI (i.e. Jenkins) allocating a tty and Prow not. When stdin is not a tty, QEMU would immediately gets EOF if it tries to read anything. I'm not sure exactly what happens, but I think the virtio-serial hang is linked to this (even though there's no userspace code in the guest trying to read from the virtio-serial port). Work around this by explicitly feeding `/dev/zero` to QEMU's stdin. --- src/cmdlib.sh | 2 +- src/secex-genprotimgvm-scripts/runvm.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmdlib.sh b/src/cmdlib.sh index 123d560cd6..c183beaf8d 100755 --- a/src/cmdlib.sh +++ b/src/cmdlib.sh @@ -823,7 +823,7 @@ EOF if ! "${kola_args[@]}" -- "${base_qemu_args[@]}" \ -device virtserialport,chardev=virtioserial0,name=cosa-cmdout \ -chardev stdio,id=virtioserial0 \ - "${qemu_args[@]}"; then + "${qemu_args[@]}" < /dev/zero; then # qemu hangs if it has nothing to read on stdin cat "${runvm_console}" fatal "Failed to run 'kola qemuexec'" fi diff --git a/src/secex-genprotimgvm-scripts/runvm.sh b/src/secex-genprotimgvm-scripts/runvm.sh index c90a2c604d..a5361a1021 100644 --- a/src/secex-genprotimgvm-scripts/runvm.sh +++ b/src/secex-genprotimgvm-scripts/runvm.sh @@ -56,7 +56,7 @@ else fi if ! "${kola_args[@]}" -- "${base_qemu_args[@]}" \ - "${qemu_args[@]}" <&-; then # the <&- here closes stdin otherwise qemu waits forever + "${qemu_args[@]}" < /dev/zero; then # qemu hangs if it has nothing to read on stdin cat "${runvm_console}" echo "Failed to run 'kola qemuexec'" exit 1