diff mbox series

[2/5] arm/lib: Factor out asyncio in FVPRunner

Message ID 20221115150116.314729-2-peter.hoyes@arm.com
State New
Headers show
Series [1/5] arm/fvp: Join cli arguments in verbose logging | expand

Commit Message

Peter Hoyes Nov. 15, 2022, 3:01 p.m. UTC
From: Peter Hoyes <Peter.Hoyes@arm.com>

FVPRunner relies heavily on asyncio, despite there being very little
concurrent work happening. Additionally, while the runfvp entry point
starts an asyncio runner, it is not practical to have a single asyncio
runtime during testimage, which is fully synchronous.

Refactor to use subprocess.Popen and related functionality. The process
object has a similar interface to its async equivalent.

Cascade the API changes to runfvp and the test target classes.

Issue-Id: SCM-5314
Signed-off-by: Peter Hoyes <Peter.Hoyes@arm.com>
Change-Id: I3e7517e8bcbb3b93c41405d43dbd8bd24a9e7eb8
---
 meta-arm/lib/fvp/runner.py                 | 35 +++++++++---------
 meta-arm/lib/oeqa/controllers/fvp.py       | 41 +++++++---------------
 meta-arm/lib/oeqa/selftest/cases/runfvp.py | 16 ++++-----
 scripts/runfvp                             | 23 +++++-------
 4 files changed, 46 insertions(+), 69 deletions(-)
diff mbox series

Patch

diff --git a/meta-arm/lib/fvp/runner.py b/meta-arm/lib/fvp/runner.py
index f5c61f8b..9b537e27 100644
--- a/meta-arm/lib/fvp/runner.py
+++ b/meta-arm/lib/fvp/runner.py
@@ -1,4 +1,3 @@ 
-import asyncio
 import re
 import subprocess
 import os
@@ -57,7 +56,7 @@  class FVPRunner:
     def add_line_callback(self, callback):
         self._line_callbacks.append(callback)
 
-    async def start(self, config, extra_args=[], terminal_choice="none"):
+    def start(self, config, extra_args=[], terminal_choice="none"):
         cli = cli_from_config(config, terminal_choice)
         cli += extra_args
 
@@ -69,8 +68,8 @@  class FVPRunner:
                 env[name] = os.environ[name]
 
         self._logger.debug(f"Constructed FVP call: {shlex.join(cli)}")
-        self._fvp_process = await asyncio.create_subprocess_exec(
-            *cli,
+        self._fvp_process = subprocess.Popen(
+            cli,
             stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
             env=env)
 
@@ -82,13 +81,13 @@  class FVPRunner:
                 self._terminal_ports[terminal] = port
         self.add_line_callback(detect_terminals)
 
-    async def stop(self):
+    def stop(self):
         if self._fvp_process:
             self._logger.debug(f"Terminating FVP PID {self._fvp_process.pid}")
             try:
                 self._fvp_process.terminate()
-                await asyncio.wait_for(self._fvp_process.wait(), 10.0)
-            except asyncio.TimeoutError:
+                self._fvp_process.wait(10.0)
+            except subprocess.TimeoutExpired:
                 self._logger.debug(f"Killing FVP PID {self._fvp_process.pid}")
                 self._fvp_process.kill()
             except ProcessLookupError:
@@ -97,8 +96,8 @@  class FVPRunner:
         for telnet in self._telnets:
             try:
                 telnet.terminate()
-                await asyncio.wait_for(telnet.wait(), 10.0)
-            except asyncio.TimeoutError:
+                telnet.wait(10.0)
+            except subprocess.TimeoutExpired:
                 telnet.kill()
             except ProcessLookupError:
                 pass
@@ -118,34 +117,34 @@  class FVPRunner:
         else:
             return 0
 
-    async def run(self, until=None):
+    def run(self, until=None):
         if until and until():
             return
 
-        async for line in self._fvp_process.stdout:
+        for line in self._fvp_process.stdout:
             line = line.strip().decode("utf-8", errors="replace")
             for callback in self._line_callbacks:
                 callback(line)
             if until and until():
                 return
 
-    async def _get_terminal_port(self, terminal, timeout):
+    def _get_terminal_port(self, terminal):
         def terminal_exists():
             return terminal in self._terminal_ports
-        await asyncio.wait_for(self.run(terminal_exists), timeout)
+        self.run(terminal_exists)
         return self._terminal_ports[terminal]
 
-    async def create_telnet(self, terminal, timeout=15.0):
+    def create_telnet(self, terminal):
         check_telnet()
-        port = await self._get_terminal_port(terminal, timeout)
-        telnet = await asyncio.create_subprocess_exec("telnet", "localhost", str(port), stdin=sys.stdin, stdout=sys.stdout)
+        port = self._get_terminal_port(terminal)
+        telnet = subprocess.Popen(["telnet", "localhost", str(port)], stdin=sys.stdin, stdout=sys.stdout)
         self._telnets.append(telnet)
         return telnet
 
-    async def create_pexpect(self, terminal, timeout=15.0, **kwargs):
+    def create_pexpect(self, terminal, **kwargs):
         check_telnet()
         import pexpect
-        port = await self._get_terminal_port(terminal, timeout)
+        port = self._get_terminal_port(terminal)
         instance = pexpect.spawn(f"telnet localhost {port}", **kwargs)
         self._pexpects.append(instance)
         return instance
diff --git a/meta-arm/lib/oeqa/controllers/fvp.py b/meta-arm/lib/oeqa/controllers/fvp.py
index c8dcf298..dfc2b88b 100644
--- a/meta-arm/lib/oeqa/controllers/fvp.py
+++ b/meta-arm/lib/oeqa/controllers/fvp.py
@@ -1,4 +1,3 @@ 
-import asyncio
 import pathlib
 import pexpect
 import os
@@ -25,32 +24,18 @@  class OEFVPSSHTarget(OESSHTarget):
         if not self.fvpconf.exists():
             raise FileNotFoundError(f"Cannot find {self.fvpconf}")
 
-    async def boot_fvp(self):
-        self.fvp = runner.FVPRunner(self.logger)
-        await self.fvp.start(self.config)
-        self.logger.debug(f"Started FVP PID {self.fvp.pid()}")
-        await self._after_start()
-
-    async def _after_start(self):
+    def _after_start(self):
         pass
 
-    async def _after_stop(self):
-        pass
-
-    async def stop_fvp(self):
-        returncode = await self.fvp.stop()
-        await self._after_stop()
-
-        self.logger.debug(f"Stopped FVP with return code {returncode}")
-
     def start(self, **kwargs):
-        # When we can assume Py3.7+, this can simply be asyncio.run()
-        loop = asyncio.get_event_loop()
-        loop.run_until_complete(asyncio.gather(self.boot_fvp()))
+        self.fvp = runner.FVPRunner(self.logger)
+        self.fvp.start(self.config)
+        self.logger.debug(f"Started FVP PID {self.fvp.pid()}")
+        self._after_start()
 
     def stop(self, **kwargs):
-        loop = asyncio.get_event_loop()
-        loop.run_until_complete(asyncio.gather(self.stop_fvp()))
+        returncode = self.fvp.stop()
+        self.logger.debug(f"Stopped FVP with return code {returncode}")
 
 
 class OEFVPTarget(OEFVPSSHTarget):
@@ -66,9 +51,9 @@  class OEFVPTarget(OEFVPSSHTarget):
         # FVPs boot slowly, so allow ten minutes
         self.boot_timeout = 10 * 60
 
-    async def _after_start(self):
+    def _after_start(self):
         self.logger.debug(f"Awaiting console on terminal {self.config['consoles']['default']}")
-        console = await self.fvp.create_pexpect(self.config['consoles']['default'])
+        console = self.fvp.create_pexpect(self.config['consoles']['default'])
         try:
             console.expect("login\\:", timeout=self.boot_timeout)
             self.logger.debug("Found login prompt")
@@ -100,11 +85,11 @@  class OEFVPSerialTarget(OEFVPSSHTarget):
         self.test_log_suffix = pathlib.Path(bootlog).suffix
         self.bootlog = bootlog
 
-    async def _add_terminal(self, name, fvp_name):
+    def _add_terminal(self, name, fvp_name):
         logfile = self._create_logfile(name)
         self.logger.info(f'Creating terminal {name} on {fvp_name}')
         self.terminals[name] = \
-            await self.fvp.create_pexpect(fvp_name, logfile=logfile)
+            self.fvp.create_pexpect(fvp_name, logfile=logfile)
 
     def _create_logfile(self, name):
         fvp_log_file = f"{name}_log{self.test_log_suffix}"
@@ -117,9 +102,9 @@  class OEFVPSerialTarget(OEFVPSSHTarget):
         os.symlink(fvp_log_file, fvp_log_symlink)
         return open(fvp_log_path, 'wb')
 
-    async def _after_start(self):
+    def _after_start(self):
         for name, console in self.config["consoles"].items():
-            await self._add_terminal(name, console)
+            self._add_terminal(name, console)
 
             # testimage.bbclass expects to see a log file at `bootlog`,
             # so make a symlink to the 'default' log file
diff --git a/meta-arm/lib/oeqa/selftest/cases/runfvp.py b/meta-arm/lib/oeqa/selftest/cases/runfvp.py
index cf8a3c53..5cc8660f 100644
--- a/meta-arm/lib/oeqa/selftest/cases/runfvp.py
+++ b/meta-arm/lib/oeqa/selftest/cases/runfvp.py
@@ -81,13 +81,13 @@  class ConfFileTests(OESelftestTestCase):
 
 class RunnerTests(OESelftestTestCase):
     def create_mock(self):
-        return unittest.mock.patch("asyncio.create_subprocess_exec")
+        return unittest.mock.patch("subprocess.Popen")
 
     def test_start(self):
         from fvp import runner
         with self.create_mock() as m:
             fvp = runner.FVPRunner(self.logger)
-            asyncio.run(fvp.start({
+            fvp.start({
                 "fvp-bindir": "/usr/bin",
                 "exe": "FVP_Binary",
                 "parameters": {'foo': 'bar'},
@@ -96,13 +96,13 @@  class RunnerTests(OESelftestTestCase):
                 "terminals": {},
                 "args": ['--extra-arg'],
                 "env": {"FOO": "BAR"}
-            }))
+            })
 
-            m.assert_called_once_with('/usr/bin/FVP_Binary',
+            m.assert_called_once_with(['/usr/bin/FVP_Binary',
                 '--parameter', 'foo=bar',
                 '--data', 'data1',
                 '--application', 'a1=file',
-                '--extra-arg',
+                '--extra-arg'],
                 stdin=unittest.mock.ANY,
                 stdout=unittest.mock.ANY,
                 stderr=unittest.mock.ANY,
@@ -113,7 +113,7 @@  class RunnerTests(OESelftestTestCase):
         from fvp import runner
         with self.create_mock() as m:
             fvp = runner.FVPRunner(self.logger)
-            asyncio.run(fvp.start({
+            fvp.start({
                 "fvp-bindir": "/usr/bin",
                 "exe": "FVP_Binary",
                 "parameters": {},
@@ -122,9 +122,9 @@  class RunnerTests(OESelftestTestCase):
                 "terminals": {},
                 "args": [],
                 "env": {"FOO": "BAR"}
-            }))
+            })
 
-            m.assert_called_once_with('/usr/bin/FVP_Binary',
+            m.assert_called_once_with(['/usr/bin/FVP_Binary'],
                 stdin=unittest.mock.ANY,
                 stdout=unittest.mock.ANY,
                 stderr=unittest.mock.ANY,
diff --git a/scripts/runfvp b/scripts/runfvp
index c5a74b2f..727223ca 100755
--- a/scripts/runfvp
+++ b/scripts/runfvp
@@ -1,6 +1,5 @@ 
 #! /usr/bin/env python3
 
-import asyncio
 import os
 import pathlib
 import signal
@@ -47,11 +46,10 @@  def parse_args(arguments):
     logger.debug(f"FVP arguments: {fvp_args}")
     return args, fvp_args
 
-
-async def start_fvp(args, config, extra_args):
+def start_fvp(args, config, extra_args):
     fvp = runner.FVPRunner(logger)
     try:
-        await fvp.start(config, extra_args, args.terminals)
+        fvp.start(config, extra_args, args.terminals)
 
         if args.console:
             fvp.add_line_callback(lambda line: logger.debug(f"FVP output: {line}"))
@@ -59,15 +57,16 @@  async def start_fvp(args, config, extra_args):
             if not expected_terminal:
                 logger.error("--console used but FVP_CONSOLE not set in machine configuration")
                 return 1
-            telnet = await fvp.create_telnet(expected_terminal)
-            await telnet.wait()
+            telnet = fvp.create_telnet(expected_terminal)
+            telnet.wait()
             logger.debug(f"Telnet quit, cancelling tasks")
         else:
             fvp.add_line_callback(lambda line: print(line))
-            await fvp.run()
+            fvp.run()
 
     finally:
-        await fvp.stop()
+        fvp.stop()
+
 
 def runfvp(cli_args):
     args, extra_args = parse_args(cli_args)
@@ -77,14 +76,8 @@  def runfvp(cli_args):
         config_file = conffile.find(args.config)
     logger.debug(f"Loading {config_file}")
     config = conffile.load(config_file)
+    start_fvp(args, config, extra_args)
 
-    try:
-        # When we can assume Py3.7+, this can simply be asyncio.run()
-        loop = asyncio.get_event_loop()
-        return loop.run_until_complete(start_fvp(args, config, extra_args))
-    except asyncio.CancelledError:
-        # This means telnet exited, which isn't an error
-        return 0
 
 if __name__ == "__main__":
     try: