Re: [PATCH 06/13] scripts/qmp_helper: add support for a timeout logic

[Jonathan Cameron via qemu development <qemu-devel@nongnu.org>]

On Wed, 21 Jan 2026 12:25:14 +0100
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> We can't inject a new GHES record to the same source before
> it has been acked. There is an async mechanism to verify when
> the Kernel is ready, which is implemented at QEMU's ghes
> driver.
> 
> If error inject is too fast, QEMU may return an error. When
> such errors occur, implement a retry mechanism, based on a
> maximum timeout.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
A few trivial comments below. Either way this seems fine to me and
should make the tooling easier to use.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> ---
>  scripts/qmp_helper.py | 47 +++++++++++++++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 13 deletions(-)
> 
> diff --git a/scripts/qmp_helper.py b/scripts/qmp_helper.py
> index 40059cd105f6..63f3df2d75c3 100755
> --- a/scripts/qmp_helper.py
> +++ b/scripts/qmp_helper.py
> @@ -14,6 +14,7 @@
>  
>  from datetime import datetime
>  from os import path as os_path
> +from time import sleep
>  
>  try:
>      qemu_dir = os_path.abspath(os_path.dirname(os_path.dirname(__file__)))
> @@ -324,7 +325,8 @@ class qmp:
>      Opens a connection and send/receive QMP commands.
>      """
>  
> -    def send_cmd(self, command, args=None, may_open=False, return_error=True):
> +    def send_cmd(self, command, args=None, may_open=False, return_error=True,
> +                 timeout=None):
>          """Send a command to QMP, optinally opening a connection"""
>  
>          if may_open:
> @@ -336,12 +338,31 @@ def send_cmd(self, command, args=None, may_open=False, return_error=True):
>          if args:
>              msg['arguments'] = args
>  
> -        try:
> -            obj = self.qmp_monitor.cmd_obj(msg)
> -        # Can we use some other exception class here?
> -        except Exception as e:                         # pylint: disable=W0718
> -            print(f"Command: {command}")
> -            print(f"Failed to inject error: {e}.")
> +        if timeout and timeout > 0:
> +            attempts = int(timeout * 10)
> +        else:
> +            attempts = 1
> +
> +        # Try up to attempts
That reads oddly because of the variable name.  Made me ask myself
"How many attempts?"
Maybe  " Retry up to attempts times" or something like that.

> +        for i in range(0, attempts):
> +            try:
> +                obj = self.qmp_monitor.cmd_obj(msg)
> +
> +                if obj and "return" in obj and not obj["return"]:
> +                    break
> +
> +            except Exception as e:                     # pylint: disable=W0718
> +                print(f"Command: {command}")
> +                print(f"Failed to inject error: {e}.")
> +                obj = None
> +
> +            if attempts > 1:
> +                print(f"Error inject attempt {i + 1}/{attempts} failed.")
> +
> +            if i + 1 < attempts:
> +                sleep(0.1)

Do we care about a sleep at the end?  Feels like a micro optimization that
isn't needed.

> +
> +        if not obj:
>              return None