diff mbox series

[bpf-next] selftests/bpf: Propagate error code of the command to vmtest.sh

Message ID 20210225161947.1778590-1-kpsingh@kernel.org (mailing list archive)
State Accepted
Commit 2854436612c4d2606c9246ce2976ab6634276337
Delegated to: BPF
Headers show
Series [bpf-next] selftests/bpf: Propagate error code of the command to vmtest.sh | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 7 maintainers not CCed: netdev@vger.kernel.org shuah@kernel.org linux-kselftest@vger.kernel.org songliubraving@fb.com yhs@fb.com kafai@fb.com john.fastabend@gmail.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 79 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

KP Singh Feb. 25, 2021, 4:19 p.m. UTC 
From: KP Singh <kpsingh@google.com>

When vmtest.sh ran a command in a VM, it did not record or propagate the
error code of the command. This made the script less "script-able". The
script now saves the error code of the said command in a file in the VM,
copies the file back to the host and (when available) uses this error
code instead of its own.

Signed-off-by: KP Singh <kpsingh@google.com>
---
 tools/testing/selftests/bpf/vmtest.sh | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

Comments

Andrii Nakryiko Feb. 26, 2021, 9:18 p.m. UTC | #1 
On Thu, Feb 25, 2021 at 8:19 AM KP Singh <kpsingh@kernel.org> wrote:
>
> From: KP Singh <kpsingh@google.com>
>
> When vmtest.sh ran a command in a VM, it did not record or propagate the
> error code of the command. This made the script less "script-able". The
> script now saves the error code of the said command in a file in the VM,
> copies the file back to the host and (when available) uses this error
> code instead of its own.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---

Worked for me, thanks! Applied to bpf-next.

>  tools/testing/selftests/bpf/vmtest.sh | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh
> index 26ae8d0b6ce3..22554894db99 100755
> --- a/tools/testing/selftests/bpf/vmtest.sh
> +++ b/tools/testing/selftests/bpf/vmtest.sh
> @@ -17,6 +17,9 @@ KCONFIG_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vm
>  KCONFIG_API_URL="https://api.github.com/repos/libbpf/libbpf/contents/travis-ci/vmtest/configs/latest.config"
>  INDEX_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/INDEX"
>  NUM_COMPILE_JOBS="$(nproc)"
> +LOG_FILE_BASE="$(date +"bpf_selftests.%Y-%m-%d_%H-%M-%S")"
> +LOG_FILE="${LOG_FILE_BASE}.log"
> +EXIT_STATUS_FILE="${LOG_FILE_BASE}.exit_status"
>
>  usage()
>  {
> @@ -146,7 +149,6 @@ update_init_script()
>         local init_script_dir="${OUTPUT_DIR}/${MOUNT_DIR}/etc/rcS.d"
>         local init_script="${init_script_dir}/S50-startup"
>         local command="$1"
> -       local log_file="$2"
>
>         mount_image
>
> @@ -163,11 +165,16 @@ EOF
>         sudo bash -c "cat >${init_script}" <<EOF
>  #!/bin/bash
>
> +# Have a default value in the exit status file
> +# incase the VM is forcefully stopped.
> +echo "130" > "/root/${EXIT_STATUS_FILE}"
> +
>  {
>         cd /root/bpf
>         echo ${command}
>         stdbuf -oL -eL ${command}
> -} 2>&1 | tee /root/${log_file}
> +       echo "\$?" > "/root/${EXIT_STATUS_FILE}"
> +} 2>&1 | tee "/root/${LOG_FILE}"
>  poweroff -f
>  EOF
>
> @@ -221,10 +228,12 @@ EOF
>  copy_logs()
>  {
>         local mount_dir="${OUTPUT_DIR}/${MOUNT_DIR}"
> -       local log_file="${mount_dir}/root/$1"
> +       local log_file="${mount_dir}/root/${LOG_FILE}"
> +       local exit_status_file="${mount_dir}/root/${EXIT_STATUS_FILE}"
>
>         mount_image
>         sudo cp ${log_file} "${OUTPUT_DIR}"
> +       sudo cp ${exit_status_file} "${OUTPUT_DIR}"
>         sudo rm -f ${log_file}
>         unmount_image
>  }
> @@ -263,7 +272,6 @@ main()
>  {
>         local script_dir="$(cd -P -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)"
>         local kernel_checkout=$(realpath "${script_dir}"/../../../../)
> -       local log_file="$(date +"bpf_selftests.%Y-%m-%d_%H-%M-%S.log")"
>         # By default the script searches for the kernel in the checkout directory but
>         # it also obeys environment variables O= and KBUILD_OUTPUT=
>         local kernel_bzimage="${kernel_checkout}/${X86_BZIMAGE}"
> @@ -347,19 +355,23 @@ main()
>         fi
>
>         update_selftests "${kernel_checkout}" "${make_command}"
> -       update_init_script "${command}" "${log_file}"
> +       update_init_script "${command}"
>         run_vm "${kernel_bzimage}"
> -       copy_logs "${log_file}"
> -       echo "Logs saved in ${OUTPUT_DIR}/${log_file}"
> +       copy_logs
> +       echo "Logs saved in ${OUTPUT_DIR}/${LOG_FILE}"
>  }
>
>  catch()
>  {
>         local exit_code=$1
> +       local exit_status_file="${OUTPUT_DIR}/${EXIT_STATUS_FILE}"
>         # This is just a cleanup and the directory may
>         # have already been unmounted. So, don't let this
>         # clobber the error code we intend to return.
>         unmount_image || true
> +       if [[ -f "${exit_status_file}" ]]; then
> +               exit_code="$(cat ${exit_status_file})"
> +       fi
>         exit ${exit_code}
>  }
>
> --
> 2.30.1.766.gb4fecdf3b7-goog
>
patchwork-bot+netdevbpf@kernel.org Feb. 26, 2021, 9:20 p.m. UTC | #2 
Hello:

This patch was applied to bpf/bpf-next.git (refs/heads/master):

On Thu, 25 Feb 2021 16:19:47 +0000 you wrote:
> From: KP Singh <kpsingh@google.com>
> 
> When vmtest.sh ran a command in a VM, it did not record or propagate the
> error code of the command. This made the script less "script-able". The
> script now saves the error code of the said command in a file in the VM,
> copies the file back to the host and (when available) uses this error
> code instead of its own.
> 
> [...]

Here is the summary with links:
  - [bpf-next] selftests/bpf: Propagate error code of the command to vmtest.sh
    https://git.kernel.org/bpf/bpf-next/c/2854436612c4

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh
index 26ae8d0b6ce3..22554894db99 100755
--- a/tools/testing/selftests/bpf/vmtest.sh
+++ b/tools/testing/selftests/bpf/vmtest.sh
@@ -17,6 +17,9 @@  KCONFIG_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vm
 KCONFIG_API_URL="https://api.github.com/repos/libbpf/libbpf/contents/travis-ci/vmtest/configs/latest.config"
 INDEX_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/INDEX"
 NUM_COMPILE_JOBS="$(nproc)"
+LOG_FILE_BASE="$(date +"bpf_selftests.%Y-%m-%d_%H-%M-%S")"
+LOG_FILE="${LOG_FILE_BASE}.log"
+EXIT_STATUS_FILE="${LOG_FILE_BASE}.exit_status"
 
 usage()
 {
@@ -146,7 +149,6 @@  update_init_script()
 	local init_script_dir="${OUTPUT_DIR}/${MOUNT_DIR}/etc/rcS.d"
 	local init_script="${init_script_dir}/S50-startup"
 	local command="$1"
-	local log_file="$2"
 
 	mount_image
 
@@ -163,11 +165,16 @@  EOF
 	sudo bash -c "cat >${init_script}" <<EOF
 #!/bin/bash
 
+# Have a default value in the exit status file
+# incase the VM is forcefully stopped.
+echo "130" > "/root/${EXIT_STATUS_FILE}"
+
 {
 	cd /root/bpf
 	echo ${command}
 	stdbuf -oL -eL ${command}
-} 2>&1 | tee /root/${log_file}
+	echo "\$?" > "/root/${EXIT_STATUS_FILE}"
+} 2>&1 | tee "/root/${LOG_FILE}"
 poweroff -f
 EOF
 
@@ -221,10 +228,12 @@  EOF
 copy_logs()
 {
 	local mount_dir="${OUTPUT_DIR}/${MOUNT_DIR}"
-	local log_file="${mount_dir}/root/$1"
+	local log_file="${mount_dir}/root/${LOG_FILE}"
+	local exit_status_file="${mount_dir}/root/${EXIT_STATUS_FILE}"
 
 	mount_image
 	sudo cp ${log_file} "${OUTPUT_DIR}"
+	sudo cp ${exit_status_file} "${OUTPUT_DIR}"
 	sudo rm -f ${log_file}
 	unmount_image
 }
@@ -263,7 +272,6 @@  main()
 {
 	local script_dir="$(cd -P -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)"
 	local kernel_checkout=$(realpath "${script_dir}"/../../../../)
-	local log_file="$(date +"bpf_selftests.%Y-%m-%d_%H-%M-%S.log")"
 	# By default the script searches for the kernel in the checkout directory but
 	# it also obeys environment variables O= and KBUILD_OUTPUT=
 	local kernel_bzimage="${kernel_checkout}/${X86_BZIMAGE}"
@@ -347,19 +355,23 @@  main()
 	fi
 
 	update_selftests "${kernel_checkout}" "${make_command}"
-	update_init_script "${command}" "${log_file}"
+	update_init_script "${command}"
 	run_vm "${kernel_bzimage}"
-	copy_logs "${log_file}"
-	echo "Logs saved in ${OUTPUT_DIR}/${log_file}"
+	copy_logs
+	echo "Logs saved in ${OUTPUT_DIR}/${LOG_FILE}"
 }
 
 catch()
 {
 	local exit_code=$1
+	local exit_status_file="${OUTPUT_DIR}/${EXIT_STATUS_FILE}"
 	# This is just a cleanup and the directory may
 	# have already been unmounted. So, don't let this
 	# clobber the error code we intend to return.
 	unmount_image || true
+	if [[ -f "${exit_status_file}" ]]; then
+		exit_code="$(cat ${exit_status_file})"
+	fi
 	exit ${exit_code}
 }