Re: [PATCH RFC 16/32] python//qmp.py: re-absorb MonitorResponseError

[Philippe Mathieu-Daudé]

On 5/14/20 7:53 AM, John Snow wrote:
> When I initially split this out, I considered this more of a machine
> error than a QMP protocol error, but I think that's misguided.
>
> Move this back to qmp.py and name it QMPResponseError. Convert
> qmp.command() to use this exception type.
>
> Signed-off-by: John Snow <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
>   python/qemu/lib/machine.py    | 15 +--------------
>   python/qemu/lib/qmp.py        | 17 +++++++++++++++--
>   scripts/render_block_graph.py |  4 ++--
>   3 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/python/qemu/lib/machine.py b/python/qemu/lib/machine.py
> index 2f94c851ed..c31bf7cabb 100644
> --- a/python/qemu/lib/machine.py
> +++ b/python/qemu/lib/machine.py
> @@ -48,19 +48,6 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
>       """
>   
>   
> -class MonitorResponseError(qmp.QMPError):
> -    """
> -    Represents erroneous QMP monitor reply
> -    """
> -    def __init__(self, reply):
> -        try:
> -            desc = reply["error"]["desc"]
> -        except KeyError:
> -            desc = reply
> -        super().__init__(desc)
> -        self.reply = reply
> -
> -
>   class QEMUMachine:
>       """
>       A QEMU VM
> @@ -433,7 +420,7 @@ def command(self, cmd, conv_keys=True, **args):
>           if reply is None:
>               raise qmp.QMPError("Monitor is closed")
>           if "error" in reply:
> -            raise MonitorResponseError(reply)
> +            raise qmp.QMPResponseError(reply)
>           return reply["return"]
>   
>       def get_qmp_event(self, wait=False):
> diff --git a/python/qemu/lib/qmp.py b/python/qemu/lib/qmp.py
> index 911da59888..82f86b4e45 100644
> --- a/python/qemu/lib/qmp.py
> +++ b/python/qemu/lib/qmp.py
> @@ -61,6 +61,19 @@ class QMPTimeoutError(QMPError):
>       """
>   
>   
> +class QMPResponseError(QMPError):
> +    """
> +    Represents erroneous QMP monitor reply
> +    """
> +    def __init__(self, reply: QMPMessage):
> +        try:
> +            desc = reply['error']['desc']
> +        except KeyError:
> +            desc = reply
> +        super().__init__(desc)
> +        self.reply = reply
> +
> +
>   class QEMUMonitorProtocol:
>       """
>       Provide an API to connect to QEMU via QEMU Monitor Protocol (QMP) and then
> @@ -250,8 +263,8 @@ def command(self, cmd, **kwds):
>           Build and send a QMP command to the monitor, report errors if any
>           """
>           ret = self.cmd(cmd, kwds)
> -        if "error" in ret:
> -            raise Exception(ret['error']['desc'])
> +        if 'error' in ret:
> +            raise QMPResponseError(ret)
>           return ret['return']
>   
>       def pull_event(self, wait=False):
> diff --git a/scripts/render_block_graph.py b/scripts/render_block_graph.py
> index 8048d9fbbe..332ab49a91 100755
> --- a/scripts/render_block_graph.py
> +++ b/scripts/render_block_graph.py
> @@ -26,7 +26,7 @@
>   
>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
>   from qemu.lib import QEMUMonitorProtocol
> -from qemu.lib.machine import MonitorResponseError
> +from qemu.lib.qmp import QMPResponseError
>   
>   
>   def perm(arr):
> @@ -103,7 +103,7 @@ def command(self, cmd):
>           reply = json.loads(subprocess.check_output(ar))
>   
>           if 'error' in reply:
> -            raise MonitorResponseError(reply)
> +            raise QEMUResponseError(reply)
>   
>           return reply['return']
>