[PATCH net-next 00/12] selftests/vsock: refactor and improve vmtest infrastructure

[PATCH net-next 00/12] selftests/vsock: refactor and improve vmtest infrastructure

[Bobby Eshleman]

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,
-- 
Bobby Eshleman <bobbyeshleman@meta.com>

[PATCH net-next 01/12] selftests/vsock: improve logging in vmtest.sh

[Bobby Eshleman]

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

-- 
2.47.3

[PATCH net-next 02/12] selftests/vsock: make wait_for_listener() work even if pipefail is on

[Bobby Eshleman]

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() {

-- 
2.47.3

[PATCH net-next 03/12] selftests/vsock: reuse logic for vsock_test through wrapper functions

[Bobby Eshleman]

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() {

-- 
2.47.3

[PATCH net-next 04/12] selftests/vsock: avoid multi-VM pidfile collisions with QEMU

[Bobby Eshleman]

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}"
 

-- 
2.47.3

[PATCH net-next 05/12] selftests/vsock: do not unconditionally die if qemu fails

[Bobby Eshleman]

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() {

-- 
2.47.3

[PATCH net-next 06/12] selftests/vsock: speed up tests by reducing the QEMU pidfile timeout

[Bobby Eshleman]

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'
 }
 

-- 
2.47.3

[PATCH net-next 07/12] selftests/vsock: add check_result() for pass/fail counting

[Bobby Eshleman]

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}"

-- 
2.47.3

[PATCH net-next 08/12] selftests/vsock: identify and execute tests that can re-use VM

[Bobby Eshleman]

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}"

-- 
2.47.3

[PATCH net-next 09/12] selftests/vsock: add BUILD=0 definition

[Bobby Eshleman]

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

-- 
2.47.3

[PATCH net-next 10/12] selftests/vsock: avoid false-positives when checking dmesg

[Bobby Eshleman]

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

-- 
2.47.3

[PATCH net-next 11/12] selftests/vsock: add 1.37 to tested virtme-ng versions

[Bobby Eshleman]

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

-- 
2.47.3

[PATCH net-next 12/12] selftests/vsock: add vsock_loopback module loading

[Bobby Eshleman]

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

-- 
2.47.3

Re: [PATCH net-next 01/12] selftests/vsock: improve logging in vmtest.sh

[Simon Horman]

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}" ]]; then
> +			cat | awk -v prefix="${prefix}" '{printf "%s: %s\n", prefix, $0}'

FIWIIW, I would drop cat from this line.

> +		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 $@

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.

>  }

...

Re: [PATCH net-next 02/12] selftests/vsock: make wait_for_listener() work even if pipefail is on

[Simon Horman]

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
> 
> 

Re: [PATCH net-next 04/12] selftests/vsock: avoid multi-VM pidfile collisions with QEMU

[Simon Horman]

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>

...

Re: [PATCH net-next 03/12] selftests/vsock: reuse logic for vsock_test through wrapper functions

[Simon Horman]

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?

...

Re: [PATCH net-next 01/12] selftests/vsock: improve logging in vmtest.sh

[Bobby Eshleman]

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}" ]]; then
> > +			cat | awk -v prefix="${prefix}" '{printf "%s: %s\n", prefix, $0}'
> 
> FIWIIW, I would drop cat from this line.
> 

sgtm!

> > +		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 $@
> 
> 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

Re: [PATCH net-next 09/12] selftests/vsock: add BUILD=0 definition

[Simon Horman]

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.

...

Re: [PATCH net-next 02/12] selftests/vsock: make wait_for_listener() work even if pipefail is on

[Bobby Eshleman]

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

Re: [PATCH net-next 04/12] selftests/vsock: avoid multi-VM pidfile collisions with QEMU

[Bobby Eshleman]

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

Re: [PATCH net-next 03/12] selftests/vsock: reuse logic for vsock_test through wrapper functions

[Bobby Eshleman]

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

Re: [PATCH net-next 03/12] selftests/vsock: reuse logic for vsock_test through wrapper functions

[Bobby Eshleman]

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

Re: [PATCH net-next 03/12] selftests/vsock: reuse logic for vsock_test through wrapper functions

[Jakub Kicinski]

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.

Re: [PATCH net-next 03/12] selftests/vsock: reuse logic for vsock_test through wrapper functions

[Bobby Eshleman]

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

Re: [PATCH net-next 02/12] selftests/vsock: make wait_for_listener() work even if pipefail is on

[Simon Horman]

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.

Re: [PATCH net-next 03/12] selftests/vsock: reuse logic for vsock_test through wrapper functions

[Simon Horman]

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
> 

Re: [PATCH net-next 05/12] selftests/vsock: do not unconditionally die if qemu fails

[Simon Horman]

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>

Re: [PATCH net-next 06/12] selftests/vsock: speed up tests by reducing the QEMU pidfile timeout

[Simon Horman]

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>

Re: [PATCH net-next 07/12] selftests/vsock: add check_result() for pass/fail counting

[Simon Horman]

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>

Re: [PATCH net-next 08/12] selftests/vsock: identify and execute tests that can re-use VM

[Simon Horman]

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>

Re: [PATCH net-next 10/12] selftests/vsock: avoid false-positives when checking dmesg

[Simon Horman]

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>

Re: [PATCH net-next 03/12] selftests/vsock: reuse logic for vsock_test through wrapper functions

[Bobby Eshleman]

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

Re: [PATCH net-next 11/12] selftests/vsock: add 1.37 to tested virtme-ng versions

[Simon Horman]

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>