From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yann E. MORIN <yann.morin.1998@free.fr>
Date: Mon, 13 Apr 2020 09:15:32 +0200
Subject: [Buildroot] [PATCH v5 4/5] support/scripts/boot-qemu-image.py:
 boot Qemu images with Qemu-system.
In-Reply-To: <20200217205030.8157-5-romain.naour@smile.fr>
References: <20200217205030.8157-1-romain.naour@smile.fr>
 <20200217205030.8157-5-romain.naour@smile.fr>
Message-ID: <20200413071532.GV29898@scaer>
List-Id: <buildroot.busybox.net>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: buildroot@busybox.net

Romain, Jugurtha, All,

On 2020-02-17 21:50 +0100, Romain Naour spake thusly:
> From: Jugurtha BELKALEM <jugurtha.belkalem@smile.fr>
> This script is intended to be used by gitlab CI to test
> at runtime Qemu images generated by Buildroot's Qemu defconfigs.
[--SNIP--]
> diff --git a/support/scripts/boot-qemu-image.py b/support/scripts/boot-qemu-image.py
> new file mode 100755
> index 0000000000..abaf88d446
> --- /dev/null
> +++ b/support/scripts/boot-qemu-image.py
> @@ -0,0 +1,105 @@
> +#!/usr/bin/env python3
> +
> +# This script expect to run from the Buildroot top directory.
> +
> +import pexpect
> +import sys
> +import os
> +import re
> +import shlex
> +import shutil
> +import subprocess
> +
> +argc = len(sys.argv)
> +if not (argc == 2):
> +    print("Error: incorrect number of arguments")
> +    print("""Usage: boot-qemu-image.py <qemu_arch_defconfig>""")
> +    sys.exit(1)
> +
> +defconfig_filename = sys.argv[1]
> +
> +# Ignore non Qemu defconfig
> +if defconfig_filename.startswith('qemu_') is False:
> +    sys.exit(0)
> +
> +qemu_start_script_filepath = os.path.join(os.getcwd(), 'output/images/start-qemu.sh')
> +
> +# Ignore if start-qemu.sh is missing, we can't test.
> +if not os.path.exists(qemu_start_script_filepath):
> +    print("Error: " + qemu_start_script_filepath) + " file is missing."
> +    sys.exit(1)
> +
> +qemu_cmd = ""
> +
> +with open(qemu_start_script_filepath, 'r') as script_file:
> +    for line in script_file:
> +        if re.search("qemu-system", line):
> +            qemu_cmd = line
> +            break
> +
> +if not qemu_cmd:
> +    print("Error: No QEMU command line found in " + qemu_start_script_filepath)
> +    sys.exit(1)

Why do you extract the command line, and can't directly run the script?

> +# Replace bashism
> +qemu_cmd = line.replace("${IMAGE_DIR}", "output/images")

This would avoid this dance...

> +# pexpect needs a list, convert a sting to a list and escape quoted substring.
> +qemu_cmd = shlex.split(qemu_cmd)

... as well as this one.

> +# Use host Qemu if provided by Buildroot.
> +os.environ["PATH"] = os.getcwd() + "/output/host/bin" + os.pathsep + os.environ["PATH"]

And this one can be avoided too if the script is called with PATH
properly set to include ${HOST_DIR}/bin. This should be easy, as this
script is only expected to run from the gitlab-ci pipelines.

> +# Ignore when no Qemu emulation is available
> +if not shutil.which(qemu_cmd[0]):
> +    print("No " + qemu_cmd[0] + " binary available, THIS DEFCONFIG CAN NOT BE TESTED!")
> +    sys.exit(0)
> +
> +
> +def main():
> +    global child

Mixing code at the top-level and in a main() is ugly. Mocve everything
to a main (and helper functions if youlike), but don't mix the two.

> +    try:
> +        child.expect(["buildroot login:", pexpect.TIMEOUT], timeout=60)
> +    except pexpect.EOF:
> +        print("Connection problem, exiting.")
> +        sys.exit(1)
> +    except pexpect.TIMEOUT:
> +        print("System did not boot in time, exiting.")
> +        sys.exit(1)
> +
> +    child.sendline("root\r")
> +
> +    try:
> +        child.expect(["# ", pexpect.TIMEOUT], timeout=60)
> +    except pexpect.EOF:
> +        print("Cannot connect to shell")
> +        sys.exit(1)
> +    except pexpect.TIMEOUT:
> +        print("Timeout while waiting for shell")
> +        sys.exit(1)
> +
> +    child.sendline("poweroff\r")
> +
> +    try:
> +        child.expect(["System halted", pexpect.TIMEOUT], timeout=60)
> +        child.expect(pexpect.EOF)
> +    except pexpect.EOF:
> +        sys.exit(0)
> +    except pexpect.TIMEOUT:
> +        print("Cannot halt machine")
> +        # Qemu may not exit properly after "System halted", ignore.
> +        sys.exit(0)
> +
> +
> +# Log the Qemu version
> +subprocess.call([qemu_cmd[0], "--version"])
> +
> +# Log the Qemu command line
> +print(qemu_cmd)

You would not have to do that if you just called the start-qemu.sh script.

Regards,
Yann E. MORIN.

> +child = pexpect.spawn(qemu_cmd[0], qemu_cmd[1:], timeout=5, encoding='utf-8',
> +                      env={"QEMU_AUDIO_DRV": "none", 'PATH': os.environ["PATH"]})
> +# We want only stdout into the log to avoid double echo
> +child.logfile = sys.stdout
> +main()
> -- 
> 2.24.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'