Hey all,
This patch series refactors the vsock selftest VM infrastructure to improve test run times, reduce false-positives, improve logging generally, and fix several bugs.
It also prepares for future tests which make heavy usage of these refactored functions and have new requirements such as simultaneous QEMU processes.
These patches were broken off from this prior series: https://lore.kernel.org/all/20251021-vsock-vmtest-v7-0-0661b7b6f081@meta.com...
To: Stefano Garzarella sgarzare@redhat.com To: Shuah Khan shuah@kernel.org To: Jakub Kicinski kuba@kernel.org To: Bobby Eshleman bobbyeshleman@gmail.com Cc: virtualization@lists.linux.dev Cc: netdev@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
--- Bobby Eshleman (12): selftests/vsock: improve logging in vmtest.sh selftests/vsock: make wait_for_listener() work even if pipefail is on selftests/vsock: reuse logic for vsock_test through wrapper functions selftests/vsock: avoid multi-VM pidfile collisions with QEMU selftests/vsock: do not unconditionally die if qemu fails selftests/vsock: speed up tests by reducing the QEMU pidfile timeout selftests/vsock: add check_result() for pass/fail counting selftests/vsock: identify and execute tests that can re-use VM selftests/vsock: add BUILD=0 definition selftests/vsock: avoid false-positives when checking dmesg selftests/vsock: add 1.37 to tested virtme-ng versions selftests/vsock: add vsock_loopback module loading
tools/testing/selftests/vsock/vmtest.sh | 345 +++++++++++++++++++++----------- 1 file changed, 227 insertions(+), 118 deletions(-) --- base-commit: 962ac5ca99a5c3e7469215bf47572440402dfd59 change-id: 20251021-vsock-selftests-fixes-and-improvements-057440ffb2fa
Best regards,
From: Bobby Eshleman bobbyeshleman@meta.com
Improve usability of logging functions. Remove the test name prefix from logging functions so that logging calls can be made deeper into the call stack without passing down the test name or setting some global. Teach log function to accept a LOG_PREFIX variable to avoid unnecessary argument shifting.
Remove log_setup() and instead use log_host(). The host/guest prefixes are useful to show whether a failure happened on the guest or host side, but "setup" doesn't really give additional useful information. Since all log_setup() calls happen on the host, lets just use log_host() instead.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- Changes from previous series: - do not use log levels, keep as on/off switch, after revising the other patch series the levels became unnecessary. --- tools/testing/selftests/vsock/vmtest.sh | 69 ++++++++++++++------------------- 1 file changed, 29 insertions(+), 40 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index edacebfc1632..561600814bef 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -271,60 +271,51 @@ EOF
host_wait_for_listener() { wait_for_listener "${TEST_HOST_PORT_LISTENER}" "${WAIT_PERIOD}" "${WAIT_PERIOD_MAX}" -} - -__log_stdin() { - cat | awk '{ printf "%s:\t%s\n","'"${prefix}"'", $0 }' -}
-__log_args() { - echo "$*" | awk '{ printf "%s:\t%s\n","'"${prefix}"'", $0 }' }
log() { - local prefix="$1" + local redirect + local prefix
- shift - local redirect= if [[ ${VERBOSE} -eq 0 ]]; then redirect=/dev/null else redirect=/dev/stdout fi
+ prefix="${LOG_PREFIX:-}" + if [[ "$#" -eq 0 ]]; then - __log_stdin | tee -a "${LOG}" > ${redirect} + if [[ -n "${prefix}" ]]; then + cat | awk -v prefix="${prefix}" '{printf "%s: %s\n", prefix, $0}' + else + cat + fi else - __log_args "$@" | tee -a "${LOG}" > ${redirect} - fi -} - -log_setup() { - log "setup" "$@" + if [[ -n "${prefix}" ]]; then + echo "${prefix}: " "$@" + else + echo "$@" + fi + fi | tee -a "${LOG}" > ${redirect} }
log_host() { - local testname=$1 - - shift - log "test:${testname}:host" "$@" + LOG_PREFIX=host log $@ }
log_guest() { - local testname=$1 - - shift - log "test:${testname}:guest" "$@" + LOG_PREFIX=guest log $@ }
test_vm_server_host_client() { - local testname="${FUNCNAME[0]#test_}"
vm_ssh -- "${VSOCK_TEST}" \ --mode=server \ --control-port="${TEST_GUEST_PORT}" \ --peer-cid=2 \ - 2>&1 | log_guest "${testname}" & + 2>&1 | log_guest &
vm_wait_for_listener "${TEST_GUEST_PORT}"
@@ -332,18 +323,17 @@ test_vm_server_host_client() { --mode=client \ --control-host=127.0.0.1 \ --peer-cid="${VSOCK_CID}" \ - --control-port="${TEST_HOST_PORT}" 2>&1 | log_host "${testname}" + --control-port="${TEST_HOST_PORT}" 2>&1 | log_host
return $? }
test_vm_client_host_server() { - local testname="${FUNCNAME[0]#test_}"
${VSOCK_TEST} \ --mode "server" \ --control-port "${TEST_HOST_PORT_LISTENER}" \ - --peer-cid "${VSOCK_CID}" 2>&1 | log_host "${testname}" & + --peer-cid "${VSOCK_CID}" 2>&1 | log_host &
host_wait_for_listener
@@ -351,19 +341,18 @@ test_vm_client_host_server() { --mode=client \ --control-host=10.0.2.2 \ --peer-cid=2 \ - --control-port="${TEST_HOST_PORT_LISTENER}" 2>&1 | log_guest "${testname}" + --control-port="${TEST_HOST_PORT_LISTENER}" 2>&1 | log_guest
return $? }
test_vm_loopback() { - local testname="${FUNCNAME[0]#test_}" local port=60000 # non-forwarded local port
vm_ssh -- "${VSOCK_TEST}" \ --mode=server \ --control-port="${port}" \ - --peer-cid=1 2>&1 | log_guest "${testname}" & + --peer-cid=1 2>&1 | log_guest &
vm_wait_for_listener "${port}"
@@ -371,7 +360,7 @@ test_vm_loopback() { --mode=client \ --control-host="127.0.0.1" \ --control-port="${port}" \ - --peer-cid=1 2>&1 | log_guest "${testname}" + --peer-cid=1 2>&1 | log_guest
return $? } @@ -399,25 +388,25 @@ run_test() {
host_oops_cnt_after=$(dmesg | grep -i 'Oops' | wc -l) if [[ ${host_oops_cnt_after} -gt ${host_oops_cnt_before} ]]; then - echo "FAIL: kernel oops detected on host" | log_host "${name}" + echo "FAIL: kernel oops detected on host" | log_host rc=$KSFT_FAIL fi
host_warn_cnt_after=$(dmesg --level=warn | wc -l) if [[ ${host_warn_cnt_after} -gt ${host_warn_cnt_before} ]]; then - echo "FAIL: kernel warning detected on host" | log_host "${name}" + echo "FAIL: kernel warning detected on host" | log_host rc=$KSFT_FAIL fi
vm_oops_cnt_after=$(vm_ssh -- dmesg | grep -i 'Oops' | wc -l) if [[ ${vm_oops_cnt_after} -gt ${vm_oops_cnt_before} ]]; then - echo "FAIL: kernel oops detected on vm" | log_host "${name}" + echo "FAIL: kernel oops detected on vm" | log_host rc=$KSFT_FAIL fi
vm_warn_cnt_after=$(vm_ssh -- dmesg --level=warn | wc -l) if [[ ${vm_warn_cnt_after} -gt ${vm_warn_cnt_before} ]]; then - echo "FAIL: kernel warning detected on vm" | log_host "${name}" + echo "FAIL: kernel warning detected on vm" | log_host rc=$KSFT_FAIL fi
@@ -452,10 +441,10 @@ handle_build
echo "1..${#ARGS[@]}"
-log_setup "Booting up VM" +log_host "Booting up VM" vm_start vm_wait_for_ssh -log_setup "VM booted up" +log_host "VM booted up"
cnt_pass=0 cnt_fail=0
On Wed, Oct 22, 2025 at 06:00:05PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Improve usability of logging functions. Remove the test name prefix from logging functions so that logging calls can be made deeper into the call stack without passing down the test name or setting some global. Teach log function to accept a LOG_PREFIX variable to avoid unnecessary argument shifting.
Remove log_setup() and instead use log_host(). The host/guest prefixes are useful to show whether a failure happened on the guest or host side, but "setup" doesn't really give additional useful information. Since all log_setup() calls happen on the host, lets just use log_host() instead.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
...
log() {
- local prefix="$1"
- local redirect
- local prefix
- shift
- local redirect= if [[ ${VERBOSE} -eq 0 ]]; then redirect=/dev/null else redirect=/dev/stdout fi
- prefix="${LOG_PREFIX:-}"
- if [[ "$#" -eq 0 ]]; then
__log_stdin | tee -a "${LOG}" > ${redirect}
if [[ -n "${prefix}" ]]; thencat | awk -v prefix="${prefix}" '{printf "%s: %s\n", prefix, $0}'
FIWIIW, I would drop cat from this line.
elsecatfi
__log_args "$@" | tee -a "${LOG}" > ${redirect}- fi
-}
-log_setup() {
- log "setup" "$@"
if [[ -n "${prefix}" ]]; thenecho "${prefix}: " "$@"elseecho "$@"fi- fi | tee -a "${LOG}" > ${redirect}
} log_host() {
- local testname=$1
- shift
- log "test:${testname}:host" "$@"
- LOG_PREFIX=host log $@
shellcheck suggests keeping the quoting of $@. This seems reasonable to me. Although in practice I don't think it will change the behaviour of this script.
} log_host log_guest() {
- local testname=$1
- shift
- log "test:${testname}:guest" "$@"
- LOG_PREFIX=guest log $@
shellcheck also points out that log_guest is never passed arguments, so $@ can be dropped. If you prefer to keep it then, as per log_host, it seems reasonable for it to be quoted.
}
...
On Mon, Oct 27, 2025 at 04:45:27PM +0000, Simon Horman wrote:
log() {
- local prefix="$1"
- local redirect
- local prefix
- shift
- local redirect= if [[ ${VERBOSE} -eq 0 ]]; then redirect=/dev/null else redirect=/dev/stdout fi
- prefix="${LOG_PREFIX:-}"
- if [[ "$#" -eq 0 ]]; then
__log_stdin | tee -a "${LOG}" > ${redirect}
if [[ -n "${prefix}" ]]; thencat | awk -v prefix="${prefix}" '{printf "%s: %s\n", prefix, $0}'FIWIIW, I would drop cat from this line.
sgtm!
elsecatfi
__log_args "$@" | tee -a "${LOG}" > ${redirect}- fi
-}
-log_setup() {
- log "setup" "$@"
if [[ -n "${prefix}" ]]; thenecho "${prefix}: " "$@"elseecho "$@"fi- fi | tee -a "${LOG}" > ${redirect}
} log_host() {
- local testname=$1
- shift
- log "test:${testname}:host" "$@"
- LOG_PREFIX=host log $@
shellcheck suggests keeping the quoting of $@. This seems reasonable to me. Although in practice I don't think it will change the behaviour of this script.
Ah right, makes sense to me.
} log_host log_guest() {
- local testname=$1
- shift
- log "test:${testname}:guest" "$@"
- LOG_PREFIX=guest log $@
shellcheck also points out that log_guest is never passed arguments, so $@ can be dropped. If you prefer to keep it then, as per log_host, it seems reasonable for it to be quoted.
Quoting it sounds best to me, in keeping with log_host().
Thanks, Bobby
From: Bobby Eshleman bobbyeshleman@meta.com
Save/restore pipefail to not mistakenly trip the if-condition in wait_for_listener().
awk doesn't gracefully handle SIGPIPE with a non-zero exit code, so grep exiting upon finding a match causes false-positives when the pipefail option is used. This will enable pipefail usage, so that we can losing failures when piping test output into log() functions.
Fixes: a4a65c6fe08b ("selftests/vsock: add initial vmtest.sh for vsock") Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index 561600814bef..ec3ff443f49a 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -243,6 +243,7 @@ wait_for_listener() local port=$1 local interval=$2 local max_intervals=$3 + local old_pipefail local protocol=tcp local pattern local i @@ -251,6 +252,13 @@ wait_for_listener()
# for tcp protocol additionally check the socket state [ "${protocol}" = "tcp" ] && pattern="${pattern}0A" + + # 'grep -q' exits on match, sending SIGPIPE to 'awk', which exits with + # an error, causing the if-condition to fail when pipefail is set. + # Instead, temporarily disable pipefail and restore it later. + old_pipefail=$(set -o | awk '/^pipefail[[:space:]]+(on|off)$/{print $2}') + set +o pipefail + for i in $(seq "${max_intervals}"); do if awk '{print $2" "$4}' /proc/net/"${protocol}"* | \ grep -q "${pattern}"; then @@ -258,6 +266,10 @@ wait_for_listener() fi sleep "${interval}" done + + if [[ "${old_pipefail}" == on ]]; then + set -o pipefail + fi }
vm_wait_for_listener() {
On Wed, Oct 22, 2025 at 06:00:06PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Save/restore pipefail to not mistakenly trip the if-condition in wait_for_listener().
awk doesn't gracefully handle SIGPIPE with a non-zero exit code, so grep exiting upon finding a match causes false-positives when the pipefail option is used. This will enable pipefail usage, so that we can losing failures when piping test output into log() functions.
Fixes: a4a65c6fe08b ("selftests/vsock: add initial vmtest.sh for vsock") Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
tools/testing/selftests/vsock/vmtest.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index 561600814bef..ec3ff443f49a 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -243,6 +243,7 @@ wait_for_listener() local port=$1 local interval=$2 local max_intervals=$3
- local old_pipefail local protocol=tcp local pattern local i
@@ -251,6 +252,13 @@ wait_for_listener() # for tcp protocol additionally check the socket state [ "${protocol}" = "tcp" ] && pattern="${pattern}0A"
- # 'grep -q' exits on match, sending SIGPIPE to 'awk', which exits with
- # an error, causing the if-condition to fail when pipefail is set.
- # Instead, temporarily disable pipefail and restore it later.
- old_pipefail=$(set -o | awk '/^pipefail[[:space:]]+(on|off)$/{print $2}')
- set +o pipefail
- for i in $(seq "${max_intervals}"); do if awk '{print $2" "$4}' /proc/net/"${protocol}"* | \ grep -q "${pattern}"; then
Hi Bobby,
I agree this is a problem. But I'm wondering if you considered moving the pattern matching into the awk script. I'm no awk expert. But suspect that would lead to a more elegant solution.
@@ -258,6 +266,10 @@ wait_for_listener() fi sleep "${interval}" done
- if [[ "${old_pipefail}" == on ]]; then
set -o pipefail- fi
} vm_wait_for_listener() {
-- 2.47.3
On Mon, Oct 27, 2025 at 04:48:28PM +0000, Simon Horman wrote:
On Wed, Oct 22, 2025 at 06:00:06PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Save/restore pipefail to not mistakenly trip the if-condition in wait_for_listener().
awk doesn't gracefully handle SIGPIPE with a non-zero exit code, so grep exiting upon finding a match causes false-positives when the pipefail option is used. This will enable pipefail usage, so that we can losing failures when piping test output into log() functions.
Fixes: a4a65c6fe08b ("selftests/vsock: add initial vmtest.sh for vsock") Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
tools/testing/selftests/vsock/vmtest.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index 561600814bef..ec3ff443f49a 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -243,6 +243,7 @@ wait_for_listener() local port=$1 local interval=$2 local max_intervals=$3
- local old_pipefail local protocol=tcp local pattern local i
@@ -251,6 +252,13 @@ wait_for_listener() # for tcp protocol additionally check the socket state [ "${protocol}" = "tcp" ] && pattern="${pattern}0A"
- # 'grep -q' exits on match, sending SIGPIPE to 'awk', which exits with
- # an error, causing the if-condition to fail when pipefail is set.
- # Instead, temporarily disable pipefail and restore it later.
- old_pipefail=$(set -o | awk '/^pipefail[[:space:]]+(on|off)$/{print $2}')
- set +o pipefail
- for i in $(seq "${max_intervals}"); do if awk '{print $2" "$4}' /proc/net/"${protocol}"* | \ grep -q "${pattern}"; then
Hi Bobby,
I agree this is a problem. But I'm wondering if you considered moving the pattern matching into the awk script. I'm no awk expert. But suspect that would lead to a more elegant solution.
I bet you are right.
Playing around with awk, I find that this seems to work:
$ pattern=":$(printf '%04X' ${port}) 0A" $ awk -v pattern="${pattern}" 'BEGIN {rc=1} $2" "$4 ~ pattern {rc=0} END {exit rc}' /proc/net/tcp && echo FOUND
I think it beats doing the save/restore on pipefail?
Best, Bobby
On Mon, Oct 27, 2025 at 10:55:52AM -0700, Bobby Eshleman wrote:
On Mon, Oct 27, 2025 at 04:48:28PM +0000, Simon Horman wrote:
On Wed, Oct 22, 2025 at 06:00:06PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Save/restore pipefail to not mistakenly trip the if-condition in wait_for_listener().
awk doesn't gracefully handle SIGPIPE with a non-zero exit code, so grep exiting upon finding a match causes false-positives when the pipefail option is used. This will enable pipefail usage, so that we can losing failures when piping test output into log() functions.
Fixes: a4a65c6fe08b ("selftests/vsock: add initial vmtest.sh for vsock") Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
tools/testing/selftests/vsock/vmtest.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index 561600814bef..ec3ff443f49a 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -243,6 +243,7 @@ wait_for_listener() local port=$1 local interval=$2 local max_intervals=$3
- local old_pipefail local protocol=tcp local pattern local i
@@ -251,6 +252,13 @@ wait_for_listener() # for tcp protocol additionally check the socket state [ "${protocol}" = "tcp" ] && pattern="${pattern}0A"
- # 'grep -q' exits on match, sending SIGPIPE to 'awk', which exits with
- # an error, causing the if-condition to fail when pipefail is set.
- # Instead, temporarily disable pipefail and restore it later.
- old_pipefail=$(set -o | awk '/^pipefail[[:space:]]+(on|off)$/{print $2}')
- set +o pipefail
- for i in $(seq "${max_intervals}"); do if awk '{print $2" "$4}' /proc/net/"${protocol}"* | \ grep -q "${pattern}"; then
Hi Bobby,
I agree this is a problem. But I'm wondering if you considered moving the pattern matching into the awk script. I'm no awk expert. But suspect that would lead to a more elegant solution.
I bet you are right.
Playing around with awk, I find that this seems to work:
$ pattern=":$(printf '%04X' ${port}) 0A" $ awk -v pattern="${pattern}" 'BEGIN {rc=1} $2" "$4 ~ pattern {rc=0} END {exit rc}' /proc/net/tcp && echo FOUND
I think it beats doing the save/restore on pipefail?
Yes, I think so.
From: Bobby Eshleman bobbyeshleman@meta.com
Add wrapper functions vm_vsock_test() and host_vsock_test() to invoke the vsock_test binary. This encapsulates several items of repeat logic, such as waiting for the server to reach listening state and enabling/disabling the bash option pipefail to avoid pipe-style logging from hiding failures.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 131 ++++++++++++++++++++++---------- 1 file changed, 92 insertions(+), 39 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index ec3ff443f49a..29b36b4d301d 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -283,7 +283,78 @@ EOF
host_wait_for_listener() { wait_for_listener "${TEST_HOST_PORT_LISTENER}" "${WAIT_PERIOD}" "${WAIT_PERIOD_MAX}" +} + +vm_vsock_test() { + local host=$1 + local cid=$2 + local port=$3 + local rc + + set -o pipefail + if [[ "${host}" != server ]]; then + # log output and use pipefail to respect vsock_test errors + vm_ssh -- "${VSOCK_TEST}" \ + --mode=client \ + --control-host="${host}" \ + --peer-cid="${cid}" \ + --control-port="${port}" \ + 2>&1 | log_guest + rc=$? + else + # log output and use pipefail to respect vsock_test errors + vm_ssh -- "${VSOCK_TEST}" \ + --mode=server \ + --peer-cid="${cid}" \ + --control-port="${port}" \ + 2>&1 | log_guest & + rc=$? + + if [[ $rc -ne 0 ]]; then + set +o pipefail + return $rc + fi + + vm_wait_for_listener "${port}" + rc=$? + fi + set +o pipefail
+ return $rc +} + +host_vsock_test() { + local host=$1 + local cid=$2 + local port=$3 + local rc + + # log output and use pipefail to respect vsock_test errors + set -o pipefail + if [[ "${host}" != server ]]; then + ${VSOCK_TEST} \ + --mode=client \ + --peer-cid="${cid}" \ + --control-host="${host}" \ + --control-port="${port}" 2>&1 | log_host + rc=$? + else + ${VSOCK_TEST} \ + --mode=server \ + --peer-cid="${cid}" \ + --control-port="${port}" 2>&1 | log_host & + rc=$? + + if [[ $rc -ne 0 ]]; then + return $rc + fi + + host_wait_for_listener "${port}" "${WAIT_PERIOD}" "${WAIT_PERIOD_MAX}" + rc=$? + fi + set +o pipefail + + return $rc }
log() { @@ -322,59 +393,41 @@ log_guest() { }
test_vm_server_host_client() { + if ! vm_vsock_test "server" 2 "${TEST_GUEST_PORT}"; then + return "${KSFT_FAIL}" + fi
- vm_ssh -- "${VSOCK_TEST}" \ - --mode=server \ - --control-port="${TEST_GUEST_PORT}" \ - --peer-cid=2 \ - 2>&1 | log_guest & - - vm_wait_for_listener "${TEST_GUEST_PORT}" - - ${VSOCK_TEST} \ - --mode=client \ - --control-host=127.0.0.1 \ - --peer-cid="${VSOCK_CID}" \ - --control-port="${TEST_HOST_PORT}" 2>&1 | log_host + if ! host_vsock_test "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"; then + return "${KSFT_FAIL}" + fi
- return $? + return "${KSFT_PASS}" }
test_vm_client_host_server() { + if ! host_vsock_test "server" "${VSOCK_CID}" "${TEST_HOST_PORT_LISTENER}"; then + return "${KSFT_FAIL}" + fi
- ${VSOCK_TEST} \ - --mode "server" \ - --control-port "${TEST_HOST_PORT_LISTENER}" \ - --peer-cid "${VSOCK_CID}" 2>&1 | log_host & - - host_wait_for_listener - - vm_ssh -- "${VSOCK_TEST}" \ - --mode=client \ - --control-host=10.0.2.2 \ - --peer-cid=2 \ - --control-port="${TEST_HOST_PORT_LISTENER}" 2>&1 | log_guest + if ! vm_vsock_test "10.0.2.2" 2 "${TEST_HOST_PORT_LISTENER}"; then + return "${KSFT_FAIL}" + fi
- return $? + return "${KSFT_PASS}" }
test_vm_loopback() { local port=60000 # non-forwarded local port
- vm_ssh -- "${VSOCK_TEST}" \ - --mode=server \ - --control-port="${port}" \ - --peer-cid=1 2>&1 | log_guest & - - vm_wait_for_listener "${port}" + if ! vm_vsock_test "server" 1 "${port}"; then + return "${KSFT_FAIL}" + fi
- vm_ssh -- "${VSOCK_TEST}" \ - --mode=client \ - --control-host="127.0.0.1" \ - --control-port="${port}" \ - --peer-cid=1 2>&1 | log_guest + if ! vm_vsock_test "127.0.0.1" 1 "${port}"; then + return "${KSFT_FAIL}" + fi
- return $? + return "${KSFT_PASS}" }
run_test() {
On Wed, Oct 22, 2025 at 06:00:07PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Add wrapper functions vm_vsock_test() and host_vsock_test() to invoke the vsock_test binary. This encapsulates several items of repeat logic, such as waiting for the server to reach listening state and enabling/disabling the bash option pipefail to avoid pipe-style logging from hiding failures.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
shellcheck has some (new) things to say about this patch too. Could you take a look over them?
...
On Mon, Oct 27, 2025 at 04:58:02PM +0000, Simon Horman wrote:
On Wed, Oct 22, 2025 at 06:00:07PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Add wrapper functions vm_vsock_test() and host_vsock_test() to invoke the vsock_test binary. This encapsulates several items of repeat logic, such as waiting for the server to reach listening state and enabling/disabling the bash option pipefail to avoid pipe-style logging from hiding failures.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
shellcheck has some (new) things to say about this patch too. Could you take a look over them?
...
No problem, will do.
Thanks for the reviews!
Best, Bobby
On Mon, Oct 27, 2025 at 11:01:36AM -0700, Bobby Eshleman wrote:
On Mon, Oct 27, 2025 at 04:58:02PM +0000, Simon Horman wrote:
On Wed, Oct 22, 2025 at 06:00:07PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Add wrapper functions vm_vsock_test() and host_vsock_test() to invoke the vsock_test binary. This encapsulates several items of repeat logic, such as waiting for the server to reach listening state and enabling/disabling the bash option pipefail to avoid pipe-style logging from hiding failures.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
shellcheck has some (new) things to say about this patch too. Could you take a look over them?
...
It looks like the errors are SC2317 and SC2119, but are false-positives. Invoking a program as a variable (e.g., "${VSOCK_TEST}") is tripping SC2317 (command unreachable), and SC2119 is due to log_{guest,host}() being passed zero arguments (logging its stdin instead).
I also see that SC2317 has many other false positives elsewhere in the file (80+), reporting even lines like `rm "${QEMU_PIDFILE}"` as unreachable. I wonder if we should add a patch to this series to disable this check at the file-level?
Best, Bobby
On Mon, 27 Oct 2025 12:08:48 -0700 Bobby Eshleman wrote:
shellcheck has some (new) things to say about this patch too. Could you take a look over them?
It looks like the errors are SC2317 and SC2119, but are false-positives. Invoking a program as a variable (e.g., "${VSOCK_TEST}") is tripping SC2317 (command unreachable), and SC2119 is due to log_{guest,host}() being passed zero arguments (logging its stdin instead).
I also see that SC2317 has many other false positives elsewhere in the file (80+), reporting even lines like `rm "${QEMU_PIDFILE}"` as unreachable. I wonder if we should add a patch to this series to disable this check at the file-level?
Yes, FWIW, don't hesitate to disable things at the file level. We should probably revisit which of the checks need to be disabled globally. But file level is also useful for manual testing.
On Mon, Oct 27, 2025 at 04:22:44PM -0700, Jakub Kicinski wrote:
On Mon, 27 Oct 2025 12:08:48 -0700 Bobby Eshleman wrote:
shellcheck has some (new) things to say about this patch too. Could you take a look over them?
It looks like the errors are SC2317 and SC2119, but are false-positives. Invoking a program as a variable (e.g., "${VSOCK_TEST}") is tripping SC2317 (command unreachable), and SC2119 is due to log_{guest,host}() being passed zero arguments (logging its stdin instead).
I also see that SC2317 has many other false positives elsewhere in the file (80+), reporting even lines like `rm "${QEMU_PIDFILE}"` as unreachable. I wonder if we should add a patch to this series to disable this check at the file-level?
Yes, FWIW, don't hesitate to disable things at the file level. We should probably revisit which of the checks need to be disabled globally. But file level is also useful for manual testing.
Got it, will do!
Thanks, Bobby
On Mon, Oct 27, 2025 at 12:08:48PM -0700, Bobby Eshleman wrote:
On Mon, Oct 27, 2025 at 11:01:36AM -0700, Bobby Eshleman wrote:
On Mon, Oct 27, 2025 at 04:58:02PM +0000, Simon Horman wrote:
On Wed, Oct 22, 2025 at 06:00:07PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Add wrapper functions vm_vsock_test() and host_vsock_test() to invoke the vsock_test binary. This encapsulates several items of repeat logic, such as waiting for the server to reach listening state and enabling/disabling the bash option pipefail to avoid pipe-style logging from hiding failures.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
shellcheck has some (new) things to say about this patch too. Could you take a look over them?
...
It looks like the errors are SC2317 and SC2119, but are false-positives. Invoking a program as a variable (e.g., "${VSOCK_TEST}") is tripping SC2317 (command unreachable), and SC2119 is due to log_{guest,host}() being passed zero arguments (logging its stdin instead).
Sorry about that, I thought I saw something meaningful in there. I guess I was mistaken.
I also see that SC2317 has many other false positives elsewhere in the file (80+), reporting even lines like `rm "${QEMU_PIDFILE}"` as unreachable. I wonder if we should add a patch to this series to disable this check at the file-level?
Best, Bobby
On Wed, Oct 29, 2025 at 04:58:05PM +0000, Simon Horman wrote:
On Mon, Oct 27, 2025 at 12:08:48PM -0700, Bobby Eshleman wrote:
On Mon, Oct 27, 2025 at 11:01:36AM -0700, Bobby Eshleman wrote:
On Mon, Oct 27, 2025 at 04:58:02PM +0000, Simon Horman wrote:
On Wed, Oct 22, 2025 at 06:00:07PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Add wrapper functions vm_vsock_test() and host_vsock_test() to invoke the vsock_test binary. This encapsulates several items of repeat logic, such as waiting for the server to reach listening state and enabling/disabling the bash option pipefail to avoid pipe-style logging from hiding failures.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
shellcheck has some (new) things to say about this patch too. Could you take a look over them?
...
It looks like the errors are SC2317 and SC2119, but are false-positives. Invoking a program as a variable (e.g., "${VSOCK_TEST}") is tripping SC2317 (command unreachable), and SC2119 is due to log_{guest,host}() being passed zero arguments (logging its stdin instead).
Sorry about that, I thought I saw something meaningful in there. I guess I was mistaken.
No problem at all, it brought my attention to shellcheck and the need for exclusions, which I honestly did not know we used formally upstream.
Best, Bobby
From: Bobby Eshleman bobbyeshleman@meta.com
Change QEMU to use generated pidfile names instead of just a single globally-defined pidfile. This allows multiple QEMU instances to co-exist with different pidfiles. This is required for future tests that use multiple VMs to check for CID collissions.
Additionally, this also places the burden of killing the QEMU process and cleaning up the pidfile on the caller of vm_start(). To help with this, a function terminate_pidfiles() is introduced that callers use to perform the cleanup. The terminate_pidfiles() function supports multiple pidfile removals because future patches will need to process two pidfiles at a time.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 53 +++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index 29b36b4d301d..9c72559aa894 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -23,7 +23,7 @@ readonly VSOCK_CID=1234 readonly WAIT_PERIOD=3 readonly WAIT_PERIOD_MAX=60 readonly WAIT_TOTAL=$(( WAIT_PERIOD * WAIT_PERIOD_MAX )) -readonly QEMU_PIDFILE=$(mktemp /tmp/qemu_vsock_vmtest_XXXX.pid) +readonly PIDFILE_TEMPLATE=/tmp/vsock_vmtest_XXXX.pid
# virtme-ng offers a netdev for ssh when using "--ssh", but we also need a # control port forwarded for vsock_test. Because virtme-ng doesn't support @@ -33,12 +33,6 @@ readonly QEMU_PIDFILE=$(mktemp /tmp/qemu_vsock_vmtest_XXXX.pid) # add the kernel cmdline options that virtme-init uses to setup the interface. readonly QEMU_TEST_PORT_FWD="hostfwd=tcp::${TEST_HOST_PORT}-:${TEST_GUEST_PORT}" readonly QEMU_SSH_PORT_FWD="hostfwd=tcp::${SSH_HOST_PORT}-:${SSH_GUEST_PORT}" -readonly QEMU_OPTS="\ - -netdev user,id=n0,${QEMU_TEST_PORT_FWD},${QEMU_SSH_PORT_FWD} \ - -device virtio-net-pci,netdev=n0 \ - -device vhost-vsock-pci,guest-cid=${VSOCK_CID} \ - --pidfile ${QEMU_PIDFILE} \ -" readonly KERNEL_CMDLINE="\ virtme.dhcp net.ifnames=0 biosdevname=0 \ virtme.ssh virtme_ssh_channel=tcp virtme_ssh_user=$USER \ @@ -89,17 +83,6 @@ vm_ssh() { return $? }
-cleanup() { - if [[ -s "${QEMU_PIDFILE}" ]]; then - pkill -SIGTERM -F "${QEMU_PIDFILE}" > /dev/null 2>&1 - fi - - # If failure occurred during or before qemu start up, then we need - # to clean this up ourselves. - if [[ -e "${QEMU_PIDFILE}" ]]; then - rm "${QEMU_PIDFILE}" - fi -}
check_args() { local found @@ -188,10 +171,26 @@ handle_build() { popd &>/dev/null }
+terminate_pidfiles() { + local pidfile + + for pidfile in "$@"; do + if [[ -s "${pidfile}" ]]; then + pkill -SIGTERM -F "${pidfile}" > /dev/null 2>&1 + fi + + if [[ -e "${pidfile}" ]]; then + rm -f "${pidfile}" + fi + done +} + vm_start() { + local pidfile=$1 local logfile=/dev/null local verbose_opt="" local kernel_opt="" + local qemu_opts="" local qemu
qemu=$(command -v "${QEMU}") @@ -201,6 +200,13 @@ vm_start() { logfile=/dev/stdout fi
+ qemu_opts="\ + -netdev user,id=n0,${QEMU_TEST_PORT_FWD},${QEMU_SSH_PORT_FWD} \ + -device virtio-net-pci,netdev=n0 \ + -device vhost-vsock-pci,guest-cid=${VSOCK_CID} \ + --pidfile ${pidfile} + " + if [[ "${BUILD}" -eq 1 ]]; then kernel_opt="${KERNEL_CHECKOUT}" fi @@ -209,14 +215,14 @@ vm_start() { --run \ ${kernel_opt} \ ${verbose_opt} \ - --qemu-opts="${QEMU_OPTS}" \ + --qemu-opts="${qemu_opts}" \ --qemu="${qemu}" \ --user root \ --append "${KERNEL_CMDLINE}" \ --rw &> ${logfile} &
if ! timeout ${WAIT_TOTAL} \ - bash -c 'while [[ ! -s '"${QEMU_PIDFILE}"' ]]; do sleep 1; done; exit 0'; then + bash -c 'while [[ ! -s '"${pidfile}"' ]]; do sleep 1; done; exit 0'; then die "failed to boot VM" fi } @@ -491,8 +497,6 @@ do done shift $((OPTIND-1))
-trap cleanup EXIT - if [[ ${#} -eq 0 ]]; then ARGS=("${TEST_NAMES[@]}") else @@ -507,7 +511,8 @@ handle_build echo "1..${#ARGS[@]}"
log_host "Booting up VM" -vm_start +pidfile="$(mktemp -u $PIDFILE_TEMPLATE)" +vm_start "${pidfile}" vm_wait_for_ssh log_host "VM booted up"
@@ -531,6 +536,8 @@ for arg in "${ARGS[@]}"; do cnt_total=$(( cnt_total + 1 )) done
+terminate_pidfiles "${pidfile}" + echo "SUMMARY: PASS=${cnt_pass} SKIP=${cnt_skip} FAIL=${cnt_fail}" echo "Log: ${LOG}"
On Wed, Oct 22, 2025 at 06:00:08PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Change QEMU to use generated pidfile names instead of just a single globally-defined pidfile. This allows multiple QEMU instances to co-exist with different pidfiles. This is required for future tests that use multiple VMs to check for CID collissions.
Additionally, this also places the burden of killing the QEMU process and cleaning up the pidfile on the caller of vm_start(). To help with this, a function terminate_pidfiles() is introduced that callers use to perform the cleanup. The terminate_pidfiles() function supports multiple pidfile removals because future patches will need to process two pidfiles at a time.
It seems that this will no longer cleanup, via a trap, if there is an early exit. Is that intentional?
This patch also changes the handling of QEMU_OPTS. I think that should be mentioned in the commit message too.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
...
On Mon, Oct 27, 2025 at 04:57:29PM +0000, Simon Horman wrote:
On Wed, Oct 22, 2025 at 06:00:08PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Change QEMU to use generated pidfile names instead of just a single globally-defined pidfile. This allows multiple QEMU instances to co-exist with different pidfiles. This is required for future tests that use multiple VMs to check for CID collissions.
Additionally, this also places the burden of killing the QEMU process and cleaning up the pidfile on the caller of vm_start(). To help with this, a function terminate_pidfiles() is introduced that callers use to perform the cleanup. The terminate_pidfiles() function supports multiple pidfile removals because future patches will need to process two pidfiles at a time.
It seems that this will no longer cleanup, via a trap, if there is an early exit. Is that intentional?
Yes, intentional. We're trusting the vm_start() caller to do any cleanup now. The assumption being that with no "set -e", vm_start() should be able to return to the caller.
If that seems too bold, we could add some function like create_pidfile() that generates the pidfiles and registers them into an array that is cleaned up via trap.
This patch also changes the handling of QEMU_OPTS. I think that should be mentioned in the commit message too.
Sounds good.
Best, Bobby
From: Bobby Eshleman bobbyeshleman@meta.com
If QEMU fails to boot, then set the returncode (via timeout) instead of unconditionally dying. This is in preparation for tests that expect QEMU to fail to boot. In that case, we just want to know if the boot failed or not so we can test the pass/fail criteria, and continue executing the next test.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index 9c72559aa894..6c8f199b771b 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -221,10 +221,8 @@ vm_start() { --append "${KERNEL_CMDLINE}" \ --rw &> ${logfile} &
- if ! timeout ${WAIT_TOTAL} \ - bash -c 'while [[ ! -s '"${pidfile}"' ]]; do sleep 1; done; exit 0'; then - die "failed to boot VM" - fi + timeout "${WAIT_TOTAL}" \ + bash -c 'while [[ ! -s '"${pidfile}"' ]]; do sleep 1; done; exit 0' }
vm_wait_for_ssh() {
On Wed, Oct 22, 2025 at 06:00:09PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
If QEMU fails to boot, then set the returncode (via timeout) instead of unconditionally dying. This is in preparation for tests that expect QEMU to fail to boot. In that case, we just want to know if the boot failed or not so we can test the pass/fail criteria, and continue executing the next test.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
Reviewed-by: Simon Horman horms@kernel.org
From: Bobby Eshleman bobbyeshleman@meta.com
Reduce the time waiting for the QEMU pidfile from three minutes to five seconds. The three minute time window was chosen to make sure QEMU had enough time to fully boot up. This, however, is an unreasonably long delay for QEMU to write the pidfile, which happens earlier when the QEMU process starts (not after VM boot). The three minute delay becomes noticeably wasteful in future tests that expect QEMU to fail and wait a full three minutes for a pidfile that will never exist.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index 6c8f199b771b..99db2e415253 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -22,7 +22,7 @@ readonly SSH_HOST_PORT=2222 readonly VSOCK_CID=1234 readonly WAIT_PERIOD=3 readonly WAIT_PERIOD_MAX=60 -readonly WAIT_TOTAL=$(( WAIT_PERIOD * WAIT_PERIOD_MAX )) +readonly WAIT_QEMU=5 readonly PIDFILE_TEMPLATE=/tmp/vsock_vmtest_XXXX.pid
# virtme-ng offers a netdev for ssh when using "--ssh", but we also need a @@ -221,7 +221,7 @@ vm_start() { --append "${KERNEL_CMDLINE}" \ --rw &> ${logfile} &
- timeout "${WAIT_TOTAL}" \ + timeout "${WAIT_QEMU}" \ bash -c 'while [[ ! -s '"${pidfile}"' ]]; do sleep 1; done; exit 0' }
On Wed, Oct 22, 2025 at 06:00:10PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Reduce the time waiting for the QEMU pidfile from three minutes to five seconds. The three minute time window was chosen to make sure QEMU had enough time to fully boot up. This, however, is an unreasonably long delay for QEMU to write the pidfile, which happens earlier when the QEMU process starts (not after VM boot). The three minute delay becomes noticeably wasteful in future tests that expect QEMU to fail and wait a full three minutes for a pidfile that will never exist.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
Reviewed-by: Simon Horman horms@kernel.org
From: Bobby Eshleman bobbyeshleman@meta.com
Add check_result() function to reuse logic for incrementing the pass/fail counters. This function will get used by different callers as we add different types of tests in future patches (namely, namespace and non-namespace tests will be called at different places, and re-use this function).
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index 99db2e415253..0957d6a41d08 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -78,6 +78,26 @@ die() { exit "${KSFT_FAIL}" }
+check_result() { + local rc num + + rc=$1 + num=$(( cnt_total + 1 )) + + if [[ ${rc} -eq $KSFT_PASS ]]; then + cnt_pass=$(( cnt_pass + 1 )) + echo "ok ${num} ${arg}" + elif [[ ${rc} -eq $KSFT_SKIP ]]; then + cnt_skip=$(( cnt_skip + 1 )) + echo "ok ${num} ${arg} # SKIP" + elif [[ ${rc} -eq $KSFT_FAIL ]]; then + cnt_fail=$(( cnt_fail + 1 )) + echo "not ok ${num} ${arg} # exit=$rc" + fi + + cnt_total=$(( cnt_total + 1 )) +} + vm_ssh() { ssh -q -o UserKnownHostsFile=/dev/null -p ${SSH_HOST_PORT} localhost "$@" return $? @@ -521,17 +541,7 @@ cnt_total=0 for arg in "${ARGS[@]}"; do run_test "${arg}" rc=$? - if [[ ${rc} -eq $KSFT_PASS ]]; then - cnt_pass=$(( cnt_pass + 1 )) - echo "ok ${cnt_total} ${arg}" - elif [[ ${rc} -eq $KSFT_SKIP ]]; then - cnt_skip=$(( cnt_skip + 1 )) - echo "ok ${cnt_total} ${arg} # SKIP" - elif [[ ${rc} -eq $KSFT_FAIL ]]; then - cnt_fail=$(( cnt_fail + 1 )) - echo "not ok ${cnt_total} ${arg} # exit=$rc" - fi - cnt_total=$(( cnt_total + 1 )) + check_result ${rc} done
terminate_pidfiles "${pidfile}"
On Wed, Oct 22, 2025 at 06:00:11PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Add check_result() function to reuse logic for incrementing the pass/fail counters. This function will get used by different callers as we add different types of tests in future patches (namely, namespace and non-namespace tests will be called at different places, and re-use this function).
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
Reviewed-by: Simon Horman horms@kernel.org
From: Bobby Eshleman bobbyeshleman@meta.com
In preparation for future patches that introduce tests that cannot re-use the same VM, add functions to identify those that *can* re-use a VM.
By continuing to re-use the same VM for these tests we can save time by avoiding the delay of booting a VM for every test.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 63 ++++++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index 0957d6a41d08..a728958c58ee 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -45,6 +45,8 @@ readonly TEST_DESCS=( "Run vsock_test using the loopback transport in the VM." )
+readonly USE_SHARED_VM=(vm_server_host_client vm_client_host_server vm_loopback) + VERBOSE=0
usage() { @@ -454,7 +456,44 @@ test_vm_loopback() { return "${KSFT_PASS}" }
-run_test() { +shared_vm_test() { + local tname + + tname="${1}" + + for testname in "${USE_SHARED_VM[@]}"; do + if [[ "${tname}" == "${testname}" ]]; then + return 0 + fi + done + + return 1 +} + +shared_vm_tests_requested() { + for arg in "$@"; do + if shared_vm_test "${arg}"; then + return 0 + fi + done + + return 1 +} + +run_shared_vm_tests() { + local arg + + for arg in "$@"; do + if ! shared_vm_test "${arg}"; then + continue + fi + + run_shared_vm_test "${arg}" + check_result $? + done +} + +run_shared_vm_test() { local host_oops_cnt_before local host_warn_cnt_before local vm_oops_cnt_before @@ -528,23 +567,21 @@ handle_build
echo "1..${#ARGS[@]}"
-log_host "Booting up VM" -pidfile="$(mktemp -u $PIDFILE_TEMPLATE)" -vm_start "${pidfile}" -vm_wait_for_ssh -log_host "VM booted up" - cnt_pass=0 cnt_fail=0 cnt_skip=0 cnt_total=0 -for arg in "${ARGS[@]}"; do - run_test "${arg}" - rc=$? - check_result ${rc} -done
-terminate_pidfiles "${pidfile}" +if shared_vm_tests_requested "${ARGS[@]}"; then + log_host "Booting up VM" + pidfile=$(mktemp $PIDFILE_TEMPLATE) + vm_start "${pidfile}" + vm_wait_for_ssh + log_host "VM booted up" + + run_shared_vm_tests "${ARGS[@]}" + terminate_pidfiles "${pidfile}" +fi
echo "SUMMARY: PASS=${cnt_pass} SKIP=${cnt_skip} FAIL=${cnt_fail}" echo "Log: ${LOG}"
On Wed, Oct 22, 2025 at 06:00:12PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
In preparation for future patches that introduce tests that cannot re-use the same VM, add functions to identify those that *can* re-use a VM.
By continuing to re-use the same VM for these tests we can save time by avoiding the delay of booting a VM for every test.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
Reviewed-by: Simon Horman horms@kernel.org
From: Bobby Eshleman bobbyeshleman@meta.com
Add the definition for BUILD and initialize it to zero. This avoids 'bash -u vmtest.sh` from throwing 'unbound variable' when BUILD is not set to 1 and is later checked for its value.
Fixes: a4a65c6fe08b ("selftests/vsock: add initial vmtest.sh for vsock") Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index a728958c58ee..a312930cb8b7 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -541,6 +541,7 @@ run_shared_vm_test() { return "${rc}" }
+BUILD=0 QEMU="qemu-system-$(uname -m)"
while getopts :hvsq:b o
On Wed, Oct 22, 2025 at 06:00:13PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Add the definition for BUILD and initialize it to zero. This avoids 'bash -u vmtest.sh` from throwing 'unbound variable' when BUILD is not set to 1 and is later checked for its value.
Fixes: a4a65c6fe08b ("selftests/vsock: add initial vmtest.sh for vsock") Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
Hi Bobby,
I think that if this is a fix for the cited commit, which is present in net, then this patch should be targeted at net rather than net-next.
...
From: Bobby Eshleman bobbyeshleman@meta.com
Sometimes VMs will have some intermittent dmesg warnings that are unrelated to vsock. Change the dmesg parsing to filter on strings containing 'vsock' to avoid false positive failures that are unrelated to vsock. The downside is that it is possible for some vsock related warnings to not contain the substring 'vsock', so those will be missed.
Fixes: a4a65c6fe08b ("selftests/vsock: add initial vmtest.sh for vsock") Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index a312930cb8b7..aa7199c94780 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -506,9 +506,9 @@ run_shared_vm_test() { local rc
host_oops_cnt_before=$(dmesg | grep -c -i 'Oops') - host_warn_cnt_before=$(dmesg --level=warn | wc -l) + host_warn_cnt_before=$(dmesg --level=warn | grep -c -i 'vsock') vm_oops_cnt_before=$(vm_ssh -- dmesg | grep -c -i 'Oops') - vm_warn_cnt_before=$(vm_ssh -- dmesg --level=warn | wc -l) + vm_warn_cnt_before=$(vm_ssh -- dmesg --level=warn | grep -c -i 'vsock')
name=$(echo "${1}" | awk '{ print $1 }') eval test_"${name}" @@ -520,7 +520,7 @@ run_shared_vm_test() { rc=$KSFT_FAIL fi
- host_warn_cnt_after=$(dmesg --level=warn | wc -l) + host_warn_cnt_after=$(dmesg --level=warn | grep -c -i vsock) if [[ ${host_warn_cnt_after} -gt ${host_warn_cnt_before} ]]; then echo "FAIL: kernel warning detected on host" | log_host rc=$KSFT_FAIL @@ -532,7 +532,7 @@ run_shared_vm_test() { rc=$KSFT_FAIL fi
- vm_warn_cnt_after=$(vm_ssh -- dmesg --level=warn | wc -l) + vm_warn_cnt_after=$(vm_ssh -- dmesg --level=warn | grep -c -i vsock) if [[ ${vm_warn_cnt_after} -gt ${vm_warn_cnt_before} ]]; then echo "FAIL: kernel warning detected on vm" | log_host rc=$KSFT_FAIL
On Wed, Oct 22, 2025 at 06:00:14PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Sometimes VMs will have some intermittent dmesg warnings that are unrelated to vsock. Change the dmesg parsing to filter on strings containing 'vsock' to avoid false positive failures that are unrelated to vsock. The downside is that it is possible for some vsock related warnings to not contain the substring 'vsock', so those will be missed.
Fixes: a4a65c6fe08b ("selftests/vsock: add initial vmtest.sh for vsock") Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
Reviewed-by: Simon Horman horms@kernel.org
From: Bobby Eshleman bobbyeshleman@meta.com
Testing with 1.37 shows all tests passing but emits the warning:
warning: vng version 'virtme-ng 1.37' has not been tested and may not function properly. The following versions have been tested: 1.33 1.36
This patch adds 1.37 to the virtme-ng versions to get rid of the above warning.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index aa7199c94780..038bb5e2b5e2 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -152,7 +152,7 @@ check_vng() { local version local ok
- tested_versions=("1.33" "1.36") + tested_versions=("1.33" "1.36" "1.37") version="$(vng --version)"
ok=0
On Wed, Oct 22, 2025 at 06:00:15PM -0700, Bobby Eshleman wrote:
From: Bobby Eshleman bobbyeshleman@meta.com
Testing with 1.37 shows all tests passing but emits the warning:
warning: vng version 'virtme-ng 1.37' has not been tested and may not function properly. The following versions have been tested: 1.33 1.36
This patch adds 1.37 to the virtme-ng versions to get rid of the above warning.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com
Reviewed-by: Simon Horman horms@kernel.org
From: Bobby Eshleman bobbyeshleman@meta.com
Add vsock_loopback module loading to the loopback test.
When testing vsock_loopback as a module, it must be loaded before the test executes or else the test will fail with errno 110.
Signed-off-by: Bobby Eshleman bobbyeshleman@meta.com --- tools/testing/selftests/vsock/vmtest.sh | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh index 038bb5e2b5e2..62b4f5ede9f6 100755 --- a/tools/testing/selftests/vsock/vmtest.sh +++ b/tools/testing/selftests/vsock/vmtest.sh @@ -445,6 +445,8 @@ test_vm_client_host_server() { test_vm_loopback() { local port=60000 # non-forwarded local port
+ vm_ssh -- modprobe vsock_loopback &> /dev/null || : + if ! vm_vsock_test "server" 1 "${port}"; then return "${KSFT_FAIL}" fi
linux-kselftest-mirror@lists.linaro.org
-
Bobby Eshleman -
Jakub Kicinski -
Simon Horman