* [kvm-unit-tests RFC 0/2] scripts: Fix accel handling @ 2021-03-18 12:44 Janosch Frank 2021-03-18 12:44 ` [kvm-unit-tests RFC 1/2] scripts: Check kvm availability by asking qemu Janosch Frank 2021-03-18 12:45 ` [kvm-unit-tests RFC 2/2] scripts: Set ACCEL in run_tests.sh if empty Janosch Frank 0 siblings, 2 replies; 8+ messages in thread From: Janosch Frank @ 2021-03-18 12:44 UTC (permalink / raw) To: kvm; +Cc: lvivier, thuth, david, drjones, pbonzini, cohuck When running on a system without KVM but with a /dev/kvm file or when /dev/kvm has the wrong permissions we will think that we have the kvm accelerator because we only check if /dev/kvm exists. To fix that we instead start a qemu with the kvm accel and check the exit value we can check if kvm is available. Also we only compare the accel specified in unittests.conf with the env ACCEL. That won't help if we don't have kvm but a test has KVM as a requirement in unittests.conf. My bash knowledge is rather limited, so maybe there's a better solution? Janosch Frank (2): scripts: Check kvm availability by asking qemu scripts: Set ACCEL in run_tests.sh if empty arm/run | 4 +-- powerpc/run | 4 +-- run_tests.sh | 6 +++++ s390x/run | 10 ++++--- scripts/accel.bash | 63 +++++++++++++++++++++++++++++++++++++++++++ scripts/arch-run.bash | 63 ++----------------------------------------- scripts/runtime.bash | 2 +- x86/run | 4 +-- 8 files changed, 85 insertions(+), 71 deletions(-) create mode 100644 scripts/accel.bash -- 2.27.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [kvm-unit-tests RFC 1/2] scripts: Check kvm availability by asking qemu 2021-03-18 12:44 [kvm-unit-tests RFC 0/2] scripts: Fix accel handling Janosch Frank @ 2021-03-18 12:44 ` Janosch Frank 2021-03-18 14:18 ` Andrew Jones 2021-03-18 15:31 ` Andrew Jones 2021-03-18 12:45 ` [kvm-unit-tests RFC 2/2] scripts: Set ACCEL in run_tests.sh if empty Janosch Frank 1 sibling, 2 replies; 8+ messages in thread From: Janosch Frank @ 2021-03-18 12:44 UTC (permalink / raw) To: kvm; +Cc: lvivier, thuth, david, drjones, pbonzini, cohuck The existence of the /dev/kvm character device doesn't imply that the kvm module is part of the kernel or that it's always magically loaded when needed. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- arm/run | 4 ++-- powerpc/run | 4 ++-- s390x/run | 4 ++-- scripts/arch-run.bash | 7 +++++-- x86/run | 4 ++-- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/arm/run b/arm/run index a390ca5a..ca2d44e0 100755 --- a/arm/run +++ b/arm/run @@ -10,10 +10,10 @@ if [ -z "$STANDALONE" ]; then fi processor="$PROCESSOR" -ACCEL=$(get_qemu_accelerator) || +qemu=$(search_qemu_binary) || exit $? -qemu=$(search_qemu_binary) || +ACCEL=$(get_qemu_accelerator) || exit $? if ! $qemu -machine '?' 2>&1 | grep 'ARM Virtual Machine' > /dev/null; then diff --git a/powerpc/run b/powerpc/run index 597ab96e..b51ac6fc 100755 --- a/powerpc/run +++ b/powerpc/run @@ -9,10 +9,10 @@ if [ -z "$STANDALONE" ]; then source scripts/arch-run.bash fi -ACCEL=$(get_qemu_accelerator) || +qemu=$(search_qemu_binary) || exit $? -qemu=$(search_qemu_binary) || +ACCEL=$(get_qemu_accelerator) || exit $? if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then diff --git a/s390x/run b/s390x/run index 09805044..2ec6da70 100755 --- a/s390x/run +++ b/s390x/run @@ -9,10 +9,10 @@ if [ -z "$STANDALONE" ]; then source scripts/arch-run.bash fi -ACCEL=$(get_qemu_accelerator) || +qemu=$(search_qemu_binary) || exit $? -qemu=$(search_qemu_binary) || +ACCEL=$(get_qemu_accelerator) || exit $? M='-machine s390-ccw-virtio' diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index 5997e384..8cc9a61e 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -342,8 +342,11 @@ trap_exit_push () kvm_available () { - [ -c /dev/kvm ] || - return 1 + if $($qemu -accel kvm 2> /dev/null); then + return 0; + else + return 1; + fi [ "$HOST" = "$ARCH_NAME" ] || ( [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ) || diff --git a/x86/run b/x86/run index 8b2425f4..cbf49143 100755 --- a/x86/run +++ b/x86/run @@ -9,10 +9,10 @@ if [ -z "$STANDALONE" ]; then source scripts/arch-run.bash fi -ACCEL=$(get_qemu_accelerator) || +qemu=$(search_qemu_binary) || exit $? -qemu=$(search_qemu_binary) || +ACCEL=$(get_qemu_accelerator) || exit $? if ! ${qemu} -device '?' 2>&1 | grep -F -e \"testdev\" -e \"pc-testdev\" > /dev/null; -- 2.27.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] scripts: Check kvm availability by asking qemu 2021-03-18 12:44 ` [kvm-unit-tests RFC 1/2] scripts: Check kvm availability by asking qemu Janosch Frank @ 2021-03-18 14:18 ` Andrew Jones 2021-03-18 15:31 ` Andrew Jones 1 sibling, 0 replies; 8+ messages in thread From: Andrew Jones @ 2021-03-18 14:18 UTC (permalink / raw) To: Janosch Frank; +Cc: kvm, lvivier, thuth, david, pbonzini, cohuck On Thu, Mar 18, 2021 at 12:44:59PM +0000, Janosch Frank wrote: > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > index 5997e384..8cc9a61e 100644 > --- a/scripts/arch-run.bash > +++ b/scripts/arch-run.bash > @@ -342,8 +342,11 @@ trap_exit_push () > > kvm_available () > { > - [ -c /dev/kvm ] || > - return 1 > + if $($qemu -accel kvm 2> /dev/null); then > + return 0; > + else > + return 1; > + fi > Hi Janosch, Are we sure that even old QEMU supports this type of probing? If not, then we should probably keep the /dev/kvm test as a fallback. Thanks, drew ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] scripts: Check kvm availability by asking qemu 2021-03-18 12:44 ` [kvm-unit-tests RFC 1/2] scripts: Check kvm availability by asking qemu Janosch Frank 2021-03-18 14:18 ` Andrew Jones @ 2021-03-18 15:31 ` Andrew Jones 2021-03-18 15:39 ` Janosch Frank 2021-03-24 14:11 ` Janosch Frank 1 sibling, 2 replies; 8+ messages in thread From: Andrew Jones @ 2021-03-18 15:31 UTC (permalink / raw) To: Janosch Frank; +Cc: kvm, lvivier, thuth, david, pbonzini, cohuck On Thu, Mar 18, 2021 at 12:44:59PM +0000, Janosch Frank wrote: > The existence of the /dev/kvm character device doesn't imply that the > kvm module is part of the kernel or that it's always magically loaded > when needed. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > arm/run | 4 ++-- > powerpc/run | 4 ++-- > s390x/run | 4 ++-- > scripts/arch-run.bash | 7 +++++-- > x86/run | 4 ++-- > 5 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/arm/run b/arm/run > index a390ca5a..ca2d44e0 100755 > --- a/arm/run > +++ b/arm/run > @@ -10,10 +10,10 @@ if [ -z "$STANDALONE" ]; then > fi > processor="$PROCESSOR" > > -ACCEL=$(get_qemu_accelerator) || > +qemu=$(search_qemu_binary) || > exit $? > > -qemu=$(search_qemu_binary) || > +ACCEL=$(get_qemu_accelerator) || > exit $? How about renaming search_qemu_binary() to set_qemu_accelerator(), which would also ensure QEMU is set (if it doesn't error out on failure) and then call that from get_qemu_accelerator()? That way we don't need to worry about this order of calls nor this lowercase 'qemu' variable being set. Also, we can rename get_qemu_accelerator() to set_qemu_accelerator() and ensure it sets ACCEL. Thanks, drew ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] scripts: Check kvm availability by asking qemu 2021-03-18 15:31 ` Andrew Jones @ 2021-03-18 15:39 ` Janosch Frank 2021-03-24 14:11 ` Janosch Frank 1 sibling, 0 replies; 8+ messages in thread From: Janosch Frank @ 2021-03-18 15:39 UTC (permalink / raw) To: Andrew Jones; +Cc: kvm, lvivier, thuth, david, pbonzini, cohuck On 3/18/21 4:31 PM, Andrew Jones wrote: > On Thu, Mar 18, 2021 at 12:44:59PM +0000, Janosch Frank wrote: >> The existence of the /dev/kvm character device doesn't imply that the >> kvm module is part of the kernel or that it's always magically loaded >> when needed. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> arm/run | 4 ++-- >> powerpc/run | 4 ++-- >> s390x/run | 4 ++-- >> scripts/arch-run.bash | 7 +++++-- >> x86/run | 4 ++-- >> 5 files changed, 13 insertions(+), 10 deletions(-) >> >> diff --git a/arm/run b/arm/run >> index a390ca5a..ca2d44e0 100755 >> --- a/arm/run >> +++ b/arm/run >> @@ -10,10 +10,10 @@ if [ -z "$STANDALONE" ]; then >> fi >> processor="$PROCESSOR" >> >> -ACCEL=$(get_qemu_accelerator) || >> +qemu=$(search_qemu_binary) || >> exit $? >> >> -qemu=$(search_qemu_binary) || >> +ACCEL=$(get_qemu_accelerator) || >> exit $? > > How about renaming search_qemu_binary() to set_qemu_accelerator(), which > would also ensure QEMU is set (if it doesn't error out on failure) and > then call that from get_qemu_accelerator()? That way we don't need to > worry about this order of calls nor this lowercase 'qemu' variable being > set. Also, we can rename get_qemu_accelerator() to set_qemu_accelerator() > and ensure it sets ACCEL. Sure, I was already considering that. The fact that qemu and ACCEL are basically global and accel is not the same as ACCEL makes all of this harder to understand than it should be. > > Thanks, > drew > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] scripts: Check kvm availability by asking qemu 2021-03-18 15:31 ` Andrew Jones 2021-03-18 15:39 ` Janosch Frank @ 2021-03-24 14:11 ` Janosch Frank 1 sibling, 0 replies; 8+ messages in thread From: Janosch Frank @ 2021-03-24 14:11 UTC (permalink / raw) To: Andrew Jones; +Cc: kvm, lvivier, thuth, david, pbonzini, cohuck On 3/18/21 4:31 PM, Andrew Jones wrote: > On Thu, Mar 18, 2021 at 12:44:59PM +0000, Janosch Frank wrote: >> The existence of the /dev/kvm character device doesn't imply that the >> kvm module is part of the kernel or that it's always magically loaded >> when needed. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> arm/run | 4 ++-- >> powerpc/run | 4 ++-- >> s390x/run | 4 ++-- >> scripts/arch-run.bash | 7 +++++-- >> x86/run | 4 ++-- >> 5 files changed, 13 insertions(+), 10 deletions(-) >> >> diff --git a/arm/run b/arm/run >> index a390ca5a..ca2d44e0 100755 >> --- a/arm/run >> +++ b/arm/run >> @@ -10,10 +10,10 @@ if [ -z "$STANDALONE" ]; then >> fi >> processor="$PROCESSOR" >> >> -ACCEL=$(get_qemu_accelerator) || >> +qemu=$(search_qemu_binary) || >> exit $? >> >> -qemu=$(search_qemu_binary) || >> +ACCEL=$(get_qemu_accelerator) || >> exit $? > > How about renaming search_qemu_binary() to set_qemu_accelerator(), which > would also ensure QEMU is set (if it doesn't error out on failure) and > then call that from get_qemu_accelerator()? That way we don't need to > worry about this order of calls nor this lowercase 'qemu' variable being > set. Also, we can rename get_qemu_accelerator() to set_qemu_accelerator() > and ensure it sets ACCEL. > > Thanks, > drew > I've already broken off the accel.bash change and fixed the arch/run problem but this here will have two wait until after my easter vacation so don't wait for patches the next ~2 weeks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [kvm-unit-tests RFC 2/2] scripts: Set ACCEL in run_tests.sh if empty 2021-03-18 12:44 [kvm-unit-tests RFC 0/2] scripts: Fix accel handling Janosch Frank 2021-03-18 12:44 ` [kvm-unit-tests RFC 1/2] scripts: Check kvm availability by asking qemu Janosch Frank @ 2021-03-18 12:45 ` Janosch Frank 2021-03-18 15:42 ` Andrew Jones 1 sibling, 1 reply; 8+ messages in thread From: Janosch Frank @ 2021-03-18 12:45 UTC (permalink / raw) To: kvm; +Cc: lvivier, thuth, david, drjones, pbonzini, cohuck The current checks compare the env ACCEL to the unittests.conf provided accel. That's all fine as long as the user always specifies the ACCEL env variable. If that's not the case and KVM is not available or if a test specifies tcg and we start a qemu with the kvm acceleration as it's the default we'll run into problems. So let's fetch the accelerator before calling the arch/run script and check it against the test's specified accel. Yes, we now do that twice, once in the run_tests.sh and one in arch/run, but I don't think there's a good way around it since you can execute arch/run without run_tests.sh. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- run_tests.sh | 6 ++++ s390x/run | 6 +++- scripts/accel.bash | 63 +++++++++++++++++++++++++++++++++++++++++ scripts/arch-run.bash | 66 ++----------------------------------------- scripts/runtime.bash | 2 +- 5 files changed, 77 insertions(+), 66 deletions(-) create mode 100644 scripts/accel.bash diff --git a/run_tests.sh b/run_tests.sh index 65108e73..9ccb97bd 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -10,6 +10,7 @@ if [ ! -f config.mak ]; then fi source config.mak source scripts/common.bash +source scripts/accel.bash function usage() { @@ -164,6 +165,11 @@ if [[ $tap_output == "yes" ]]; then echo "TAP version 13" fi +qemu=$(search_qemu_binary) +if [ -z "$ACCEL" ]; then + ACCEL=$(get_qemu_accelerator) +fi + trap "wait; exit 130" SIGINT ( diff --git a/s390x/run b/s390x/run index 2ec6da70..df7ef5ca 100755 --- a/s390x/run +++ b/s390x/run @@ -12,8 +12,12 @@ fi qemu=$(search_qemu_binary) || exit $? -ACCEL=$(get_qemu_accelerator) || +if [ -z "$DEF_ACCEL "]; then + ACCEL=$(get_qemu_accelerator) || exit $? +else + ACCEL=$DEF_ACCEL +fi M='-machine s390-ccw-virtio' M+=",accel=$ACCEL" diff --git a/scripts/accel.bash b/scripts/accel.bash new file mode 100644 index 00000000..ea12412a --- /dev/null +++ b/scripts/accel.bash @@ -0,0 +1,63 @@ +search_qemu_binary () +{ + local save_path=$PATH + local qemucmd qemu + + export PATH=$PATH:/usr/libexec + for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}; do + if $qemucmd --help 2>/dev/null | grep -q 'QEMU'; then + qemu="$qemucmd" + break + fi + done + + if [ -z "$qemu" ]; then + echo "A QEMU binary was not found." >&2 + echo "You can set a custom location by using the QEMU=<path> environment variable." >&2 + return 2 + fi + command -v $qemu + export PATH=$save_path +} + +kvm_available () +{ + if $($qemu -accel kvm 2> /dev/null); then + return 0; + else + return 1; + fi + + [ "$HOST" = "$ARCH_NAME" ] || + ( [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ) || + ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) +} + +hvf_available () +{ + [ "$(sysctl -n kern.hv_support 2>/dev/null)" = "1" ] || return 1 + [ "$HOST" = "$ARCH_NAME" ] || + ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) +} + +get_qemu_accelerator () +{ + if [ "$ACCEL" = "kvm" ] && ! kvm_available; then + echo "KVM is needed, but not available on this host" >&2 + return 2 + fi + if [ "$ACCEL" = "hvf" ] && ! hvf_available; then + echo "HVF is needed, but not available on this host" >&2 + return 2 + fi + + if [ "$ACCEL" ]; then + echo $ACCEL + elif kvm_available; then + echo kvm + elif hvf_available; then + echo hvf + else + echo tcg + fi +} diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index 8cc9a61e..85c07792 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -24,6 +24,8 @@ # 1..127 - FAILURE (could be QEMU, a run script, or the unittest) # >= 128 - Signal (signum = status - 128) ############################################################################## +source scripts/accel.bash + run_qemu () { local stdout errors ret sig @@ -171,28 +173,6 @@ migration_cmd () fi } -search_qemu_binary () -{ - local save_path=$PATH - local qemucmd qemu - - export PATH=$PATH:/usr/libexec - for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}; do - if $qemucmd --help 2>/dev/null | grep -q 'QEMU'; then - qemu="$qemucmd" - break - fi - done - - if [ -z "$qemu" ]; then - echo "A QEMU binary was not found." >&2 - echo "You can set a custom location by using the QEMU=<path> environment variable." >&2 - return 2 - fi - command -v $qemu - export PATH=$save_path -} - initrd_create () { if [ "$ENVIRON_DEFAULT" = "yes" ]; then @@ -339,45 +319,3 @@ trap_exit_push () local old_exit=$(trap -p EXIT | sed "s/^[^']*'//;s/'[^']*$//") trap -- "$1; $old_exit" EXIT } - -kvm_available () -{ - if $($qemu -accel kvm 2> /dev/null); then - return 0; - else - return 1; - fi - - [ "$HOST" = "$ARCH_NAME" ] || - ( [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ) || - ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) -} - -hvf_available () -{ - [ "$(sysctl -n kern.hv_support 2>/dev/null)" = "1" ] || return 1 - [ "$HOST" = "$ARCH_NAME" ] || - ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) -} - -get_qemu_accelerator () -{ - if [ "$ACCEL" = "kvm" ] && ! kvm_available; then - echo "KVM is needed, but not available on this host" >&2 - return 2 - fi - if [ "$ACCEL" = "hvf" ] && ! hvf_available; then - echo "HVF is needed, but not available on this host" >&2 - return 2 - fi - - if [ "$ACCEL" ]; then - echo $ACCEL - elif kvm_available; then - echo kvm - elif hvf_available; then - echo hvf - else - echo tcg - fi -} diff --git a/scripts/runtime.bash b/scripts/runtime.bash index 132389c7..5d444db4 100644 --- a/scripts/runtime.bash +++ b/scripts/runtime.bash @@ -30,7 +30,7 @@ premature_failure() get_cmdline() { local kernel=$1 - echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts" + echo "TESTNAME=$testname TIMEOUT=$timeout DEF_ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts" } skip_nodefault() -- 2.27.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests RFC 2/2] scripts: Set ACCEL in run_tests.sh if empty 2021-03-18 12:45 ` [kvm-unit-tests RFC 2/2] scripts: Set ACCEL in run_tests.sh if empty Janosch Frank @ 2021-03-18 15:42 ` Andrew Jones 0 siblings, 0 replies; 8+ messages in thread From: Andrew Jones @ 2021-03-18 15:42 UTC (permalink / raw) To: Janosch Frank; +Cc: kvm, lvivier, thuth, david, pbonzini, cohuck On Thu, Mar 18, 2021 at 12:45:00PM +0000, Janosch Frank wrote: > The current checks compare the env ACCEL to the unittests.conf > provided accel. That's all fine as long as the user always specifies > the ACCEL env variable. > > If that's not the case and KVM is not available or if a test specifies > tcg and we start a qemu with the kvm acceleration as it's the default > we'll run into problems. > > So let's fetch the accelerator before calling the arch/run script and > check it against the test's specified accel. Yes, we now do that > twice, once in the run_tests.sh and one in arch/run, but I don't think > there's a good way around it since you can execute arch/run > without run_tests.sh. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > run_tests.sh | 6 ++++ > s390x/run | 6 +++- > scripts/accel.bash | 63 +++++++++++++++++++++++++++++++++++++++++ > scripts/arch-run.bash | 66 ++----------------------------------------- > scripts/runtime.bash | 2 +- > 5 files changed, 77 insertions(+), 66 deletions(-) > create mode 100644 scripts/accel.bash > > diff --git a/run_tests.sh b/run_tests.sh > index 65108e73..9ccb97bd 100755 > --- a/run_tests.sh > +++ b/run_tests.sh > @@ -10,6 +10,7 @@ if [ ! -f config.mak ]; then > fi > source config.mak > source scripts/common.bash > +source scripts/accel.bash > > function usage() > { > @@ -164,6 +165,11 @@ if [[ $tap_output == "yes" ]]; then > echo "TAP version 13" > fi > > +qemu=$(search_qemu_binary) > +if [ -z "$ACCEL" ]; then > + ACCEL=$(get_qemu_accelerator) > +fi > + > trap "wait; exit 130" SIGINT > > ( > diff --git a/s390x/run b/s390x/run > index 2ec6da70..df7ef5ca 100755 > --- a/s390x/run > +++ b/s390x/run > @@ -12,8 +12,12 @@ fi > qemu=$(search_qemu_binary) || > exit $? > > -ACCEL=$(get_qemu_accelerator) || > +if [ -z "$DEF_ACCEL "]; then > + ACCEL=$(get_qemu_accelerator) || > exit $? > +else > + ACCEL=$DEF_ACCEL > +fi > > M='-machine s390-ccw-virtio' > M+=",accel=$ACCEL" > diff --git a/scripts/accel.bash b/scripts/accel.bash > new file mode 100644 > index 00000000..ea12412a > --- /dev/null > +++ b/scripts/accel.bash > @@ -0,0 +1,63 @@ > +search_qemu_binary () > +{ > + local save_path=$PATH > + local qemucmd qemu > + > + export PATH=$PATH:/usr/libexec > + for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}; do > + if $qemucmd --help 2>/dev/null | grep -q 'QEMU'; then > + qemu="$qemucmd" > + break > + fi > + done > + > + if [ -z "$qemu" ]; then > + echo "A QEMU binary was not found." >&2 > + echo "You can set a custom location by using the QEMU=<path> environment variable." >&2 > + return 2 > + fi > + command -v $qemu > + export PATH=$save_path > +} > + > +kvm_available () > +{ > + if $($qemu -accel kvm 2> /dev/null); then > + return 0; > + else > + return 1; > + fi > + > + [ "$HOST" = "$ARCH_NAME" ] || > + ( [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ) || > + ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) > +} > + > +hvf_available () > +{ > + [ "$(sysctl -n kern.hv_support 2>/dev/null)" = "1" ] || return 1 > + [ "$HOST" = "$ARCH_NAME" ] || > + ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) > +} > + > +get_qemu_accelerator () > +{ > + if [ "$ACCEL" = "kvm" ] && ! kvm_available; then > + echo "KVM is needed, but not available on this host" >&2 > + return 2 > + fi > + if [ "$ACCEL" = "hvf" ] && ! hvf_available; then > + echo "HVF is needed, but not available on this host" >&2 > + return 2 > + fi > + > + if [ "$ACCEL" ]; then > + echo $ACCEL > + elif kvm_available; then > + echo kvm > + elif hvf_available; then > + echo hvf > + else > + echo tcg > + fi > +} > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash > index 8cc9a61e..85c07792 100644 > --- a/scripts/arch-run.bash > +++ b/scripts/arch-run.bash > @@ -24,6 +24,8 @@ > # 1..127 - FAILURE (could be QEMU, a run script, or the unittest) > # >= 128 - Signal (signum = status - 128) > ############################################################################## > +source scripts/accel.bash > + > run_qemu () > { > local stdout errors ret sig > @@ -171,28 +173,6 @@ migration_cmd () > fi > } > > -search_qemu_binary () > -{ > - local save_path=$PATH > - local qemucmd qemu > - > - export PATH=$PATH:/usr/libexec > - for qemucmd in ${QEMU:-qemu-system-$ARCH_NAME qemu-kvm}; do > - if $qemucmd --help 2>/dev/null | grep -q 'QEMU'; then > - qemu="$qemucmd" > - break > - fi > - done > - > - if [ -z "$qemu" ]; then > - echo "A QEMU binary was not found." >&2 > - echo "You can set a custom location by using the QEMU=<path> environment variable." >&2 > - return 2 > - fi > - command -v $qemu > - export PATH=$save_path > -} > - > initrd_create () > { > if [ "$ENVIRON_DEFAULT" = "yes" ]; then > @@ -339,45 +319,3 @@ trap_exit_push () > local old_exit=$(trap -p EXIT | sed "s/^[^']*'//;s/'[^']*$//") > trap -- "$1; $old_exit" EXIT > } > - > -kvm_available () > -{ > - if $($qemu -accel kvm 2> /dev/null); then > - return 0; > - else > - return 1; > - fi > - > - [ "$HOST" = "$ARCH_NAME" ] || > - ( [ "$HOST" = aarch64 ] && [ "$ARCH" = arm ] ) || > - ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) > -} > - > -hvf_available () > -{ > - [ "$(sysctl -n kern.hv_support 2>/dev/null)" = "1" ] || return 1 > - [ "$HOST" = "$ARCH_NAME" ] || > - ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] ) > -} > - > -get_qemu_accelerator () > -{ > - if [ "$ACCEL" = "kvm" ] && ! kvm_available; then > - echo "KVM is needed, but not available on this host" >&2 > - return 2 > - fi > - if [ "$ACCEL" = "hvf" ] && ! hvf_available; then > - echo "HVF is needed, but not available on this host" >&2 > - return 2 > - fi > - > - if [ "$ACCEL" ]; then > - echo $ACCEL > - elif kvm_available; then > - echo kvm > - elif hvf_available; then > - echo hvf > - else > - echo tcg > - fi > -} > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > index 132389c7..5d444db4 100644 > --- a/scripts/runtime.bash > +++ b/scripts/runtime.bash > @@ -30,7 +30,7 @@ premature_failure() > get_cmdline() > { > local kernel=$1 > - echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts" > + echo "TESTNAME=$testname TIMEOUT=$timeout DEF_ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts" I think this name change can break any $TEST_DIR/run files that aren't updated to check DEF_ACCEL. This patch only updates the runner for s390. Also, it'd be better to do the code movement (scripts/accel.bash) of this patch separately with a patch that does not have any functional change. Thanks, drew ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-03-24 14:12 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-18 12:44 [kvm-unit-tests RFC 0/2] scripts: Fix accel handling Janosch Frank 2021-03-18 12:44 ` [kvm-unit-tests RFC 1/2] scripts: Check kvm availability by asking qemu Janosch Frank 2021-03-18 14:18 ` Andrew Jones 2021-03-18 15:31 ` Andrew Jones 2021-03-18 15:39 ` Janosch Frank 2021-03-24 14:11 ` Janosch Frank 2021-03-18 12:45 ` [kvm-unit-tests RFC 2/2] scripts: Set ACCEL in run_tests.sh if empty Janosch Frank 2021-03-18 15:42 ` Andrew Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).