diff mbox series

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

Message ID 20240226093832.1468383-2-npiggin@gmail.com (mailing list archive)
State New, archived
Headers show
Series more migration enhancements and tests | expand

Commit Message

Nicholas Piggin Feb. 26, 2024, 9:38 a.m. UTC
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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 scripts/arch-run.bash | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Thomas Huth March 1, 2024, 1:32 p.m. UTC | #1
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>
Nicholas Piggin March 5, 2024, 2:21 a.m. UTC | #2
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
diff mbox series

Patch

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}
 
 	# Ensure the incoming socket is removed, ready for next destination
 	if [ -S ${dst_incoming} ] ; then