Re: [kvm-unit-tests PATCH v1 1/3] runtime: add support for panic tests

Thomas Huth
2022-06-30T17:49:55Z

On 30/06/2022 13.30, Nico Boehr wrote:
> QEMU suports a guest state "guest-panicked" which indicates something in

s/suports/supports/

> the guest went wrong, for example on s390x, when an external interrupt
> loop was triggered.
>
> Since the guest does not continue to run when it is in the
> guest-panicked state, it is currently impossible to write panicking
> tests in kvm-unit-tests. Support from the runtime is needed to check
> that the guest enters the guest-panicked state.
>
> Similar to migration tests, add a new group panic. Tests in this
> group must enter the guest-panicked state to succeed.
>
> The runtime will spawn a QEMU instance, connect to the QMP and listen
> for events. To parse the QMP protocol, jq[1] is used. Same as with
> netcat in the migration tests, panic tests won't run if jq is not
> installed.
>
> The guest is created in the stopped state and only continued when
> connection to the QMP was successful. This ensures no events are missed
> between QEMU start and the connect to the QMP.
>
> [1] https://stedolan.github.io/jq/
>
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
> s390x/run | 2 +-
> scripts/arch-run.bash | 47 +++++++++++++++++++++++++++++++++++++++++++
> scripts/runtime.bash | 3 +++
> 3 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/s390x/run b/s390x/run
> index 24138f6803be..f1111dbdbe62 100755
> --- a/s390x/run
> +++ b/s390x/run
> @@ -30,7 +30,7 @@ M+=",accel=$ACCEL"
> command="$qemu -nodefaults -nographic $M"
> command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"
> command+=" -kernel"
> -command="$(migration_cmd) $(timeout_cmd) $command"
> +command="$(panic_cmd) $(migration_cmd) $(timeout_cmd) $command"
>
> # We return the exit code via stdout, not via the QEMU return code
> run_qemu_status $command "$@"
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 0dfaf017db0a..5663a1ddb09e 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -104,6 +104,12 @@ qmp ()
> echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | ncat -U $1
> }
>
> +qmp_events ()
> +{
> + while ! test -S "$1"; do sleep 0.1; done
> + echo '{ "execute": "qmp_capabilities" }{ "execute": "cont" }' | ncat --no-shutdown -U $1 | jq -c 'select(has("event"))'

Break the long line into two or three?

> +}
> +
> run_migration ()
> {
> if ! command -v ncat >/dev/null 2>&1; then
> @@ -164,6 +170,40 @@ run_migration ()
> return $ret
> }
>
> +run_panic ()
> +{
> + if ! command -v ncat >/dev/null 2>&1; then
> + echo "${FUNCNAME[0]} needs ncat (netcat)" >&2
> + return 77
> + fi
> +
> + if ! command -v jq >/dev/null 2>&1; then
> + echo "${FUNCNAME[0]} needs jq" >&2
> + return 77
> + fi
> +
> + qmp=$(mktemp -u -t panic-qmp.XXXXXXXXXX)
> +
> + trap '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 \
> + -mon chardev=mon1,mode=control -S &
> +
> + panic_event_count=$(qmp_events ${qmp} | jq -c 'select(.event == "GUEST_PANICKED")' | wc -l)
> + if [ $panic_event_count -lt 1 ]; then

Maybe put double-quotes around $panic_event_count , just to be sure?

With the nits fixed:

Reviewed-by: Thomas Huth <thuth@redhat.com>