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

["Nicholas Piggin" <npiggin@gmail.com>]

On Fri Mar 1, 2024 at 11:32 PM AEST, Thomas Huth wrote:
> 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 :-)

It was a bit of a stretch, but I'm glad you liked it :) I may
incorporate your suggestion to improve it.

>
> > 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:

Yep, deleted by trap handler.

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

Thanks,
Nick