From: Nicholas Piggin <npiggin@gmail.com> Date: 2024-02-02 06:58:52
There were not many comments on the previous post last year, so
this a rebase and resend. No significant change to migration patches,
but this rebases on Marc's better fix for cleaning auxinfo. So that
s390 patch is dropped, but added a minor fix for it instead :).
Multi migration works fine. And arm now has a reason to implement a
a getchar that can run more than 15 times.
Thanks,
Nick
Nicholas Piggin (9):
(arm|powerpc|s390x): Makefile: Fix .aux.o generation
arch-run: Clean up temporary files properly
arch-run: Clean up initrd cleanup
migration: use a more robust way to wait for background job
migration: Support multiple migrations
arch-run: rename migration variables
migration: Add quiet migration support
Add common/ directory for architecture-independent tests
migration: add a migration selftest
arm/Makefile.common | 3 +-
arm/sieve.c | 2 +-
arm/unittests.cfg | 6 ++
common/selftest-migration.c | 34 +++++++
common/sieve.c | 51 ++++++++++
lib/migrate.c | 20 +++-
lib/migrate.h | 2 +
powerpc/Makefile.common | 3 +-
powerpc/unittests.cfg | 4 +
s390x/Makefile | 3 +-
s390x/sieve.c | 2 +-
s390x/unittests.cfg | 4 +
scripts/arch-run.bash | 181 ++++++++++++++++++++++++++----------
x86/sieve.c | 52 +----------
14 files changed, 260 insertions(+), 107 deletions(-)
create mode 100644 common/selftest-migration.c
create mode 100644 common/sieve.c
mode change 100644 => 120000 x86/sieve.c
--
2.42.0
From: Nicholas Piggin <npiggin@gmail.com> Date: 2024-02-02 06:59:35
Using all prerequisites for the source file results in the build
dying on the second time around with:
gcc: fatal error: cannot specify ‘-o’ with ‘-c’, ‘-S’ or ‘-E’ with multiple files
This is due to auxinfo.h becoming a prerequisite after the first
build recorded the dependency.
Use the first prerequisite for this recipe.
Fixes: f2372f2d49135 ("(arm|powerpc|s390x): Makefile: add `%.aux.o` target")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arm/Makefile.common | 2 +-
powerpc/Makefile.common | 2 +-
s390x/Makefile | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2024-02-02 07:00:18
Migration files weren't being removed when tests were interrupted.
This improves the situation.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
scripts/arch-run.bash | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
@@ -134,12 +134,14 @@ run_migration ()qmp1=$(mktemp-u-tmig-helper-qmp1.XXXXXXXXXX)qmp2=$(mktemp-u-tmig-helper-qmp2.XXXXXXXXXX)fifo=$(mktemp-u-tmig-helper-fifo.XXXXXXXXXX)++# race here between file creation and trap+trap"trap - TERM ; kill 0 ; exit 2"INTTERM+trap"rm -f ${migout1}${migsock}${qmp1}${qmp2}${fifo}"RETURNEXIT+qmpout1=/dev/nullqmpout2=/dev/null-trap'kill 0; exit 2'INTTERM-trap'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}'RETURNEXIT-eval"$@"-chardevsocket,id=mon1,path=${qmp1},server=on,wait=off\-monchardev=mon1,mode=control|tee${migout1}&live_pid=`jobs-l%+|grep"eval"|awk'{print$2}'`
@@ -211,8 +213,8 @@ run_panic ()qmp=$(mktemp-u-tpanic-qmp.XXXXXXXXXX)-trap'kill 0; exit 2'INTTERM-trap'rm -f ${qmp}'RETURNEXIT+trap"trap - TERM ; kill 0 ; exit 2"INTTERM+trap"rm -f ${qmp}"RETURNEXIT# start VM stopped so we don't miss any eventseval"$@"-chardevsocket,id=mon1,path=${qmp},server=on,wait=off\
From: Nicholas Piggin <npiggin@gmail.com> Date: 2024-02-02 07:01:08
Rather than put a big script into the trap handler, have it call
a function.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
scripts/arch-run.bash | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2024-02-02 07:01:51
Starting a pipeline of jobs in the background does not seem to have
a simple way to reliably find the pid of a particular process in the
pipeline (because not all processes are started when the shell
continues to execute).
The way PID of QEMU is derived can result in a failure waiting on a
PID that is not running. This is easier to hit with subsequent
multiple-migration support. Changing this to use $! by swapping the
pipeline for a fifo is more robust.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
scripts/arch-run.bash | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
@@ -143,8 +144,9 @@ run_migration ()qmpout2=/dev/nulleval"$@"-chardevsocket,id=mon1,path=${qmp1},server=on,wait=off\--monchardev=mon1,mode=control|tee${migout1}&-live_pid=`jobs-l%+|grep"eval"|awk'{print$2}'`+-monchardev=mon1,mode=control>${migout_fifo1}&+live_pid=$!+cat${migout_fifo1}|tee${migout1}&# 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 +154,7 @@ run_migration ()mkfifo${fifo}eval"$@"-chardevsocket,id=mon2,path=${qmp2},server=on,wait=off\-monchardev=mon2,mode=control-incomingunix:${migsock}<<(cat${fifo})&-incoming_pid=`jobs-l%+|awk'{print$2}'`+incoming_pid=$!# The test must prompt the user to migrate, so wait for the "migrate" keywordwhile!grep-q-i"Now migrate the VM"<${migout1};do
@@ -166,6 +168,10 @@ run_migration ()sleep1done+# Wait until the destination has created the incoming and qmp sockets+while![-S${migsock}];dosleep0.1;done+while![-S${qmp2}];dosleep0.1;done+qmp${qmp1}'"migrate", "arguments": { "uri": "unix:'${migsock}'" }'>${qmpout1}# Wait for the migration to complete
From: Nicholas Piggin <npiggin@gmail.com> Date: 2024-02-02 07:02:34
Support multiple migrations by flipping dest file/socket variables to
source after the migration is complete, ready to start again. A new
destination is created if the test outputs the migrate line again.
Test cases may now switch to calling migrate() one or more times.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
lib/migrate.c | 8 ++--
lib/migrate.h | 1 +
scripts/arch-run.bash | 93 +++++++++++++++++++++++++++++++++++++------
3 files changed, 85 insertions(+), 17 deletions(-)
@@ -8,8 +8,10 @@#include<libcflat.h>#include"migrate.h"-/* static for now since we only support migrating exactly once per test. */-staticvoidmigrate(void)+/*+*Initiatemigrationandwaitforittocomplete.+*/+voidmigrate(void){puts("Now migrate the VM, then press a key to continue...\n");(void)getchar();
@@ -132,29 +132,76 @@ run_migration ()migsock=$(mktemp-u-tmig-helper-socket.XXXXXXXXXX)migout1=$(mktemp-tmig-helper-stdout1.XXXXXXXXXX)migout_fifo1=$(mktemp-u-tmig-helper-fifo-stdout1.XXXXXXXXXX)+migout2=$(mktemp-tmig-helper-stdout2.XXXXXXXXXX)+migout_fifo2=$(mktemp-u-tmig-helper-fifo-stdout2.XXXXXXXXXX)qmp1=$(mktemp-u-tmig-helper-qmp1.XXXXXXXXXX)qmp2=$(mktemp-u-tmig-helper-qmp2.XXXXXXXXXX)fifo=$(mktemp-u-tmig-helper-fifo.XXXXXXXXXX)# race here between file creation and traptrap"trap - TERM ; kill 0 ; exit 2"INTTERM-trap"rm -f ${migout1}${migsock}${qmp1}${qmp2}${fifo}"RETURNEXIT+trap"rm -f ${migout1}${migout2}${migout_fifo1}${migout_fifo2}${migsock}${qmp1}${qmp2}${fifo}"RETURNEXITqmpout1=/dev/nullqmpout2=/dev/null+migcmdline=$@-eval"$@"-chardevsocket,id=mon1,path=${qmp1},server=on,wait=off\+mkfifo${migout_fifo1}+mkfifo${migout_fifo2}++eval"$migcmdline"\+-chardevsocket,id=mon1,path=${qmp1},server=on,wait=off\-monchardev=mon1,mode=control>${migout_fifo1}&live_pid=$!cat${migout_fifo1}|tee${migout1}&-# 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...+# The test must prompt the user to migrate, so wait for the "migrate"+# keyword+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+qmp${qmp1}'"quit"'>${qmpout1}2>/dev/null+return3+fi+sleep0.1+done++# This starts the first source QEMU in advance of the test reaching the+# migration point, since we expect at least one migration. Subsequent+# sources are started as the test hits migrate keywords.+do_migration||return$?++whileps-p${live_pid}>/dev/null;do+# Wait for EXIT or further migrations+if!grep-q-i"Now migrate the VM"<${migout1};then+sleep0.1+else+do_migration||return$?+fi+done++wait${live_pid}+ret=$?++while(($(jobs-r|wc-l)>0));do+sleep0.1+done++return$ret+}++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${fifo}-eval"$@"-chardevsocket,id=mon2,path=${qmp2},server=on,wait=off\--monchardev=mon2,mode=control-incomingunix:${migsock}<<(cat${fifo})&+eval"$migcmdline"\+-chardevsocket,id=mon2,path=${qmp2},server=on,wait=off\+-monchardev=mon2,mode=control-incomingunix:${migsock}\+<<(cat${fifo})>${migout_fifo2}&incoming_pid=$!+cat${migout_fifo2}|tee${migout2}&# The test must prompt the user to migrate, so wait for the "migrate" keywordwhile!grep-q-i"Now migrate the VM"<${migout1};do
@@ -165,7 +212,7 @@ run_migration ()qmp${qmp2}'"quit"'>${qmpout2}2>/dev/nullreturn3fi-sleep1+sleep0.1done# Wait until the destination has created the incoming and qmp sockets
@@ -177,7 +224,7 @@ run_migration ()# Wait for the migration to completemigstatus=`qmp${qmp1}'"query-migrate"'|grepreturn`while!grep-q'"completed"'<<<"$migstatus";do-sleep1+sleep0.1if!migstatus=`qmp${qmp1}'"query-migrate"'`;thenecho"ERROR: Querying migration state failed.">&2echo>${fifo}
@@ -193,14 +240,34 @@ run_migration ()return2fidone+qmp${qmp1}'"quit"'>${qmpout1}2>/dev/null++# keypress to dst so getchar completes and test continuesecho>${fifo}-wait$incoming_pid+rm${fifo}++# Ensure the incoming socket is removed, ready for next destination+if[-S${migsock}];then+echo"ERROR: Incoming migration socket not removed after migration.">&2+qmp${qmp2}'"quit"'>${qmpout2}2>/dev/null+return2+fi++wait${live_pid}ret=$?-while(($(jobs-r|wc-l)>0));do-sleep0.5-done+# Now flip the variables because dest becomes source+live_pid=${incoming_pid}+tmp=${migout1}+migout1=${migout2}+migout2=${tmp}+tmp=${migout_fifo1}+migout_fifo1=${migout_fifo2}+migout_fifo2=${tmp}+tmp=${qmp1}+qmp1=${qmp2}+qmp2=${tmp}return$ret}
From: Nicholas Piggin <npiggin@gmail.com> Date: 2024-02-02 07:03:17
Using 1 and 2 for source and destination is confusing, particularly
now with multiple migrations that flip between them. Do a rename
pass to tidy things up.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
scripts/arch-run.bash | 112 +++++++++++++++++++++---------------------
1 file changed, 57 insertions(+), 55 deletions(-)
@@ -129,38 +129,39 @@ run_migration ()return77fi-migsock=$(mktemp-u-tmig-helper-socket.XXXXXXXXXX)-migout1=$(mktemp-tmig-helper-stdout1.XXXXXXXXXX)-migout_fifo1=$(mktemp-u-tmig-helper-fifo-stdout1.XXXXXXXXXX)-migout2=$(mktemp-tmig-helper-stdout2.XXXXXXXXXX)-migout_fifo2=$(mktemp-u-tmig-helper-fifo-stdout2.XXXXXXXXXX)-qmp1=$(mktemp-u-tmig-helper-qmp1.XXXXXXXXXX)-qmp2=$(mktemp-u-tmig-helper-qmp2.XXXXXXXXXX)-fifo=$(mktemp-u-tmig-helper-fifo.XXXXXXXXXX)+dst_incoming=$(mktemp-u-tmig-helper-socket-incoming.XXXXXXXXXX)+src_out=$(mktemp-tmig-helper-stdout1.XXXXXXXXXX)+src_outfifo=$(mktemp-u-tmig-helper-fifo-stdout1.XXXXXXXXXX)+dst_out=$(mktemp-tmig-helper-stdout2.XXXXXXXXXX)+dst_outfifo=$(mktemp-u-tmig-helper-fifo-stdout2.XXXXXXXXXX)+src_qmp=$(mktemp-u-tmig-helper-qmp1.XXXXXXXXXX)+dst_qmp=$(mktemp-u-tmig-helper-qmp2.XXXXXXXXXX)+dst_infifo=$(mktemp-u-tmig-helper-fifo-stdin.XXXXXXXXXX)# race here between file creation and traptrap"trap - TERM ; kill 0 ; exit 2"INTTERM-trap"rm -f ${migout1}${migout2}${migout_fifo1}${migout_fifo2}${migsock}${qmp1}${qmp2}${fifo}"RETURNEXIT+trap"rm -f ${src_out}${dst_out}${src_outfifo}${dst_outfifo}${dst_incoming}${src_qmp}${dst_qmp}${dst_infifo}"RETURNEXIT++src_qmpout=/dev/null+dst_qmpout=/dev/null-qmpout1=/dev/null-qmpout2=/dev/nullmigcmdline=$@-mkfifo${migout_fifo1}-mkfifo${migout_fifo2}+mkfifo${src_outfifo}+mkfifo${dst_outfifo}eval"$migcmdline"\--chardevsocket,id=mon1,path=${qmp1},server=on,wait=off\--monchardev=mon1,mode=control>${migout_fifo1}&+-chardevsocket,id=mon,path=${src_qmp},server=on,wait=off\+-monchardev=mon,mode=control>${src_outfifo}&live_pid=$!-cat${migout_fifo1}|tee${migout1}&+cat${src_outfifo}|tee${src_out}&# The test must prompt the user to migrate, so wait for the "migrate"# keyword-while!grep-q-i"Now migrate the VM"<${migout1};do+while!grep-q-i"Now migrate the VM"<${src_out};doif!ps-p${live_pid}>/dev/null;thenecho"ERROR: Test exit before migration point.">&2-qmp${qmp1}'"quit"'>${qmpout1}2>/dev/null+qmp${src_qmp}'"quit"'>${src_qmpout}2>/dev/nullreturn3fisleep0.1
@@ -173,7 +174,7 @@ run_migration ()whileps-p${live_pid}>/dev/null;do# Wait for EXIT or further migrations-if!grep-q-i"Now migrate the VM"<${migout1};then+if!grep-q-i"Now migrate the VM"<${src_out};thensleep0.1elsedo_migration||return$?
@@ -195,79 +196,80 @@ 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${fifo}+mkfifo${dst_infifo}eval"$migcmdline"\--chardevsocket,id=mon2,path=${qmp2},server=on,wait=off\--monchardev=mon2,mode=control-incomingunix:${migsock}\-<<(cat${fifo})>${migout_fifo2}&+-chardevsocket,id=mon,path=${dst_qmp},server=on,wait=off\+-monchardev=mon,mode=control-incomingunix:${dst_incoming}\+<<(cat${dst_infifo})>${dst_outfifo}&incoming_pid=$!-cat${migout_fifo2}|tee${migout2}&+cat${dst_outfifo}|tee${dst_out}&# The test must prompt the user to migrate, so wait for the "migrate" keyword-while!grep-q-i"Now migrate the VM"<${migout1};do+while!grep-q-i"Now migrate the VM"<${src_out};doif!ps-p${live_pid}>/dev/null;thenecho"ERROR: Test exit before migration point.">&2-echo>${fifo}-qmp${qmp1}'"quit"'>${qmpout1}2>/dev/null-qmp${qmp2}'"quit"'>${qmpout2}2>/dev/null+echo>${dst_infifo}+qmp${src_qmp}'"quit"'>${src_qmpout}2>/dev/null+qmp${dst_qmp}'"quit"'>${dst_qmpout}2>/dev/nullreturn3fisleep0.1done# Wait until the destination has created the incoming and qmp sockets-while![-S${migsock}];dosleep0.1;done-while![-S${qmp2}];dosleep0.1;done+while![-S${dst_incoming}];dosleep0.1;done+while![-S${dst_qmp}];dosleep0.1;done-qmp${qmp1}'"migrate", "arguments": { "uri": "unix:'${migsock}'" }'>${qmpout1}+qmp${src_qmp}'"migrate", "arguments": { "uri": "unix:'${dst_incoming}'" }'>${src_qmpout}# Wait for the migration to complete-migstatus=`qmp${qmp1}'"query-migrate"'|grepreturn`+migstatus=`qmp${src_qmp}'"query-migrate"'|grepreturn`while!grep-q'"completed"'<<<"$migstatus";dosleep0.1-if!migstatus=`qmp${qmp1}'"query-migrate"'`;then+if!migstatus=`qmp${src_qmp}'"query-migrate"'`;thenecho"ERROR: Querying migration state failed.">&2-echo>${fifo}-qmp${qmp2}'"quit"'>${qmpout2}2>/dev/null+echo>${dst_infifo}+qmp${dst_qmp}'"quit"'>${dst_qmpout}2>/dev/nullreturn2fimigstatus=`grepreturn<<<"$migstatus"`ifgrep-q'"failed"'<<<"$migstatus";thenecho"ERROR: Migration failed.">&2-echo>${fifo}-qmp${qmp1}'"quit"'>${qmpout1}2>/dev/null-qmp${qmp2}'"quit"'>${qmpout2}2>/dev/null+echo>${dst_infifo}+qmp${src_qmp}'"quit"'>${src_qmpout}2>/dev/null+qmp${dst_qmp}'"quit"'>${dst_qmpout}2>/dev/nullreturn2fidone-qmp${qmp1}'"quit"'>${qmpout1}2>/dev/null+qmp${src_qmp}'"quit"'>${src_qmpout}2>/dev/null# keypress to dst so getchar completes and test continues-echo>${fifo}-rm${fifo}+echo>${dst_infifo}+rm${dst_infifo}# Ensure the incoming socket is removed, ready for next destination-if[-S${migsock}];then+if[-S${dst_incoming}];thenecho"ERROR: Incoming migration socket not removed after migration.">&2-qmp${qmp2}'"quit"'>${qmpout2}2>/dev/null+qmp${dst_qmp}'"quit"'>${dst_qmpout}2>/dev/nullreturn2fiwait${live_pid}ret=$?-# Now flip the variables because dest becomes source+# Now flip the variables because destination machine becomes source+# for the next migration.live_pid=${incoming_pid}-tmp=${migout1}-migout1=${migout2}-migout2=${tmp}-tmp=${migout_fifo1}-migout_fifo1=${migout_fifo2}-migout_fifo2=${tmp}-tmp=${qmp1}-qmp1=${qmp2}-qmp2=${tmp}+tmp=${src_out}+src_out=${dst_out}+dst_out=${tmp}+tmp=${src_outfifo}+src_outfifo=${dst_outfifo}+dst_outfifo=${tmp}+tmp=${src_qmp}+src_qmp=${dst_qmp}+dst_qmp=${tmp}return$ret}
@@ -290,8 +292,8 @@ run_panic ()trap"rm -f ${qmp}"RETURNEXIT# start VM stopped so we don't miss any events-eval"$@"-chardevsocket,id=mon1,path=${qmp},server=on,wait=off\--monchardev=mon1,mode=control-S&+eval"$@"-chardevsocket,id=mon,path=${qmp},server=on,wait=off\+-monchardev=mon,mode=control-S&panic_event_count=$(qmp_events${qmp}|jq-c'select(.event == "GUEST_PANICKED")'|wc-l)if["$panic_event_count"-lt1];then
@@ -154,7 +154,7 @@ run_migration ()-chardevsocket,id=mon,path=${src_qmp},server=on,wait=off\-monchardev=mon,mode=control>${src_outfifo}&live_pid=$!-cat${src_outfifo}|tee${src_out}&+cat${src_outfifo}|tee${src_out}|grep-v"Now migrate the VM (quiet)"&# The test must prompt the user to migrate, so wait for the "migrate"# keyword
@@ -202,7 +202,7 @@ do_migration ()-monchardev=mon,mode=control-incomingunix:${dst_incoming}\<<(cat${dst_infifo})>${dst_outfifo}&incoming_pid=$!-cat${dst_outfifo}|tee${dst_out}&+cat${dst_outfifo}|tee${dst_out}|grep-v"Now migrate the VM (quiet)"&# The test must prompt the user to migrate, so wait for the "migrate" keywordwhile!grep-q-i"Now migrate the VM"<${src_out};do
From: Nicholas Piggin <npiggin@gmail.com> Date: 2024-02-02 07:04:49
x86/sieve.c is used by s390x and arm via symbolic link. Make a new
directory common/ for architecture-independent tests and move
sieve.c here.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arm/sieve.c | 2 +-
common/sieve.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
s390x/sieve.c | 2 +-
x86/sieve.c | 52 +-------------------------------------------------
4 files changed, 54 insertions(+), 53 deletions(-)
create mode 100644 common/sieve.c
mode change 100644 => 120000 x86/sieve.c
From: Nicholas Piggin <npiggin@gmail.com> Date: 2024-02-02 07:05:31
Add a selftest for migration support in guest library and test harness
code. It performs migrations a tight loop to irritate races and bugs in
the test harness code.
Acked-by: Claudio Imbrenda <imbrenda@linux.ibm.com> (s390x)
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
This has flushed out several bugs in developing the multi migration test
harness code already.
---
arm/Makefile.common | 1 +
arm/unittests.cfg | 6 ++++++
common/selftest-migration.c | 34 ++++++++++++++++++++++++++++++++++
powerpc/Makefile.common | 1 +
powerpc/unittests.cfg | 4 ++++
s390x/Makefile | 1 +
s390x/unittests.cfg | 4 ++++
7 files changed, 51 insertions(+)
create mode 100644 common/selftest-migration.c
@@ -24,6 +24,10 @@ groups = selftest# please keep the kernel cmdline in sync with $(TEST_DIR)/selftest.parmfileextra_params=-append 'test 123'+[selftest-migration]+file=selftest-migration.elf+groups=selftest migration+[intercept]file=intercept.elf
From: Andrew Jones <hidden> Date: 2024-02-02 09:41:24
On Fri, Feb 02, 2024 at 04:57:32PM +1000, Nicholas Piggin wrote:
quoted hunk
Using all prerequisites for the source file results in the build
dying on the second time around with:
gcc: fatal error: cannot specify ‘-o’ with ‘-c’, ‘-S’ or ‘-E’ with multiple files
This is due to auxinfo.h becoming a prerequisite after the first
build recorded the dependency.
Use the first prerequisite for this recipe.
Fixes: f2372f2d49135 ("(arm|powerpc|s390x): Makefile: add `%.aux.o` target")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arm/Makefile.common | 2 +-
powerpc/Makefile.common | 2 +-
s390x/Makefile | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
There are two instances of the %.aux.o target in arm/Makefile.common. We
need to fix both. We can actually pull the target out of the two arms of
the CONFIG_EFI if-else, though, by changing the .efi/.flat to .$(exe).
Thanks,
drew
From: Thomas Huth <hidden> Date: 2024-02-05 11:29:41
On 02/02/2024 10.30, Andrew Jones wrote:
On Fri, Feb 02, 2024 at 04:57:32PM +1000, Nicholas Piggin wrote:
quoted
Using all prerequisites for the source file results in the build
dying on the second time around with:
gcc: fatal error: cannot specify ‘-o’ with ‘-c’, ‘-S’ or ‘-E’ with multiple files
This is due to auxinfo.h becoming a prerequisite after the first
build recorded the dependency.
D'oh, of course I only tried to run "make" once when testing that patch :-/
There are two instances of the %.aux.o target in arm/Makefile.common. We
need to fix both. We can actually pull the target out of the two arms of
the CONFIG_EFI if-else, though, by changing the .efi/.flat to .$(exe).
I went ahead and pushed this patch with the trivial fix for the else-branch
to the repo to unbreak the build. If you think it's worthwhile to unify the
target, please provide a patch to do so, thanks!
Thomas
From: Thomas Huth <hidden> Date: 2024-02-05 12:05:22
On 02/02/2024 07.57, Nicholas Piggin wrote:
quoted hunk
Rather than put a big script into the trap handler, have it call
a function.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
scripts/arch-run.bash | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
Why don't you move the 'rm -f $KVM_UNIT_TESTS_ENV' into the initrd_cleanup()
function, too? ... that would IMHO make more sense for a function that is
called *_cleanup() ?
Thomas
From: Marc Hartmayer <hidden> Date: 2024-02-05 17:38:00
On Fri, Feb 02, 2024 at 04:57 PM +1000, Nicholas Piggin [off-list ref] wrote:
Using all prerequisites for the source file results in the build
dying on the second time around with:
gcc: fatal error: cannot specify ‘-o’ with ‘-c’, ‘-S’ or ‘-E’ with multiple files
This is due to auxinfo.h becoming a prerequisite after the first
build recorded the dependency.
Use the first prerequisite for this recipe.
Fixes: f2372f2d49135 ("(arm|powerpc|s390x): Makefile: add `%.aux.o` target")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arm/Makefile.common | 2 +-
[…snip]
Thanks a ton for fixing this!
Reviewed-by: Marc Hartmayer <redacted>
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
From: Marc Hartmayer <hidden> Date: 2024-02-05 17:38:43
On Fri, Feb 02, 2024 at 04:57 PM +1000, Nicholas Piggin [off-list ref] wrote:
Starting a pipeline of jobs in the background does not seem to have
a simple way to reliably find the pid of a particular process in the
pipeline (because not all processes are started when the shell
continues to execute).
The way PID of QEMU is derived can result in a failure waiting on a
PID that is not running. This is easier to hit with subsequent
multiple-migration support. Changing this to use $! by swapping the
pipeline for a fifo is more robust.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
[…snip…]
+ # Wait until the destination has created the incoming and qmp sockets
+ while ! [ -S ${migsock} ] ; do sleep 0.1 ; done
+ while ! [ -S ${qmp2} ] ; do sleep 0.1 ; done
There should be timeout implemented, otherwise we might end in an
endless loop in case of a bug. Or is the global timeout good enough to
handle this situation?
+
qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
# Wait for the migration to complete
--
2.42.0
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
On Mon Feb 5, 2024 at 10:04 PM AEST, Thomas Huth wrote:
On 02/02/2024 07.57, Nicholas Piggin wrote:
quoted
Rather than put a big script into the trap handler, have it call
a function.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
scripts/arch-run.bash | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
Why don't you move the 'rm -f $KVM_UNIT_TESTS_ENV' into the initrd_cleanup()
function, too? ... that would IMHO make more sense for a function that is
called *_cleanup() ?
On Tue Feb 6, 2024 at 12:58 AM AEST, Marc Hartmayer wrote:
On Fri, Feb 02, 2024 at 04:57 PM +1000, Nicholas Piggin [off-list ref] wrote:
quoted
Starting a pipeline of jobs in the background does not seem to have
a simple way to reliably find the pid of a particular process in the
pipeline (because not all processes are started when the shell
continues to execute).
The way PID of QEMU is derived can result in a failure waiting on a
PID that is not running. This is easier to hit with subsequent
multiple-migration support. Changing this to use $! by swapping the
pipeline for a fifo is more robust.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
[…snip…]
quoted
+ # Wait until the destination has created the incoming and qmp sockets
+ while ! [ -S ${migsock} ] ; do sleep 0.1 ; done
+ while ! [ -S ${qmp2} ] ; do sleep 0.1 ; done
There should be timeout implemented, otherwise we might end in an
endless loop in case of a bug. Or is the global timeout good enough to
handle this situation?
I was going to say it's not worthwhile since we can't recover, but
actually printing where the timeout happens if nothing else would
be pretty helpful to gather and diagnose problems especially ones
we can't reproduce locally. So, yeah good idea.
We have a bunch of potential hangs where we don't do anything already
though. Sadly it doesn't look like $BASH_LINENO can give anything
useful of the interrupted context from a SIGHUP trap. We might be able
to do something like -
timeout_handler() {
echo "Timeout $timeout_msg"
exit
}
trap timeout_handler HUP
timeout_msg="waiting for destination migration socket to be created"
while ! [ -S ${migsock} ] ; do sleep 0.1 ; done
timeout_msg="waiting for destination QMP socket to be created"
while ! [ -S ${qmp2} ] ; do sleep 0.1 ; done
timeout_msg=
Unless you have any better ideas. Not sure if there's some useful
bash debugging options that can be used. Other option is adding timeout
checks in loops and blocking commands... not sure if that's simpler and
less error prone though.
Anyway we have a bunch of potential hangs and timeouts that aren't
handled already though, so I might leave this out for a later pass at
it unless we come up with a really nice easy way to go.
Thanks,
Nick
quoted
+
qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
# Wait for the migration to complete
--
2.42.0
From: Thomas Huth <hidden> Date: 2024-02-07 07:59:24
On 02/02/2024 07.57, Nicholas Piggin wrote:
quoted hunk
Migration files weren't being removed when tests were interrupted.
This improves the situation.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
scripts/arch-run.bash | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
@@ -134,12 +134,14 @@ run_migration ()qmp1=$(mktemp-u-tmig-helper-qmp1.XXXXXXXXXX)qmp2=$(mktemp-u-tmig-helper-qmp2.XXXXXXXXXX)fifo=$(mktemp-u-tmig-helper-fifo.XXXXXXXXXX)++# race here between file creation and trap+trap"trap - TERM ; kill 0 ; exit 2"INTTERM+trap"rm -f ${migout1}${migsock}${qmp1}${qmp2}${fifo}"RETURNEXIT+qmpout1=/dev/nullqmpout2=/dev/null-trap'kill 0; exit 2'INTTERM-trap'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}'RETURNEXIT-eval"$@"-chardevsocket,id=mon1,path=${qmp1},server=on,wait=off\-monchardev=mon1,mode=control|tee${migout1}&live_pid=`jobs-l%+|grep"eval"|awk'{print$2}'`
@@ -211,8 +213,8 @@ run_panic ()qmp=$(mktemp-u-tpanic-qmp.XXXXXXXXXX)-trap'kill 0; exit 2'INTTERM-trap'rm -f ${qmp}'RETURNEXIT+trap"trap - TERM ; kill 0 ; exit 2"INTTERM+trap"rm -f ${qmp}"RETURNEXIT# start VM stopped so we don't miss any eventseval"$@"-chardevsocket,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.
Reviewed-by: Thomas Huth <redacted>
On Wed Feb 7, 2024 at 5:58 PM AEST, Thomas Huth wrote:
On 02/02/2024 07.57, Nicholas Piggin wrote:
quoted
Migration files weren't being removed when tests were interrupted.
This improves the situation.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
scripts/arch-run.bash | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
@@ -134,12 +134,14 @@ run_migration ()qmp1=$(mktemp-u-tmig-helper-qmp1.XXXXXXXXXX)qmp2=$(mktemp-u-tmig-helper-qmp2.XXXXXXXXXX)fifo=$(mktemp-u-tmig-helper-fifo.XXXXXXXXXX)++# race here between file creation and trap+trap"trap - TERM ; kill 0 ; exit 2"INTTERM+trap"rm -f ${migout1}${migsock}${qmp1}${qmp2}${fifo}"RETURNEXIT+qmpout1=/dev/nullqmpout2=/dev/null-trap'kill 0; exit 2'INTTERM-trap'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}'RETURNEXIT-eval"$@"-chardevsocket,id=mon1,path=${qmp1},server=on,wait=off\-monchardev=mon1,mode=control|tee${migout1}&live_pid=`jobs-l%+|grep"eval"|awk'{print$2}'`
@@ -211,8 +213,8 @@ run_panic ()qmp=$(mktemp-u-tpanic-qmp.XXXXXXXXXX)-trap'kill 0; exit 2'INTTERM-trap'rm -f ${qmp}'RETURNEXIT+trap"trap - TERM ; kill 0 ; exit 2"INTTERM+trap"rm -f ${qmp}"RETURNEXIT# start VM stopped so we don't miss any eventseval"$@"-chardevsocket,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