[kvm-unit-tests PATCH v2 2/9] arch-run: Clean up temporary files properly

[Nicholas Piggin]

On Wed Feb 7, 2024 at 5:58 PM AEST, Thomas Huth wrote:
> On 02/02/2024 07.57, Nicholas Piggin wrote:
> > Migration files weren't being removed when tests were interrupted.
> > This improves the situation.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> > ---
> >   scripts/arch-run.bash | 12 +++++++-----
> >   1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > index d0864360..f22ead6f 100644
> > --- a/scripts/arch-run.bash
> > +++ b/scripts/arch-run.bash
> > @@ -134,12 +134,14 @@ run_migration ()
> >   	qmp1=$(mktemp -u -t mig-helper-qmp1.XXXXXXXXXX)
> >   	qmp2=$(mktemp -u -t mig-helper-qmp2.XXXXXXXXXX)
> >   	fifo=$(mktemp -u -t mig-helper-fifo.XXXXXXXXXX)
> > +
> > +	# race here between file creation and trap
> > +	trap "trap - TERM ; kill 0 ; exit 2" INT TERM
> > +	trap "rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}" RETURN EXIT
> > +
> >   	qmpout1=/dev/null
> >   	qmpout2=/dev/null
> >   
> > -	trap 'kill 0; exit 2' INT TERM
> > -	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
> > -
> >   	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}'`
> > @@ -211,8 +213,8 @@ run_panic ()
> >   
> >   	qmp=$(mktemp -u -t panic-qmp.XXXXXXXXXX)
> >   
> > -	trap 'kill 0; exit 2' INT TERM
> > -	trap 'rm -f ${qmp}' RETURN EXIT
> > +	trap "trap - TERM ; kill 0 ; exit 2" INT TERM
> > +	trap "rm -f ${qmp}" RETURN EXIT
> >   
> >   	# start VM stopped so we don't miss any events
> >   	eval "$@" -chardev socket,id=mon1,path=${qmp},server=on,wait=off \
>
> So the point is that the "EXIT" trap wasn't executed without the "trap - 
> TERM" in the other trap? ... ok, then your patch certainly makes sense.

Iff you don't remove the TERM handler then the kill will recursively
invoke it until some crash. This did solve some cases of dangling temp
files for me, although now I test with a simple script:

  #!/bin/bash

  trap 'echo "INT" ; kill 0 ; exit 2' INT
  trap 'trap - TERM ; echo "TERM" ; kill 0 ; exit 2' TERM
  trap 'echo "RETURN"' RETURN
  trap 'echo "EXIT"' EXIT

  sleep 10
  echo "done"

If you ^C it then it still doesn't get to the EXIT or RETURN handlers.
It looks like 'kill -INT $$' might be the way to do it instad of kill 0.

Not sure if that means my observation was incorrect, or if the real
script is behaving differently. In any case, I will dig into it and
try to explain more precisely in the changelog what it is fixing. And
possibly do another patch for the 'kill -INT $$' if that is needed.

Thanks,
Nick