linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Thomas Huth <thuth@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>,
	linux-s390@vger.kernel.org, Nico Boehr <nrb@linux.ibm.com>,
	Janosch Frank <frankja@linux.ibm.com>,
	kvm@vger.kernel.org, David Hildenbrand <david@redhat.com>,
	linuxppc-dev@lists.ozlabs.org,
	Shaoqin Huang <shahuang@redhat.com>,
	Nicholas Piggin <npiggin@gmail.com>,
	Andrew Jones <andrew.jones@linux.dev>,
	Eric Auger <eric.auger@redhat.com>,
	Marc Hartmayer <mhartmay@linux.ibm.com>,
	kvm-riscv@lists.infradead.org, kvmarm@lists.linux.dev,
	Paolo Bonzini <pbonzini@redhat.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>
Subject: [kvm-unit-tests PATCH v4 3/8] migration: use a more robust way to wait for background job
Date: Fri,  9 Feb 2024 19:11:29 +1000	[thread overview]
Message-ID: <20240209091134.600228-4-npiggin@gmail.com> (raw)
In-Reply-To: <20240209091134.600228-1-npiggin@gmail.com>

Starting a pipeline of jobs in the background does not seem to have
a simple way to reliably find the pid of a particular process in the
pipeline (because not all processes are started when the shell
continues to execute).

The way PID of QEMU is derived can result in a failure waiting on a
PID that is not running. This is easier to hit with subsequent
multiple-migration support. Changing this to use $! by swapping the
pipeline for a fifo is more robust.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 scripts/arch-run.bash | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index c1dd67ab..9a5aaddc 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -130,19 +130,22 @@ run_migration ()
 	fi
 
 	trap 'trap - TERM ; kill 0 ; exit 2' INT TERM
-	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
+	trap 'rm -f ${migout1} ${migout_fifo1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
 
 	migsock=$(mktemp -u -t mig-helper-socket.XXXXXXXXXX)
 	migout1=$(mktemp -t mig-helper-stdout1.XXXXXXXXXX)
+	migout_fifo1=$(mktemp -u -t mig-helper-fifo-stdout1.XXXXXXXXXX)
 	qmp1=$(mktemp -u -t mig-helper-qmp1.XXXXXXXXXX)
 	qmp2=$(mktemp -u -t mig-helper-qmp2.XXXXXXXXXX)
 	fifo=$(mktemp -u -t mig-helper-fifo.XXXXXXXXXX)
 	qmpout1=/dev/null
 	qmpout2=/dev/null
 
+	mkfifo ${migout_fifo1}
 	eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
-		-mon chardev=mon1,mode=control | tee ${migout1} &
-	live_pid=`jobs -l %+ | grep "eval" | awk '{print$2}'`
+		-mon chardev=mon1,mode=control > ${migout_fifo1} &
+	live_pid=$!
+	cat ${migout_fifo1} | tee ${migout1} &
 
 	# We have to use cat to open the named FIFO, because named FIFO's, unlike
 	# pipes, will block on open() until the other end is also opened, and that
@@ -150,7 +153,7 @@ run_migration ()
 	mkfifo ${fifo}
 	eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
 		-mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat ${fifo}) &
-	incoming_pid=`jobs -l %+ | awk '{print$2}'`
+	incoming_pid=$!
 
 	# The test must prompt the user to migrate, so wait for the "migrate" keyword
 	while ! grep -q -i "Now migrate the VM" < ${migout1} ; do
@@ -164,6 +167,10 @@ run_migration ()
 		sleep 1
 	done
 
+	# Wait until the destination has created the incoming and qmp sockets
+	while ! [ -S ${migsock} ] ; do sleep 0.1 ; done
+	while ! [ -S ${qmp2} ] ; do sleep 0.1 ; done
+
 	qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
 
 	# Wait for the migration to complete
-- 
2.42.0


  parent reply	other threads:[~2024-02-09  9:14 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-09  9:11 [kvm-unit-tests PATCH v4 0/8] Multi-migration support Nicholas Piggin
2024-02-09  9:11 ` [kvm-unit-tests PATCH v4 1/8] arch-run: Fix TRAP handler recursion to remove temporary files properly Nicholas Piggin
2024-02-09  9:11 ` [kvm-unit-tests PATCH v4 2/8] arch-run: Clean up initrd cleanup Nicholas Piggin
2024-02-09 15:44   ` Thomas Huth
2024-02-09  9:11 ` Nicholas Piggin [this message]
2024-02-09  9:11 ` [kvm-unit-tests PATCH v4 4/8] migration: Support multiple migrations Nicholas Piggin
2024-02-09 17:57   ` Thomas Huth
2024-02-09  9:11 ` [kvm-unit-tests PATCH v4 5/8] arch-run: rename migration variables Nicholas Piggin
2024-02-09  9:11 ` [kvm-unit-tests PATCH v4 6/8] migration: Add quiet migration support Nicholas Piggin
2024-02-09 18:09   ` Thomas Huth
2024-02-09  9:11 ` [kvm-unit-tests PATCH v4 7/8] Add common/ directory for architecture-independent tests Nicholas Piggin
2024-02-09  9:11 ` [kvm-unit-tests PATCH v4 8/8] migration: add a migration selftest Nicholas Piggin
2024-02-16 11:15   ` Thomas Huth
2024-02-17  7:19     ` Nicholas Piggin
2024-02-19  6:56       ` Thomas Huth
2024-02-19 12:02         ` Nicholas Piggin
2024-02-16 14:05   ` Thomas Huth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240209091134.600228-4-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=alexandru.elisei@arm.com \
    --cc=andrew.jones@linux.dev \
    --cc=david@redhat.com \
    --cc=eric.auger@redhat.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-s390@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=lvivier@redhat.com \
    --cc=mhartmay@linux.ibm.com \
    --cc=nrb@linux.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=shahuang@redhat.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).