| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3osszz3giogog7jzs37pdqhakcrveayrqu6xduztuwrftkwrad@gjj3cyvmypw3>
Date: Wed, 5 Nov 2025 15:32:18 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Bobby Eshleman <bobbyeshleman@...il.com>
Cc: Shuah Khan <shuah@...nel.org>, Jakub Kicinski <kuba@...nel.org>,
virtualization@...ts.linux.dev, netdev@...r.kernel.org, linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org, Simon Horman <horms@...nel.org>,
Bobby Eshleman <bobbyeshleman@...a.com>
Subject: Re: [PATCH net-next v2 04/12] selftests/vsock: avoid multi-VM
pidfile collisions with QEMU
On Tue, Nov 04, 2025 at 02:38:54PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@...a.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 passed to the string assignment, and prepares for
>future vm-specific options as well (e.g., cid).
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@...a.com>
>---
>Changes in v2:
>- mention QEMU_OPTS changes in commit message (Simon)
>---
> 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 03dc4717ac3b..5637c98d5fe8 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
> }
>@@ -480,8 +486,6 @@ do
> done
> shift $((OPTIND-1))
>
>-trap cleanup EXIT
>-
Why avoiding the cleanup on exit?
Should we mention this change in commit description?
Thanks,
Stefano
> if [[ ${#} -eq 0 ]]; then
> ARGS=("${TEST_NAMES[@]}")
> else
>@@ -496,7 +500,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"
>
>@@ -520,6 +525,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
>
Powered by blists - more mailing lists