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@xxxxxxxxx>
> > ---
> > 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
