netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure
@ 2025-11-07  0:49 Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 01/11] selftests/vsock: improve logging in vmtest.sh Bobby Eshleman
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	Bobby Eshleman, Simon Horman

Hey all,

This patch series refactors the vsock selftest VM infrastructure to
improve test run times, improve logging, and prepare 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>
Cc: virtualization@lists.linux.dev
Cc: netdev@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Changes in v3:
- see per-patch changes

Changes in v2:
- remove "Fixes" for some patches because they do not fix bugs in
  kselftest runs (some fix bugs only when using bash args that kselftest
  does not use or otherwise prepare functions for new usage)
- broke out one fixes patch for "net"
- per-patch changes
- add patch for shellcheck declaration to disable false positives
- Link to v1: https://lore.kernel.org/r/20251022-vsock-selftests-fixes-and-improvements-v1-0-edeb179d6463@meta.com

---
Bobby Eshleman (11):
      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: add BUILD=0 definition
      selftests/vsock: add 1.37 to tested virtme-ng versions
      selftests/vsock: add vsock_loopback module loading
      selftests/vsock: disable shellcheck SC2317 and SC2119

 tools/testing/selftests/vsock/vmtest.sh | 355 ++++++++++++++++++++++----------
 1 file changed, 243 insertions(+), 112 deletions(-)
---
base-commit: 8a25a2e34157d882032112e4194ccdfb29c499e8
change-id: 20251021-vsock-selftests-fixes-and-improvements-057440ffb2fa

Best regards,
-- 
Bobby Eshleman <bobbyeshleman@meta.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 01/11] selftests/vsock: improve logging in vmtest.sh
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 02/11] selftests/vsock: make wait_for_listener() work even if pipefail is on Bobby Eshleman
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	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 in v2:
- add quotes around $@ in log_{host,guest} (Simon)
- remove unnecessary cat for piping into awk (Simon)

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 8ceeb8a7894f..54bae61bf6d4 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
+			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 | grep -c -i 'vsock')
 	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 | grep -c -i 'vsock')
 	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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 02/11] selftests/vsock: make wait_for_listener() work even if pipefail is on
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 01/11] selftests/vsock: improve logging in vmtest.sh Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 03/11] selftests/vsock: reuse logic for vsock_test through wrapper functions Bobby Eshleman
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	Bobby Eshleman

From: Bobby Eshleman <bobbyeshleman@meta.com>

Rewrite wait_for_listener()'s pattern matching to avoid tripping the
if-condition when pipefail is on.

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 (grep exits, SIGPIPE emits, and awk complains with a
non-zero exit code). Instead, move all of the pattern matching into awk
so that SIGPIPE cannot happen and the correct exit code is returned.

Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v2:
- use awk-only tcp port lookup
- remove fixes tag because this problem is only introduced when a later
  patch enables pipefail for other reasons (not yet in tree)
---
 tools/testing/selftests/vsock/vmtest.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index 54bae61bf6d4..d936209d3eaa 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -251,9 +251,11 @@ wait_for_listener()
 
 	# for tcp protocol additionally check the socket state
 	[ "${protocol}" = "tcp" ] && pattern="${pattern}0A"
+
 	for i in $(seq "${max_intervals}"); do
-		if awk '{print $2" "$4}' /proc/net/"${protocol}"* | \
-		   grep -q "${pattern}"; then
+		if awk -v pattern="${pattern}" \
+			'BEGIN {rc=1} $2" "$4 ~ pattern {rc=0} END {exit rc}' \
+			/proc/net/"${protocol}"*; then
 			break
 		fi
 		sleep "${interval}"

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 03/11] selftests/vsock: reuse logic for vsock_test through wrapper functions
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 01/11] selftests/vsock: improve logging in vmtest.sh Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 02/11] selftests/vsock: make wait_for_listener() work even if pipefail is on Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 04/11] selftests/vsock: avoid multi-VM pidfile collisions with QEMU Bobby Eshleman
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	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>
---
Changes in v3:
- Add port input parameter to host_wait_for_listener() to accept port
  from host_vsock_test() (Stefano)
- Change host_wait_for_listener() call-site to pass in parameter
---
 tools/testing/selftests/vsock/vmtest.sh | 134 ++++++++++++++++++++++----------
 1 file changed, 94 insertions(+), 40 deletions(-)

diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index d936209d3eaa..01ac2b7ee8db 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -272,8 +272,80 @@ EOF
 }
 
 host_wait_for_listener() {
-	wait_for_listener "${TEST_HOST_PORT_LISTENER}" "${WAIT_PERIOD}" "${WAIT_PERIOD_MAX}"
+	local port=$1
 
+	wait_for_listener "${port}" "${WAIT_PERIOD}" "${WAIT_PERIOD_MAX}"
+}
+
+vm_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
+		vm_ssh -- "${VSOCK_TEST}" \
+			--mode=client \
+			--control-host="${host}" \
+			--peer-cid="${cid}" \
+			--control-port="${port}" \
+			2>&1 | log_guest
+		rc=$?
+	else
+		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}"
+		rc=$?
+	fi
+	set +o pipefail
+
+	return $rc
 }
 
 log() {
@@ -312,59 +384,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 04/11] selftests/vsock: avoid multi-VM pidfile collisions with QEMU
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
                   ` (2 preceding siblings ...)
  2025-11-07  0:49 ` [PATCH net-next v3 03/11] selftests/vsock: reuse logic for vsock_test through wrapper functions Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  2025-11-07 15:08   ` Simon Horman
  2025-11-07  0:49 ` [PATCH net-next v3 05/11] selftests/vsock: do not unconditionally die if qemu fails Bobby Eshleman
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	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.

Change QEMU_OPTS to be initialized inside the vm_start(). This allows
the generated pidfile to be passed to the string assignment, and
prepares for future vm-specific options as well (e.g., cid).

Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v3:
- do not add unneeded -u to mktemp (Stefano)
- quote $PIDFILE_TEMPLATE (Stefano)
- do not remove cleanup(). Though it is expected that vm_start() does
  not exit(2) and its caller is responsible for pidfile cleanup,
  retaining cleanup() on EXIT is worth keeping as ill-timed kill signals
  (i.e., during manual runs) may leak those files.
- add create_pidfile() function to generate pidfile and automatically
  add it to array for cleanup() to terminate and remove later.

Changes in v2:
- mention QEMU_OPTS changes in commit message (Simon)
---
 tools/testing/selftests/vsock/vmtest.sh | 70 ++++++++++++++++++++++++---------
 1 file changed, 52 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index 01ac2b7ee8db..a461ef1fcc61 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -23,7 +23,8 @@ 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
+declare -a PIDFILES
 
 # 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 +34,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 \
@@ -90,15 +85,19 @@ vm_ssh() {
 }
 
 cleanup() {
-	if [[ -s "${QEMU_PIDFILE}" ]]; then
-		pkill -SIGTERM -F "${QEMU_PIDFILE}" > /dev/null 2>&1
-	fi
+	local pidfile
 
-	# 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
+	for pidfile in "${PIDFILES[@]}"; do
+		if [[ -s "${pidfile}" ]]; then
+			pkill -SIGTERM -F "${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 "${pidfile}" ]]; then
+			rm "${pidfile}"
+		fi
+	done
 }
 
 check_args() {
@@ -188,10 +187,35 @@ handle_build() {
 	popd &>/dev/null
 }
 
+create_pidfile() {
+	local pidfile
+
+	pidfile=$(mktemp "${PIDFILE_TEMPLATE}")
+	PIDFILES+=("${pidfile}")
+
+	echo "${pidfile}"
+}
+
+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 +225,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 +240,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
 }
@@ -498,7 +529,8 @@ handle_build
 echo "1..${#ARGS[@]}"
 
 log_host "Booting up VM"
-vm_start
+pidfile="$(create_pidfile)"
+vm_start "${pidfile}"
 vm_wait_for_ssh
 log_host "VM booted up"
 
@@ -522,6 +554,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 05/11] selftests/vsock: do not unconditionally die if qemu fails
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
                   ` (3 preceding siblings ...)
  2025-11-07  0:49 ` [PATCH net-next v3 04/11] selftests/vsock: avoid multi-VM pidfile collisions with QEMU Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 06/11] selftests/vsock: speed up tests by reducing the QEMU pidfile timeout Bobby Eshleman
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	Bobby Eshleman, Simon Horman

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.

Reviewed-by: Simon Horman <horms@kernel.org>
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 a461ef1fcc61..ede74add070a 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -246,10 +246,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 06/11] selftests/vsock: speed up tests by reducing the QEMU pidfile timeout
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
                   ` (4 preceding siblings ...)
  2025-11-07  0:49 ` [PATCH net-next v3 05/11] selftests/vsock: do not unconditionally die if qemu fails Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 07/11] selftests/vsock: add check_result() for pass/fail counting Bobby Eshleman
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	Bobby Eshleman, Simon Horman

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.

Reviewed-by: Simon Horman <horms@kernel.org>
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 ede74add070a..557f9a99a306 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
 declare -a PIDFILES
 
@@ -246,7 +246,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 07/11] selftests/vsock: add check_result() for pass/fail counting
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
                   ` (5 preceding siblings ...)
  2025-11-07  0:49 ` [PATCH net-next v3 06/11] selftests/vsock: speed up tests by reducing the QEMU pidfile timeout Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 08/11] selftests/vsock: add BUILD=0 definition Bobby Eshleman
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	Bobby Eshleman, Simon Horman

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

Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v3:
- increment cnt_total directly (no intermediary var) (Stefano)
- pass arg to check_result() from caller, dont incidentally rely on
  global (Stefano)
- use new create_pidfile() introduce in v3 of earlier patch
- continue with more disciplined variable quoting style
---
 tools/testing/selftests/vsock/vmtest.sh | 95 +++++++++++++++++++++++++--------
 1 file changed, 72 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index 557f9a99a306..05cf370a3db4 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -46,6 +46,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() {
@@ -79,6 +81,28 @@ die() {
 	exit "${KSFT_FAIL}"
 }
 
+check_result() {
+	local rc arg
+
+	rc=$1
+	arg=$2
+
+	cnt_total=$(( 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 $?
@@ -450,7 +474,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 "$?" "${arg}"
+	done
+}
+
+run_shared_vm_test() {
 	local host_oops_cnt_before
 	local host_warn_cnt_before
 	local vm_oops_cnt_before
@@ -526,33 +587,21 @@ handle_build
 
 echo "1..${#ARGS[@]}"
 
-log_host "Booting up VM"
-pidfile="$(create_pidfile)"
-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=$?
-	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 ))
-done
 
-terminate_pidfiles "${pidfile}"
+if shared_vm_tests_requested "${ARGS[@]}"; then
+	log_host "Booting up VM"
+	pidfile="$(create_pidfile)"
+	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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 08/11] selftests/vsock: add BUILD=0 definition
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
                   ` (6 preceding siblings ...)
  2025-11-07  0:49 ` [PATCH net-next v3 07/11] selftests/vsock: add check_result() for pass/fail counting Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 09/11] selftests/vsock: add 1.37 to tested virtme-ng versions Bobby Eshleman
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	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.

Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v2:
- remove fixes tag because it doesn't fix breakage of kselftest, and
  just supports otherwise invoking with bash -u
---
 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 05cf370a3db4..7962bc40d055 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -559,6 +559,7 @@ run_shared_vm_test() {
 	return "${rc}"
 }
 
+BUILD=0
 QEMU="qemu-system-$(uname -m)"
 
 while getopts :hvsq:b o

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 09/11] selftests/vsock: add 1.37 to tested virtme-ng versions
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
                   ` (7 preceding siblings ...)
  2025-11-07  0:49 ` [PATCH net-next v3 08/11] selftests/vsock: add BUILD=0 definition Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 10/11] selftests/vsock: add vsock_loopback module loading Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 11/11] selftests/vsock: disable shellcheck SC2317 and SC2119 Bobby Eshleman
  10 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	Bobby Eshleman, Simon Horman

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.

Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
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 7962bc40d055..a5c33b475a39 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -170,7 +170,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 10/11] selftests/vsock: add vsock_loopback module loading
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
                   ` (8 preceding siblings ...)
  2025-11-07  0:49 ` [PATCH net-next v3 09/11] selftests/vsock: add 1.37 to tested virtme-ng versions Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  2025-11-07  0:49 ` [PATCH net-next v3 11/11] selftests/vsock: disable shellcheck SC2317 and SC2119 Bobby Eshleman
  10 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	Bobby Eshleman

From: Bobby Eshleman <bobbyeshleman@meta.com>

Add vsock_loopback module loading to the loopback test so that vmtest.sh
can be used for kernels built with loopback as a module.

This is not technically a fix as kselftest expects loopback to be
built-in already (defined in selftests/vsock/config). This is useful
only for using vmtest.sh outside of kselftest.

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
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 a5c33b475a39..cde048bd7fe6 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -463,6 +463,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v3 11/11] selftests/vsock: disable shellcheck SC2317 and SC2119
  2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
                   ` (9 preceding siblings ...)
  2025-11-07  0:49 ` [PATCH net-next v3 10/11] selftests/vsock: add vsock_loopback module loading Bobby Eshleman
@ 2025-11-07  0:49 ` Bobby Eshleman
  10 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07  0:49 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: virtualization, netdev, linux-kselftest, linux-kernel,
	Bobby Eshleman

From: Bobby Eshleman <bobbyeshleman@meta.com>

Disable shellcheck rules SC2317 an SC2119. These rules are being
triggered due to false positives. For SC2317, many `return
"${KSFT_PASS}"` lines are reported as unreachable, even though they are
executed during normal runs. For SC2119, the fact that
log_guest/log_host accept either stdin or arguments triggers SC2119,
despite being valid.

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 cde048bd7fe6..bda1ad173ad1 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -7,6 +7,8 @@
 #		* virtme-ng
 #		* busybox-static (used by virtme-ng)
 #		* qemu	(used by virtme-ng)
+#
+# shellcheck disable=SC2317,SC2119
 
 readonly SCRIPT_DIR="$(cd -P -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)"
 readonly KERNEL_CHECKOUT=$(realpath "${SCRIPT_DIR}"/../../../../)

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v3 04/11] selftests/vsock: avoid multi-VM pidfile collisions with QEMU
  2025-11-07  0:49 ` [PATCH net-next v3 04/11] selftests/vsock: avoid multi-VM pidfile collisions with QEMU Bobby Eshleman
@ 2025-11-07 15:08   ` Simon Horman
  2025-11-07 15:35     ` Bobby Eshleman
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Horman @ 2025-11-07 15:08 UTC (permalink / raw)
  To: Bobby Eshleman
  Cc: Stefano Garzarella, Shuah Khan, virtualization, netdev,
	linux-kselftest, linux-kernel, Bobby Eshleman

On Thu, Nov 06, 2025 at 04:49:48PM -0800, Bobby Eshleman wrote:

...

> @@ -90,15 +85,19 @@ vm_ssh() {
>  }
>  
>  cleanup() {
> -	if [[ -s "${QEMU_PIDFILE}" ]]; then
> -		pkill -SIGTERM -F "${QEMU_PIDFILE}" > /dev/null 2>&1
> -	fi
> +	local pidfile
>  
> -	# 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
> +	for pidfile in "${PIDFILES[@]}"; do
> +		if [[ -s "${pidfile}" ]]; then
> +			pkill -SIGTERM -F "${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 "${pidfile}" ]]; then
> +			rm "${pidfile}"
> +		fi
> +	done
>  }

Hi Bobby,

This is completely untested, but it looks to me
like cleanup() could be implemented more succinctly like this.

cleanup() {
	terminate_pidfiles "${PIDFILES[@]}"
}

>  
>  check_args() {
> @@ -188,10 +187,35 @@ handle_build() {
>  	popd &>/dev/null
>  }
>  
> +create_pidfile() {
> +	local pidfile
> +
> +	pidfile=$(mktemp "${PIDFILE_TEMPLATE}")
> +	PIDFILES+=("${pidfile}")
> +
> +	echo "${pidfile}"
> +}
> +
> +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

I think it would be useful to remove $pidfile from $PIDFILES.
This might be easier to implement if PIDFILES was an associative array.

> +}
> +

...

> @@ -498,7 +529,8 @@ handle_build
>  echo "1..${#ARGS[@]}"
>  
>  log_host "Booting up VM"
> -vm_start
> +pidfile="$(create_pidfile)"
> +vm_start "${pidfile}"
>  vm_wait_for_ssh
>  log_host "VM booted up"
>  

> @@ -522,6 +554,8 @@ for arg in "${ARGS[@]}"; do
>  	cnt_total=$(( cnt_total + 1 ))
>  done
>  
> +terminate_pidfiles "${pidfile}"

I am assuming that there will be more calls to terminate_pidfiles
in subsequent patch-sets.

Else I think terminate_pidfiles can be removed
and instead we can rely on cleanup().

> +
>  echo "SUMMARY: PASS=${cnt_pass} SKIP=${cnt_skip} FAIL=${cnt_fail}"
>  echo "Log: ${LOG}"
>  
> 
> -- 
> 2.47.3
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v3 04/11] selftests/vsock: avoid multi-VM pidfile collisions with QEMU
  2025-11-07 15:08   ` Simon Horman
@ 2025-11-07 15:35     ` Bobby Eshleman
  0 siblings, 0 replies; 14+ messages in thread
From: Bobby Eshleman @ 2025-11-07 15:35 UTC (permalink / raw)
  To: Simon Horman
  Cc: Stefano Garzarella, Shuah Khan, virtualization, netdev,
	linux-kselftest, linux-kernel, Bobby Eshleman

On Fri, Nov 07, 2025 at 03:08:25PM +0000, Simon Horman wrote:
> On Thu, Nov 06, 2025 at 04:49:48PM -0800, Bobby Eshleman wrote:
> 
> ...
> 
> > @@ -90,15 +85,19 @@ vm_ssh() {
> >  }
> >  
> >  cleanup() {
> > -	if [[ -s "${QEMU_PIDFILE}" ]]; then
> > -		pkill -SIGTERM -F "${QEMU_PIDFILE}" > /dev/null 2>&1
> > -	fi
> > +	local pidfile
> >  
> > -	# 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
> > +	for pidfile in "${PIDFILES[@]}"; do
> > +		if [[ -s "${pidfile}" ]]; then
> > +			pkill -SIGTERM -F "${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 "${pidfile}" ]]; then
> > +			rm "${pidfile}"
> > +		fi
> > +	done
> >  }
> 
> Hi Bobby,
> 
> This is completely untested, but it looks to me
> like cleanup() could be implemented more succinctly like this.
> 
> cleanup() {
> 	terminate_pidfiles "${PIDFILES[@]}"
> }
> 

Oh right! I reverted the deletion and completely forgot about
terminate_pidfiles().

> >  
> >  check_args() {
> > @@ -188,10 +187,35 @@ handle_build() {
> >  	popd &>/dev/null
> >  }
> >  
> > +create_pidfile() {
> > +	local pidfile
> > +
> > +	pidfile=$(mktemp "${PIDFILE_TEMPLATE}")
> > +	PIDFILES+=("${pidfile}")
> > +
> > +	echo "${pidfile}"
> > +}
> > +
> > +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
> 
> I think it would be useful to remove $pidfile from $PIDFILES.
> This might be easier to implement if PIDFILES was an associative array.
> 

Using an associative makes sense, this way we can trim the set.

> > +}
> > +
> 
> ...
> 
> > @@ -498,7 +529,8 @@ handle_build
> >  echo "1..${#ARGS[@]}"
> >  
> >  log_host "Booting up VM"
> > -vm_start
> > +pidfile="$(create_pidfile)"
> > +vm_start "${pidfile}"
> >  vm_wait_for_ssh
> >  log_host "VM booted up"
> >  
> 
> > @@ -522,6 +554,8 @@ for arg in "${ARGS[@]}"; do
> >  	cnt_total=$(( cnt_total + 1 ))
> >  done
> >  
> > +terminate_pidfiles "${pidfile}"
> 
> I am assuming that there will be more calls to terminate_pidfiles
> in subsequent patch-sets.
> 
> Else I think terminate_pidfiles can be removed
> and instead we can rely on cleanup().
> 

Indeed, later patches will use terminate_pidfiles() in between spin up /
shut down of multiple VMs.


Thanks again, will incorporate your feedback in the next!

Best,
Bobby

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-11-07 15:35 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-07  0:49 [PATCH net-next v3 00/11] selftests/vsock: refactor and improve vmtest infrastructure Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 01/11] selftests/vsock: improve logging in vmtest.sh Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 02/11] selftests/vsock: make wait_for_listener() work even if pipefail is on Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 03/11] selftests/vsock: reuse logic for vsock_test through wrapper functions Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 04/11] selftests/vsock: avoid multi-VM pidfile collisions with QEMU Bobby Eshleman
2025-11-07 15:08   ` Simon Horman
2025-11-07 15:35     ` Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 05/11] selftests/vsock: do not unconditionally die if qemu fails Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 06/11] selftests/vsock: speed up tests by reducing the QEMU pidfile timeout Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 07/11] selftests/vsock: add check_result() for pass/fail counting Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 08/11] selftests/vsock: add BUILD=0 definition Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 09/11] selftests/vsock: add 1.37 to tested virtme-ng versions Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 10/11] selftests/vsock: add vsock_loopback module loading Bobby Eshleman
2025-11-07  0:49 ` [PATCH net-next v3 11/11] selftests/vsock: disable shellcheck SC2317 and SC2119 Bobby Eshleman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).