[PATCH v2 0/1] Change the serial runner usage

[PATCH v2 0/1] Change the serial runner usage

[Louis Rannou]

The actual serial runner has a different usage compare to the ssh runner. The
return status is different and failure are not raised as exceptions.

Initially, I wanted to create a new run_serial_socket and modify the old
run_serial to use the former. And there was a second patch that changed every
call to run_serial to run_serial_socket. But that is not easy as each test
can have a different manner to handle the exception.

Therefore, this patch only suggest a new runner and add a comment to deprecate
the former runner.

Louis Rannou (1):
  oeqa/utils/qemurunner: change the serial runner usage

 meta/lib/oeqa/targetcontrol.py    |  3 ++
 meta/lib/oeqa/utils/qemurunner.py | 57 +++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)


base-commit: 76db0baba26a5da5382f7cc44c64eb705e24e638
-- 
2.39.2

[PATCH v2 1/1] oeqa/utils/qemurunner: change the serial runner

[Louis Rannou]

[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(+)

diff --git a/meta/lib/oeqa/targetcontrol.py b/meta/lib/oeqa/targetcontrol.py
index 1fdff82889..99fbcb5879 100644
--- a/meta/lib/oeqa/targetcontrol.py
+++ b/meta/lib/oeqa/targetcontrol.py
@@ -210,6 +210,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 05385763ac..418e06a98e 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")
-- 
2.39.2

Re: [OE-core] [PATCH v2 0/1] Change the serial runner usage

[Richard Purdie]

On Fri, 2023-03-03 at 10:01 +0100, Louis Rannou wrote:
> The actual serial runner has a different usage compare to the ssh runner. The
> return status is different and failure are not raised as exceptions.
> 
> Initially, I wanted to create a new run_serial_socket and modify the old
> run_serial to use the former. And there was a second patch that changed every
> call to run_serial to run_serial_socket. But that is not easy as each test
> can have a different manner to handle the exception.
> 
> Therefore, this patch only suggest a new runner and add a comment to deprecate
> the former runner.
> 
> Louis Rannou (1):
>   oeqa/utils/qemurunner: change the serial runner usage
> 
>  meta/lib/oeqa/targetcontrol.py    |  3 ++
>  meta/lib/oeqa/utils/qemurunner.py | 57 +++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+)

Sorry about the late reply to this. I think the idea of the new runner
is great and we should improve and clean this up.

As the patch stands, it creates a largely duplicate function with no
users. Experience shows this tends to bitrot quickly.

For that reason I'm reluctant to take just this change without
converting at least some of the existing users to use the new API.

Ideally we'd convert everything but at the very least the new function
should be usable by a decent chunk of the existing code, even if we
have to adjust it to make that work.

Cheers,

Richard

Re: [OE-core] [PATCH v2 0/1] Change the serial runner usage

[Louis Rannou]

On 29/03/2023 12:53, Richard Purdie wrote:
> On Fri, 2023-03-03 at 10:01 +0100, Louis Rannou wrote:
>> The actual serial runner has a different usage compare to the ssh runner. The
>> return status is different and failure are not raised as exceptions.
>>
>> Initially, I wanted to create a new run_serial_socket and modify the old
>> run_serial to use the former. And there was a second patch that changed every
>> call to run_serial to run_serial_socket. But that is not easy as each test
>> can have a different manner to handle the exception.
>>
>> Therefore, this patch only suggest a new runner and add a comment to deprecate
>> the former runner.
>>
>> Louis Rannou (1):
>>    oeqa/utils/qemurunner: change the serial runner usage
>>
>>   meta/lib/oeqa/targetcontrol.py    |  3 ++
>>   meta/lib/oeqa/utils/qemurunner.py | 57 +++++++++++++++++++++++++++++++
>>   2 files changed, 60 insertions(+)
> 
> Sorry about the late reply to this. I think the idea of the new runner
> is great and we should improve and clean this up.
> 
> As the patch stands, it creates a largely duplicate function with no
> users. Experience shows this tends to bitrot quickly.
> 
> For that reason I'm reluctant to take just this change without
> converting at least some of the existing users to use the new API.
> 
> Ideally we'd convert everything but at the very least the new function
> should be usable by a decent chunk of the existing code, even if we
> have to adjust it to make that work.

Hello,
You're right. I'll send a v3 changing some tests using that serial runner.

Thanks,
Louis