[virt-tools-list] [PATCH v2] virt-manager: ignore VIR_ERR_NO_DOMAIN when a domain was just deleted

[Cole Robinson]

On 08/16/2013 09:59 AM, Giuseppe Scrivano wrote:
> Some callbacks could try to access a domain that was just deleted and
> not accessible anymore.  Detect these cases and don't propagate the
> exception.
> 
> Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
> ---
> This error appeared in some other cases, that are fixed by the patch now.
> 
> I have tried to put the entire logic in a separate function (i.e. having a
> inhibit_libvirt_error which contains the try/except code) but it didn't
> seem readable, so I preferred the following version:
> 
>  virtManager/details.py   | 10 ++++++++--
>  virtManager/domain.py    | 13 ++++++++-----
>  virtManager/manager.py   | 29 ++++++++++++++++++-----------
>  virtManager/uihelpers.py |  4 ++++
>  4 files changed, 38 insertions(+), 18 deletions(-)
> 
> diff --git a/virtManager/details.py b/virtManager/details.py
> index c8103db..35b8238 100644
> --- a/virtManager/details.py
> +++ b/virtManager/details.py
> @@ -2637,8 +2637,14 @@ class vmmDetails(vmmGObjectUI):
>  
>          # If the dialog is visible, we want to make sure the XML is always
>          # up to date
> -        if self.is_visible():
> -            self.vm.refresh_xml()
> +        try:
> +            if self.is_visible():
> +                self.vm.refresh_xml()
> +        except libvirt.libvirtError, e:
> +            if uihelpers.exception_is_libvirt_error(e, "VIR_ERR_NO_DOMAIN"):
> +                self.close()
> +                return
> +            raise
>  
>          # Stats page needs to be refreshed every tick
>          if (page == PAGE_DETAILS and
> diff --git a/virtManager/domain.py b/virtManager/domain.py
> index 234cf4a..ff57fbc 100644
> --- a/virtManager/domain.py
> +++ b/virtManager/domain.py
> @@ -411,7 +411,12 @@ class vmmDomain(vmmLibvirtObject):
>                                parsexml=xml)
>  
>      def _reparse_xml(self, ignore=None):
> -        self._guest = self._build_guest(self._get_domain_xml())
> +        try:
> +            self._guest = self._build_guest(self._get_domain_xml())
> +        except libvirt.libvirtError, e:
> +            if uihelpers.exception_is_libvirt_error(e, "VIR_ERR_NO_DOMAIN"):
> +                return
> +            raise
>  
>  
>      ##############################
> @@ -1561,14 +1566,12 @@ class vmmDomain(vmmLibvirtObject):
>          """
>          try:
>              info = self._backend.info()
> +            self._update_status(info[0])
>          except libvirt.libvirtError, e:
> -            if (hasattr(libvirt, "VIR_ERR_NO_DOMAIN") and
> -                e.get_error_code() == getattr(libvirt, "VIR_ERR_NO_DOMAIN")):
> -                # Possibly a transient domain that was removed on shutdown
> +            if uihelpers.exception_is_libvirt_error(e, "VIR_ERR_NO_DOMAIN"):
>                  return
>              raise
>  
> -        self._update_status(info[0])
>  
>      def _update_status(self, status):
>          """
> diff --git a/virtManager/manager.py b/virtManager/manager.py
> index bf83427..9116a32 100644
> --- a/virtManager/manager.py
> +++ b/virtManager/manager.py
> @@ -34,6 +34,7 @@ from virtManager.connection import vmmConnection
>  from virtManager.baseclass import vmmGObjectUI
>  from virtManager.graphwidgets import CellRendererSparkline
>  
> +import libvirt
>  
>  # Number of data points for performance graphs
>  GRAPH_LEN = 40
> @@ -891,18 +892,24 @@ class vmmManager(vmmGObjectUI):
>          if self.vm_row_key(vm) not in self.rows:
>              return
>  
> -        row = self.rows[self.vm_row_key(vm)]
> -        row[ROW_NAME] = vm.get_name()
> -        row[ROW_STATUS] = vm.run_status()
> -        row[ROW_STATUS_ICON] = vm.run_status_icon_name()
> -        row[ROW_IS_VM_RUNNING] = vm.is_active()
> -        row[ROW_MARKUP] = self._build_vm_markup(row)
>  
> -        if config_changed:
> -            desc = vm.get_description()
> -            if not can_set_row_none:
> -                desc = desc or ""
> -            row[ROW_HINT] = util.xml_escape(desc)
> +        try:
> +            row = self.rows[self.vm_row_key(vm)]
> +            row[ROW_NAME] = vm.get_name()
> +            row[ROW_STATUS] = vm.run_status()
> +            row[ROW_STATUS_ICON] = vm.run_status_icon_name()
> +            row[ROW_IS_VM_RUNNING] = vm.is_active()
> +            row[ROW_MARKUP] = self._build_vm_markup(row)
> +
> +            if config_changed:
> +                desc = vm.get_description()
> +                if not can_set_row_none:
> +                    desc = desc or ""
> +                    row[ROW_HINT] = util.xml_escape(desc)
> +        except libvirt.libvirtError, e:
> +            if uihelpers.exception_is_libvirt_error(e, "VIR_ERR_NO_DOMAIN"):
> +                return
> +            raise
>  
>          model.row_changed(row.path, row.iter)
>  
> diff --git a/virtManager/uihelpers.py b/virtManager/uihelpers.py
> index 3fd0ed8..5320ced 100644
> --- a/virtManager/uihelpers.py
> +++ b/virtManager/uihelpers.py
> @@ -1345,3 +1345,7 @@ def default_uri(always_system=False):
>          else:
>              return "qemu:///session"
>      return None
> +
> +def exception_is_libvirt_error(e, error):
> +    return (hasattr(libvirt, error) and
> +            e.get_error_code() == getattr(libvirt, error))
> 

ACK, pushed now

- Cole