| Message ID | 20230411150503.2105880-2-lrannou@baylibre.com |
|---|---|
| State | New |
| Headers | show |
| Series | oeqa: Change the serial runner | expand |
Hello Louis, On Tue, 11 Apr 2023 17:05:02 +0200 "Louis Rannou" <lrannou@baylibre.com> wrote: > [YOCTO #15021] > > Create a new runner run_serial_socket which usage matches the traditional ssh Nit: I'm not a native English speaker, but I think "whose usage" is the correct form. > runner. Its return status is 0 when the command succeeded or 0 when it > failed. If an error is encountered, it raises an Exception. Should be "or a negative error when it fails"? I'm taking the patch for testing with these two changes, let me know if I should do otherwise. Luca
Hello Luca, You are correct, thanks ! Louis On 12/04/2023 09:57, Luca Ceresoli wrote: > Hello Louis, > > On Tue, 11 Apr 2023 17:05:02 +0200 > "Louis Rannou" <lrannou@baylibre.com> wrote: > >> [YOCTO #15021] >> >> Create a new runner run_serial_socket which usage matches the traditional ssh > > Nit: I'm not a native English speaker, but I think "whose usage" is the > correct form. > >> runner. Its return status is 0 when the command succeeded or 0 when it >> failed. If an error is encountered, it raises an Exception. > > Should be "or a negative error when it fails"? > > I'm taking the patch for testing with these two changes, let me know if > I should do otherwise. > > Luca >
On 11 Apr 2023, at 16:05, Louis Rannou via lists.openembedded.org <lrannou=baylibre.com@lists.openembedded.org> wrote: > Create a new runner run_serial_socket which usage matches the traditional ssh > runner. Its return status is 0 when the command succeeded or 0 when it > failed. If an error is encountered, it raises an Exception. > > The previous serial runner is maintained and marked as deprecated. I absolutely love this because the existing run_serial has the most confusing behaviour. However, we’re now duplicating a large chunk of code: can the old function be implemented such that it calls the new function and adapts the return values? Ross
> On 5 May 2023, at 11:31, Ross Burton <ross.burton@arm.com> wrote: > > On 11 Apr 2023, at 16:05, Louis Rannou via lists.openembedded.org <lrannou=baylibre.com@lists.openembedded.org> wrote: >> Create a new runner run_serial_socket which usage matches the traditional ssh >> runner. Its return status is 0 when the command succeeded or 0 when it >> failed. If an error is encountered, it raises an Exception. >> >> The previous serial runner is maintained and marked as deprecated. > > I absolutely love this because the existing run_serial has the most confusing behaviour. However, we’re now duplicating a large chunk of code: can the old function be implemented such that it calls the new function and adapts the return values? Also, for bonus points, make the old function actual spit out a DeprecationWarning with warnings.warn. Ross
On Fri, 2023-05-05 at 10:32 +0000, Ross Burton wrote: > On 11 Apr 2023, at 16:05, Louis Rannou via lists.openembedded.org <lrannou=baylibre.com@lists.openembedded.org> wrote: > > Create a new runner run_serial_socket which usage matches the traditional ssh > > runner. Its return status is 0 when the command succeeded or 0 when it > > failed. If an error is encountered, it raises an Exception. > > > > The previous serial runner is maintained and marked as deprecated. > > I absolutely love this because the existing run_serial has the most > confusing behaviour. However, we’re now duplicating a large chunk of > code: can the old function be implemented such that it calls the new > function and adapts the return values? This has hovered in the patch queue for that reason. I also love the idea of it, it just doesn't feel quite ready. I've been hoping we could convert the existing users and be able to drop the old function but I'm not managed to see how much work is left for that. Cheers, Richard
On 05/05/2023 12:50, Richard Purdie wrote: > On Fri, 2023-05-05 at 10:32 +0000, Ross Burton wrote: >> On 11 Apr 2023, at 16:05, Louis Rannou via lists.openembedded.org <lrannou=baylibre.com@lists.openembedded.org> wrote: >>> Create a new runner run_serial_socket which usage matches the traditional ssh >>> runner. Its return status is 0 when the command succeeded or 0 when it >>> failed. If an error is encountered, it raises an Exception. >>> >>> The previous serial runner is maintained and marked as deprecated. >> >> I absolutely love this because the existing run_serial has the most >> confusing behaviour. However, we’re now duplicating a large chunk of >> code: can the old function be implemented such that it calls the new >> function and adapts the return values? > > This has hovered in the patch queue for that reason. I also love the > idea of it, it just doesn't feel quite ready. > > I've been hoping we could convert the existing users and be able to > drop the old function but I'm not managed to see how much work is left > for that. My initial idea was indeed to change the former run_serial to use the new run_serial_socket. The problem is that run_serial_socket can raise exceptions which are not handled by run_serial. It's not possible to catch them except if we create a very specific exception to this subject. About the second question: how much work is left. serial_runner remains in: - selftest/cases/wic.py - selftest/cases/imagefeatures.py - selftest/cases/overlayfs.py - selftest/cases/runtime_test.py - utils/dump.py It seemed to me that fixing those is nos as easy. Further investigation is possible, but it would be much more comfortable for me to see the first stage validated. I would say this is a pretty good checkpoint for saving this work while I try to fix what can be. Thanks, Louis
diff --git a/meta/lib/oeqa/targetcontrol.py b/meta/lib/oeqa/targetcontrol.py index d686fe07ec..3707883da7 100644 --- a/meta/lib/oeqa/targetcontrol.py +++ b/meta/lib/oeqa/targetcontrol.py @@ -206,6 +206,9 @@ class QemuTarget(BaseTarget): def run_serial(self, command, timeout=60): return self.runner.run_serial(command, timeout=timeout) + def run_serial_socket(self, command, timeout=60): + return self.runner.run_serial_socket(command, timeout=timeout) + class SimpleRemoteTarget(BaseTarget): diff --git a/meta/lib/oeqa/utils/qemurunner.py b/meta/lib/oeqa/utils/qemurunner.py index 6734cee48d..7e1c7a7eac 100644 --- a/meta/lib/oeqa/utils/qemurunner.py +++ b/meta/lib/oeqa/utils/qemurunner.py @@ -637,6 +637,7 @@ class QemuRunner: return self.qmp.cmd(command) def run_serial(self, command, raw=False, timeout=60): + # Deprecated # Returns (status, output) where status is 1 on success and 0 on error # We assume target system have echo to get command status @@ -688,6 +689,62 @@ class QemuRunner: status = 1 return (status, str(data)) + def run_serial_socket(self, command, raw=False, timeout=60): + # Returns (status, output) where status is 0 on success and a negative value on error. + + # We assume target system have echo to get command status + if not raw: + command = "%s; echo $?\n" % command + + data = '' + status = 0 + self.server_socket.sendall(command.encode('utf-8')) + start = time.time() + end = start + timeout + while True: + now = time.time() + if now >= end: + data += "<<< run_serial_socket(): command timed out after %d seconds without output >>>\r\n\r\n" % timeout + break + try: + sread, _, _ = select.select([self.server_socket],[],[], end - now) + except InterruptedError: + continue + if sread: + # try to avoid reading single character at a time + time.sleep(0.1) + answer = self.server_socket.recv(1024) + if answer: + data += answer.decode('utf-8') + # Search the prompt to stop + if re.search(self.boot_patterns['search_cmd_finished'], data): + break + else: + if self.canexit: + return (1, "") + raise Exception("No data on serial console socket, connection closed?") + + if not data: + raise Exception('serial run failed: no data') + + if raw: + status = 0 + else: + # Remove first line (command line) and last line (prompt) + data = data[data.find('$?\r\n')+4:data.rfind('\r\n')] + index = data.rfind('\r\n') + if index == -1: + data = "" + raise Exception('serial run failed: no result') + else: + status_cmd = data[index+2:] + data = data[:index] + try: + status = int(status_cmd) + except ValueError as e: + raise Exception('Could not convert to integer: {}'.format(str(e))) + return (status, str(data)) + def _dump_host(self): self.host_dumper.create_dir("qemu")
[YOCTO #15021] Create a new runner run_serial_socket which usage matches the traditional ssh runner. Its return status is 0 when the command succeeded or 0 when it failed. If an error is encountered, it raises an Exception. The previous serial runner is maintained and marked as deprecated. Signed-off-by: Louis Rannou <lrannou@baylibre.com> --- meta/lib/oeqa/targetcontrol.py | 3 ++ meta/lib/oeqa/utils/qemurunner.py | 57 +++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+)