From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Huth <thuth@redhat.com>
Date: Fri, 9 Feb 2024 08:43:35 +0100
Subject: [kvm-unit-tests PATCH v3 3/8] migration: use a more robust way to
 wait for background job
In-Reply-To: <20240209070141.421569-4-npiggin@gmail.com>
References: <20240209070141.421569-1-npiggin@gmail.com>
 <20240209070141.421569-4-npiggin@gmail.com>
Message-ID: <9b041258-e412-4745-a213-6798e682ea62@redhat.com>
List-Id: <kvm-riscv.lists.infradead.org>
To: kvm-riscv@lists.infradead.org
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

On 09/02/2024 08.01, Nicholas Piggin wrote:
> 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.
> 
> 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 1e903e83..3689d7c2 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

Reviewed-by: Thomas Huth <thuth@redhat.com>