| Message ID | 20230725033937.277156-3-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration: fixes and multiple migration | expand |
Quoting Nicholas Piggin (2023-07-25 05:39:36) > After starting the test, the harness waits polling for "migrate" in the > output. If the test does not print for some reason, the harness hangs. > > Test that the pid is still alive while polling to fix this hang. > > While here, wait for the full string "Now migrate the VM", which I think > makes it more obvious to read and could avoid an unfortunate collision > with some debugging output in a test case. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Thanks for attempting to fix this! > --- > scripts/arch-run.bash | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > index 518607f4..30e535c7 100644 > --- a/scripts/arch-run.bash > +++ b/scripts/arch-run.bash > @@ -142,6 +142,7 @@ run_migration () > > 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}'` Pardon my ignorance, but why would $! not work here?
On Fri Jul 28, 2023 at 5:34 PM AEST, Nico Boehr wrote: > Quoting Nicholas Piggin (2023-07-25 05:39:36) > > After starting the test, the harness waits polling for "migrate" in the > > output. If the test does not print for some reason, the harness hangs. > > > > Test that the pid is still alive while polling to fix this hang. > > > > While here, wait for the full string "Now migrate the VM", which I think > > makes it more obvious to read and could avoid an unfortunate collision > > with some debugging output in a test case. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > Thanks for attempting to fix this! > > > --- > > scripts/arch-run.bash | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > > index 518607f4..30e535c7 100644 > > --- a/scripts/arch-run.bash > > +++ b/scripts/arch-run.bash > > @@ -142,6 +142,7 @@ run_migration () > > > > 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}'` > > Pardon my ignorance, but why would $! not work here? My mastery of bash is poor, I copied the incoming_pid line. It seems to work, but if you think $! is better I can try it. Thanks, Nick
Quoting Nicholas Piggin (2023-07-30 12:03:36) > On Fri Jul 28, 2023 at 5:34 PM AEST, Nico Boehr wrote: > > Quoting Nicholas Piggin (2023-07-25 05:39:36) > > > After starting the test, the harness waits polling for "migrate" in the > > > output. If the test does not print for some reason, the harness hangs. > > > > > > Test that the pid is still alive while polling to fix this hang. > > > > > > While here, wait for the full string "Now migrate the VM", which I think > > > makes it more obvious to read and could avoid an unfortunate collision > > > with some debugging output in a test case. > > > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > > > Thanks for attempting to fix this! > > > > > --- > > > scripts/arch-run.bash | 10 +++++++++- > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > > > index 518607f4..30e535c7 100644 > > > --- a/scripts/arch-run.bash > > > +++ b/scripts/arch-run.bash > > > @@ -142,6 +142,7 @@ run_migration () > > > > > > 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}'` > > > > Pardon my ignorance, but why would $! not work here? > > My mastery of bash is poor, I copied the incoming_pid line. It seems > to work, but if you think $! is better I can try it. Sorry, this fell off of my radar after going to summer holiday... Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
On 25/09/2023 13.14, Nico Boehr wrote: > Quoting Nicholas Piggin (2023-07-30 12:03:36) >> On Fri Jul 28, 2023 at 5:34 PM AEST, Nico Boehr wrote: >>> Quoting Nicholas Piggin (2023-07-25 05:39:36) >>>> After starting the test, the harness waits polling for "migrate" in the >>>> output. If the test does not print for some reason, the harness hangs. >>>> >>>> Test that the pid is still alive while polling to fix this hang. >>>> >>>> While here, wait for the full string "Now migrate the VM", which I think >>>> makes it more obvious to read and could avoid an unfortunate collision >>>> with some debugging output in a test case. >>>> >>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>> >>> Thanks for attempting to fix this! >>> >>>> --- >>>> scripts/arch-run.bash | 10 +++++++++- >>>> 1 file changed, 9 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash >>>> index 518607f4..30e535c7 100644 >>>> --- a/scripts/arch-run.bash >>>> +++ b/scripts/arch-run.bash >>>> @@ -142,6 +142,7 @@ run_migration () >>>> >>>> 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}'` >>> >>> Pardon my ignorance, but why would $! not work here? >> >> My mastery of bash is poor, I copied the incoming_pid line. It seems >> to work, but if you think $! is better I can try it. > > Sorry, this fell off of my radar after going to summer holiday... > > Reviewed-by: Nico Boehr <nrb@linux.ibm.com> Hi Nicholas & Nico, do you want me to pick up this patch as is, or do you want to respin with $! instead? Thomas
Quoting Thomas Huth (2023-10-16 20:32:47) > On 25/09/2023 13.14, Nico Boehr wrote: > > Quoting Nicholas Piggin (2023-07-30 12:03:36) > >> On Fri Jul 28, 2023 at 5:34 PM AEST, Nico Boehr wrote: > >>> Quoting Nicholas Piggin (2023-07-25 05:39:36) > >>>> After starting the test, the harness waits polling for "migrate" in the > >>>> output. If the test does not print for some reason, the harness hangs. > >>>> > >>>> Test that the pid is still alive while polling to fix this hang. > >>>> > >>>> While here, wait for the full string "Now migrate the VM", which I think > >>>> makes it more obvious to read and could avoid an unfortunate collision > >>>> with some debugging output in a test case. > >>>> > >>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > >>> > >>> Thanks for attempting to fix this! > >>> > >>>> --- > >>>> scripts/arch-run.bash | 10 +++++++++- > >>>> 1 file changed, 9 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > >>>> index 518607f4..30e535c7 100644 > >>>> --- a/scripts/arch-run.bash > >>>> +++ b/scripts/arch-run.bash > >>>> @@ -142,6 +142,7 @@ run_migration () > >>>> > >>>> 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}'` > >>> > >>> Pardon my ignorance, but why would $! not work here? > >> > >> My mastery of bash is poor, I copied the incoming_pid line. It seems > >> to work, but if you think $! is better I can try it. > > > > Sorry, this fell off of my radar after going to summer holiday... > > > > Reviewed-by: Nico Boehr <nrb@linux.ibm.com> > > Hi Nicholas & Nico, > > do you want me to pick up this patch as is, or do you want to respin with $! > instead? Let's not discuss too much and get this fixed, I am fine with this as-is. Thanks.
On 19/10/2023 09.55, Nico Boehr wrote: > Quoting Thomas Huth (2023-10-16 20:32:47) >> On 25/09/2023 13.14, Nico Boehr wrote: >>> Quoting Nicholas Piggin (2023-07-30 12:03:36) >>>> On Fri Jul 28, 2023 at 5:34 PM AEST, Nico Boehr wrote: >>>>> Quoting Nicholas Piggin (2023-07-25 05:39:36) >>>>>> After starting the test, the harness waits polling for "migrate" in the >>>>>> output. If the test does not print for some reason, the harness hangs. >>>>>> >>>>>> Test that the pid is still alive while polling to fix this hang. >>>>>> >>>>>> While here, wait for the full string "Now migrate the VM", which I think >>>>>> makes it more obvious to read and could avoid an unfortunate collision >>>>>> with some debugging output in a test case. >>>>>> >>>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>>>> >>>>> Thanks for attempting to fix this! >>>>> >>>>>> --- >>>>>> scripts/arch-run.bash | 10 +++++++++- >>>>>> 1 file changed, 9 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash >>>>>> index 518607f4..30e535c7 100644 >>>>>> --- a/scripts/arch-run.bash >>>>>> +++ b/scripts/arch-run.bash >>>>>> @@ -142,6 +142,7 @@ run_migration () >>>>>> >>>>>> 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}'` >>>>> >>>>> Pardon my ignorance, but why would $! not work here? >>>> >>>> My mastery of bash is poor, I copied the incoming_pid line. It seems >>>> to work, but if you think $! is better I can try it. >>> >>> Sorry, this fell off of my radar after going to summer holiday... >>> >>> Reviewed-by: Nico Boehr <nrb@linux.ibm.com> >> >> Hi Nicholas & Nico, >> >> do you want me to pick up this patch as is, or do you want to respin with $! >> instead? > > Let's not discuss too much and get this fixed, I am fine with this as-is. > Thanks. Ok, pushed it now. Thomas
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index 518607f4..30e535c7 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -142,6 +142,7 @@ run_migration () 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}'` # 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 @@ -152,7 +153,14 @@ run_migration () incoming_pid=`jobs -l %+ | awk '{print$2}'` # The test must prompt the user to migrate, so wait for the "migrate" keyword - while ! grep -q -i "migrate" < ${migout1} ; do + while ! grep -q -i "Now migrate the VM" < ${migout1} ; do + if ! ps -p ${live_pid} > /dev/null ; then + echo "ERROR: Test exit before migration point." >&2 + echo > ${fifo} + qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null + qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null + return 3 + fi sleep 1 done
After starting the test, the harness waits polling for "migrate" in the output. If the test does not print for some reason, the harness hangs. Test that the pid is still alive while polling to fix this hang. While here, wait for the full string "Now migrate the VM", which I think makes it more obvious to read and could avoid an unfortunate collision with some debugging output in a test case. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- scripts/arch-run.bash | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)