Use expect to invoke QEMU so that we can terminate the test as soon as
we get the right string in the output instead of waiting until the
final timeout.

For timeout, instead of an hardcoding the value, use a Gitlab CI
variable "QEMU_TIMEOUT" that can be changed depending on the latest
status of the Gitlab CI runners.

Note that for simplicity in the case of dom0less test, the script only
checks for the $passed string. That is because the dom0 prompt and the
$passed string could happen in any order making it difficult to check
with expect which checks for strings in a specific order.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 automation/scripts/qemu-alpine-x86_64.sh      | 15 +++----
 automation/scripts/qemu-key.ex                | 42 +++++++++++++++++++
 automation/scripts/qemu-smoke-dom0-arm32.sh   | 15 ++++---
 automation/scripts/qemu-smoke-dom0-arm64.sh   | 15 ++++---
 .../scripts/qemu-smoke-dom0less-arm32.sh      | 14 +++----
 .../scripts/qemu-smoke-dom0less-arm64.sh      | 14 +++----
 automation/scripts/qemu-smoke-ppc64le.sh      | 12 +++---
 automation/scripts/qemu-smoke-riscv64.sh      | 12 +++---
 automation/scripts/qemu-smoke-x86-64.sh       | 14 +++----
 automation/scripts/qemu-xtf-dom0less-arm64.sh | 14 +++----
 10 files changed, 97 insertions(+), 70 deletions(-)
 create mode 100755 automation/scripts/qemu-key.ex

diff --git a/automation/scripts/qemu-alpine-x86_64.sh b/automation/scripts/qemu-alpine-x86_64.sh
index 8e398dcea3..24b23a573c 100755
--- a/automation/scripts/qemu-alpine-x86_64.sh
+++ b/automation/scripts/qemu-alpine-x86_64.sh
@@ -77,18 +77,15 @@ EOF
 # Run the test
 rm -f smoke.serial
 set +e
-timeout -k 1 720 \
-qemu-system-x86_64 \
+export qemu_cmd="qemu-system-x86_64 \
     -cpu qemu64,+svm \
     -m 2G -smp 2 \
     -monitor none -serial stdio \
     -nographic \
     -device virtio-net-pci,netdev=n0 \
-    -netdev user,id=n0,tftp=binaries,bootfile=/pxelinux.0 |& \
-        # Remove carriage returns from the stdout output, as gitlab
-        # interface chokes on them
-        tee smoke.serial | sed 's/\r//'
+    -netdev user,id=n0,tftp=binaries,bootfile=/pxelinux.0"
 
-set -e
-(grep -q "Domain-0" smoke.serial && grep -q "BusyBox" smoke.serial) || exit 1
-exit 0
+export qemu_log="smoke.serial"
+export log_msg="Domain-0"
+export passed="BusyBox"
+./automation/scripts/qemu-key.ex
diff --git a/automation/scripts/qemu-key.ex b/automation/scripts/qemu-key.ex
new file mode 100755
index 0000000000..569ef2781f
--- /dev/null
+++ b/automation/scripts/qemu-key.ex
@@ -0,0 +1,42 @@
+#!/usr/bin/expect -f
+
+set timeout $env(QEMU_TIMEOUT)
+
+log_file -a $env(qemu_log)
+
+match_max 10000
+
+eval spawn $env(qemu_cmd)
+
+expect_after {
+    -re "(.*)\r" {
+        exp_continue
+    }
+    timeout {send_user "ERROR-Timeout!\n"; exit 1}
+    eof {send_user "ERROR-EOF!\n"; exit 1}
+}
+
+if {[info exists env(uboot_cmd)]} {
+    expect "=>"
+
+    send "$env(uboot_cmd)\r"
+}
+
+if {[info exists env(log_msg)]} {
+    expect "$env(log_msg)"
+}
+
+if {[info exists env(xen_cmd)]} {
+    send "$env(xen_cmd)\r"
+}
+
+if {[info exists env(passed)]} {
+    expect {
+        "$env(passed)" {
+            exit 0
+        }
+    }
+}
+
+expect eof
+
diff --git a/automation/scripts/qemu-smoke-dom0-arm32.sh b/automation/scripts/qemu-smoke-dom0-arm32.sh
index eaaea5a982..2835d8a908 100755
--- a/automation/scripts/qemu-smoke-dom0-arm32.sh
+++ b/automation/scripts/qemu-smoke-dom0-arm32.sh
@@ -78,9 +78,7 @@ bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config
 
 rm -f ${serial_log}
 set +e
-echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"| \
-timeout -k 1 720 \
-./qemu-system-arm \
+export qemu_cmd="./qemu-system-arm \
    -machine virt \
    -machine virtualization=true \
    -smp 4 \
@@ -91,9 +89,10 @@ timeout -k 1 720 \
    -no-reboot \
    -device virtio-net-pci,netdev=n0 \
    -netdev user,id=n0,tftp=./ \
-   -bios /usr/lib/u-boot/qemu_arm/u-boot.bin |& \
-      tee ${serial_log} | sed 's/\r//'
+   -bios /usr/lib/u-boot/qemu_arm/u-boot.bin"
 
-set -e
-(grep -q "Domain-0" ${serial_log} && grep -q "^/ #" ${serial_log}) || exit 1
-exit 0
+export uboot_cmd="virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"
+export qemu_log="${serial_log}"
+export log_msg="Domain-0"
+export passed="/ #"
+../automation/scripts/qemu-key.ex
diff --git a/automation/scripts/qemu-smoke-dom0-arm64.sh b/automation/scripts/qemu-smoke-dom0-arm64.sh
index 352963a741..c2eac941d2 100755
--- a/automation/scripts/qemu-smoke-dom0-arm64.sh
+++ b/automation/scripts/qemu-smoke-dom0-arm64.sh
@@ -94,9 +94,7 @@ bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c binaries/conf
 # Run the test
 rm -f smoke.serial
 set +e
-echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"| \
-timeout -k 1 720 \
-./binaries/qemu-system-aarch64 \
+export qemu_cmd="./binaries/qemu-system-aarch64 \
     -machine virtualization=true \
     -cpu cortex-a57 -machine type=virt \
     -m 2048 -monitor none -serial stdio \
@@ -104,9 +102,10 @@ timeout -k 1 720 \
     -no-reboot \
     -device virtio-net-pci,netdev=n0 \
     -netdev user,id=n0,tftp=binaries \
-    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin |& \
-        tee smoke.serial | sed 's/\r//'
+    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin"
 
-set -e
-(grep -q "Domain-0" smoke.serial && grep -q "BusyBox" smoke.serial) || exit 1
-exit 0
+export uboot_cmd="virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"
+export qemu_log="smoke.serial"
+export log_msg="Domain-0"
+export passed="BusyBox"
+./automation/scripts/qemu-key.ex
diff --git a/automation/scripts/qemu-smoke-dom0less-arm32.sh b/automation/scripts/qemu-smoke-dom0less-arm32.sh
index c027c8c5c8..9fe08c337a 100755
--- a/automation/scripts/qemu-smoke-dom0less-arm32.sh
+++ b/automation/scripts/qemu-smoke-dom0less-arm32.sh
@@ -131,9 +131,7 @@ bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config
 # Run the test
 rm -f ${serial_log}
 set +e
-echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"| \
-timeout -k 1 240 \
-./qemu-system-arm \
+export qemu_cmd="./qemu-system-arm \
     -machine virt \
     -machine virtualization=true \
     -smp 4 \
@@ -144,9 +142,9 @@ timeout -k 1 240 \
     -no-reboot \
     -device virtio-net-pci,netdev=n0 \
     -netdev user,id=n0,tftp=./ \
-    -bios /usr/lib/u-boot/qemu_arm/u-boot.bin |& \
-        tee ${serial_log} | sed 's/\r//'
+    -bios /usr/lib/u-boot/qemu_arm/u-boot.bin"
 
-set -e
-(grep -q "${dom0_prompt}" ${serial_log} && grep -q "${passed}" ${serial_log}) || exit 1
-exit 0
+export uboot_cmd="virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"
+export qemu_log="${serial_log}"
+export passed="${passed}"
+../automation/scripts/qemu-key.ex
diff --git a/automation/scripts/qemu-smoke-dom0less-arm64.sh b/automation/scripts/qemu-smoke-dom0less-arm64.sh
index 15258692d5..fd11482f25 100755
--- a/automation/scripts/qemu-smoke-dom0less-arm64.sh
+++ b/automation/scripts/qemu-smoke-dom0less-arm64.sh
@@ -205,9 +205,7 @@ bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c binaries/conf
 # Run the test
 rm -f smoke.serial
 set +e
-echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"| \
-timeout -k 1 240 \
-./binaries/qemu-system-aarch64 \
+export qemu_cmd="./binaries/qemu-system-aarch64 \
     -machine virtualization=true \
     -cpu cortex-a57 -machine type=virt,gic-version=$gic_version \
     -m 2048 -monitor none -serial stdio \
@@ -215,9 +213,9 @@ timeout -k 1 240 \
     -no-reboot \
     -device virtio-net-pci,netdev=n0 \
     -netdev user,id=n0,tftp=binaries \
-    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin |& \
-        tee smoke.serial | sed 's/\r//'
+    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin"
 
-set -e
-(grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1
-exit 0
+export uboot_cmd="virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"
+export qemu_log="smoke.serial"
+export passed="${passed}"
+./automation/scripts/qemu-key.ex
diff --git a/automation/scripts/qemu-smoke-ppc64le.sh b/automation/scripts/qemu-smoke-ppc64le.sh
index 9088881b73..72ee9765e9 100755
--- a/automation/scripts/qemu-smoke-ppc64le.sh
+++ b/automation/scripts/qemu-smoke-ppc64le.sh
@@ -11,8 +11,7 @@ machine=$1
 rm -f ${serial_log}
 set +e
 
-timeout -k 1 20 \
-qemu-system-ppc64 \
+export qemu_cmd="qemu-system-ppc64 \
     -bios skiboot.lid \
     -M $machine \
     -m 2g \
@@ -21,9 +20,8 @@ qemu-system-ppc64 \
     -monitor none \
     -nographic \
     -serial stdio \
-    -kernel binaries/xen \
-    |& tee ${serial_log} | sed 's/\r//'
+    -kernel binaries/xen"
 
-set -e
-(grep -q "Hello, ppc64le!" ${serial_log}) || exit 1
-exit 0
+export qemu_log="${serial_log}"
+export passed="Hello, ppc64le!"
+./automation/scripts/qemu-key.ex
diff --git a/automation/scripts/qemu-smoke-riscv64.sh b/automation/scripts/qemu-smoke-riscv64.sh
index f90df3c051..64630ba7d7 100755
--- a/automation/scripts/qemu-smoke-riscv64.sh
+++ b/automation/scripts/qemu-smoke-riscv64.sh
@@ -6,15 +6,13 @@ set -ex
 rm -f smoke.serial
 set +e
 
-timeout -k 1 2 \
-qemu-system-riscv64 \
+export qemu_cmd="qemu-system-riscv64 \
     -M virt \
     -smp 1 \
     -nographic \
     -m 2g \
-    -kernel binaries/xen \
-    |& tee smoke.serial | sed 's/\r//'
+    -kernel binaries/xen"
 
-set -e
-(grep -q "All set up" smoke.serial) || exit 1
-exit 0
+export qemu_log="smoke.serial"
+export passed="All set up"
+./automation/scripts/qemu-key.ex
diff --git a/automation/scripts/qemu-smoke-x86-64.sh b/automation/scripts/qemu-smoke-x86-64.sh
index 3014d07314..269396f805 100755
--- a/automation/scripts/qemu-smoke-x86-64.sh
+++ b/automation/scripts/qemu-smoke-x86-64.sh
@@ -16,11 +16,11 @@ esac
 
 rm -f smoke.serial
 set +e
-timeout -k 1 30 \
-qemu-system-x86_64 -nographic -kernel binaries/xen \
+export qemu_cmd="qemu-system-x86_64 -nographic -kernel binaries/xen \
         -initrd xtf/tests/example/$k \
-        -append "loglvl=all console=com1 noreboot console_timestamps=boot $extra" \
-        -m 512 -monitor none -serial file:smoke.serial
-set -e
-grep -q 'Test result: SUCCESS' smoke.serial || exit 1
-exit 0
+        -append \"loglvl=all console=com1 noreboot console_timestamps=boot $extra\" \
+        -m 512 -monitor none -serial stdio"
+
+export qemu_log="smoke.serial"
+export passed="Test result: SUCCESS"
+./automation/scripts/qemu-key.ex
diff --git a/automation/scripts/qemu-xtf-dom0less-arm64.sh b/automation/scripts/qemu-xtf-dom0less-arm64.sh
index b08c2d44fb..042f4ed2d9 100755
--- a/automation/scripts/qemu-xtf-dom0less-arm64.sh
+++ b/automation/scripts/qemu-xtf-dom0less-arm64.sh
@@ -51,9 +51,7 @@ bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c binaries/conf
 # Run the test
 rm -f smoke.serial
 set +e
-echo "  virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"| \
-timeout -k 1 120 \
-./binaries/qemu-system-aarch64 \
+export qemu_cmd="./binaries/qemu-system-aarch64 \
     -machine virtualization=true \
     -cpu cortex-a57 -machine type=virt \
     -m 2048 -monitor none -serial stdio \
@@ -61,9 +59,9 @@ timeout -k 1 120 \
     -no-reboot \
     -device virtio-net-pci,netdev=n0 \
     -netdev user,id=n0,tftp=binaries \
-    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin |& \
-        tee smoke.serial | sed 's/\r//'
+    -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin"
 
-set -e
-(grep -q "${passed}" smoke.serial) || exit 1
-exit 0
+export uboot_cmd="virtio scan; dhcp; tftpb 0x40000000 boot.scr; source 0x40000000"
+export qemu_log="smoke.serial"
+export passed="${passed}"
+./automation/scripts/qemu-key.ex
-- 
2.25.1

Hi Stefano,

On 10/08/2024 08:59, Stefano Stabellini wrote:
> Use expect to invoke QEMU so that we can terminate the test as soon as
> we get the right string in the output instead of waiting until the
> final timeout.
This is a great improvement.

> 
> For timeout, instead of an hardcoding the value, use a Gitlab CI
> variable "QEMU_TIMEOUT" that can be changed depending on the latest
> status of the Gitlab CI runners.
What is the current value of QEMU_TIMEOUT? I don't see it being mentioned anywhere.
Could tests define this env var if needed? What would be the precedence then?
I'm thinking that some tests don't need a very big timeout that otherwise is necessary
for longer tests (e.g. xtf test does not need to wait 720s). 

> 
> Note that for simplicity in the case of dom0less test, the script only
> checks for the $passed string. That is because the dom0 prompt and the
> $passed string could happen in any order making it difficult to check
> with expect which checks for strings in a specific order.
I use expect in my local testing too and I search for both strings. More below.

> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
>  automation/scripts/qemu-alpine-x86_64.sh      | 15 +++----
>  automation/scripts/qemu-key.ex                | 42 +++++++++++++++++++
>  automation/scripts/qemu-smoke-dom0-arm32.sh   | 15 ++++---
>  automation/scripts/qemu-smoke-dom0-arm64.sh   | 15 ++++---
>  .../scripts/qemu-smoke-dom0less-arm32.sh      | 14 +++----
>  .../scripts/qemu-smoke-dom0less-arm64.sh      | 14 +++----
>  automation/scripts/qemu-smoke-ppc64le.sh      | 12 +++---
>  automation/scripts/qemu-smoke-riscv64.sh      | 12 +++---
>  automation/scripts/qemu-smoke-x86-64.sh       | 14 +++----
>  automation/scripts/qemu-xtf-dom0less-arm64.sh | 14 +++----
>  10 files changed, 97 insertions(+), 70 deletions(-)
>  create mode 100755 automation/scripts/qemu-key.ex
> 
> diff --git a/automation/scripts/qemu-alpine-x86_64.sh b/automation/scripts/qemu-alpine-x86_64.sh
> index 8e398dcea3..24b23a573c 100755
> --- a/automation/scripts/qemu-alpine-x86_64.sh
> +++ b/automation/scripts/qemu-alpine-x86_64.sh
> @@ -77,18 +77,15 @@ EOF
>  # Run the test
>  rm -f smoke.serial
>  set +e
> -timeout -k 1 720 \
> -qemu-system-x86_64 \
> +export qemu_cmd="qemu-system-x86_64 \
Usually (even respected by analyze.sh) exported env variables are written in capital letters

>      -cpu qemu64,+svm \
>      -m 2G -smp 2 \
>      -monitor none -serial stdio \
>      -nographic \
>      -device virtio-net-pci,netdev=n0 \
> -    -netdev user,id=n0,tftp=binaries,bootfile=/pxelinux.0 |& \
> -        # Remove carriage returns from the stdout output, as gitlab
> -        # interface chokes on them
> -        tee smoke.serial | sed 's/\r//'
> +    -netdev user,id=n0,tftp=binaries,bootfile=/pxelinux.0"
>  
> -set -e
> -(grep -q "Domain-0" smoke.serial && grep -q "BusyBox" smoke.serial) || exit 1
> -exit 0
> +export qemu_log="smoke.serial"
> +export log_msg="Domain-0"
> +export passed="BusyBox"
NIT: one empty line here for readability

> +./automation/scripts/qemu-key.ex
> diff --git a/automation/scripts/qemu-key.ex b/automation/scripts/qemu-key.ex
NIT: usually expect script have '.exp' extension.

> new file mode 100755
> index 0000000000..569ef2781f
> --- /dev/null
> +++ b/automation/scripts/qemu-key.ex
> @@ -0,0 +1,42 @@
> +#!/usr/bin/expect -f
> +
> +set timeout $env(QEMU_TIMEOUT)
> +
> +log_file -a $env(qemu_log)
> +
> +match_max 10000
> +
> +eval spawn $env(qemu_cmd)
> +
> +expect_after {
> +    -re "(.*)\r" {
> +        exp_continue
> +    }
> +    timeout {send_user "ERROR-Timeout!\n"; exit 1}
> +    eof {send_user "ERROR-EOF!\n"; exit 1}
> +}
> +
> +if {[info exists env(uboot_cmd)]} {
> +    expect "=>"
> +
> +    send "$env(uboot_cmd)\r"
> +}
> +
> +if {[info exists env(log_msg)]} {
> +    expect "$env(log_msg)"
> +}
> +
> +if {[info exists env(xen_cmd)]} {
> +    send "$env(xen_cmd)\r"
> +}
Does not seem to be used at all. For now, I'd suggest to drop it.

> +
> +if {[info exists env(passed)]} {
> +    expect {
> +        "$env(passed)" {
> +            exit 0
> +        }
> +    }
> +}
As said above, in my local testing, I search for both strings (after all we should test that dom0 works too) as follows:

if {[info exists env(dom0_passed)] && [info exists env(domu_passed)]} {
    expect {
        "$env(dom0_passed)" {
            expect "$env(domu_passed)"
            exit 0
        }
        "$env(domu_passed)" {
            expect "$env(dom0_passed)"
            exit 0
        }
    }
}

Apart from that:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal