Re: [kvm-unit-tests PATCH 1/7] arch-run: Keep infifo open

Thomas Huth
2024-03-01T13:33:02Z

On 26/02/2024 10.38, Nicholas Piggin wrote:
> The infifo fifo that is used to send characters to QEMU console is
> only able to receive one character before the cat process exits.
> Supporting interactions between test and harness involving multiple
> characters requires the fifo to remain open.
>
> This also allows us to let the cat out of the bag, simplifying the
> input pipeline.

LOL, we rather let the cat out of the subshell now, but I like the play on
words :-)

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> scripts/arch-run.bash | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 6daef3218..e5b36a07b 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -158,6 +158,11 @@ run_migration ()
> mkfifo ${src_outfifo}
> mkfifo ${dst_outfifo}
>
> + # Holding both ends of the input fifo open prevents opens from
> + # blocking and readers getting EOF when a writer closes it.
> + mkfifo ${dst_infifo}
> + exec {dst_infifo_fd}<>${dst_infifo}
> +
> eval "$migcmdline" \
> -chardev socket,id=mon,path=${src_qmp},server=on,wait=off \
> -mon chardev=mon,mode=control > ${src_outfifo} &
> @@ -191,14 +196,10 @@ run_migration ()
>
> do_migration ()
> {
> - # 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 totally breaks QEMU...
> - mkfifo ${dst_infifo}
> eval "$migcmdline" \
> -chardev socket,id=mon,path=${dst_qmp},server=on,wait=off \
> -mon chardev=mon,mode=control -incoming unix:${dst_incoming} \
> - < <(cat ${dst_infifo}) > ${dst_outfifo} &
> + < ${dst_infifo} > ${dst_outfifo} &
> incoming_pid=$!
> cat ${dst_outfifo} | tee ${dst_out} | filter_quiet_msgs &
>
> @@ -245,7 +246,6 @@ do_migration ()
>
> # keypress to dst so getchar completes and test continues
> echo > ${dst_infifo}
> - rm ${dst_infifo}

I assume it will not get deleted by the trap handler? ... sounds fine to me,
so I dare to say:

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