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

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