Re: [Qemu-devel] [PATCH 05/11] qemu.py: Use custom exceptions rather than Exception

[Eduardo Habkost]

On Thu, Jul 20, 2017 at 06:28:09PM +0200, Lukáš Doktor wrote:
> The naked Exception should not be widely used. It makes sense to be a
> bit more specific and use better-suited custom exceptions. As a benefit
> we can store the full reply in the exception in case someone needs it
> when catching the exception.
> 
> Signed-off-by: Lukáš Doktor <address@hidden>
> ---
>  scripts/qemu.py | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 5948e19..2bd522f 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -19,6 +19,19 @@ import subprocess
>  import qmp.qmp
>  
>  
> +class MonitorResponseError(qmp.qmp.QMPError):
> +    '''
> +    Represents erroneous QMP monitor reply
> +    '''
> +    def __init__(self, reply):
> +        try:
> +            desc = reply["error"]["desc"]
> +        except KeyError:
> +            desc = reply
> +        super(MonitorResponseError, self).__init__(desc)
> +        self.reply = reply

This would require every user of self.reply to first check if
it's a string or dictionary.  All because of the "Monitor is
closed" case below:

> +
> +
>  class QEMUMachine(object):
>      '''A QEMU VM'''
>  
> @@ -197,9 +210,9 @@ class QEMUMachine(object):
>          '''
>          reply = self.qmp(cmd, conv_keys, **args)
>          if reply is None:
> -            raise Exception("Monitor is closed")
> +            raise qmp.qmp.QMPError("Monitor is closed")

Should we really use the same exception type for this, if it's
not really a QMP monitor error reply, and won't even have a real
QMP reply in self.reply?


>          if "error" in reply:
> -            raise Exception(reply["error"]["desc"])
> +            raise MonitorResponseError(reply)
>          return reply["return"]
>  
>      def get_qmp_event(self, wait=False):
> -- 
> 2.9.4
> 

-- 
Eduardo