Change in vdsm[ovirt-3.5]: vm: ensure valid Vm._dom before domDependentInit

fromani at redhat.com fromani at redhat.com
Tue Aug 19 15:40:04 UTC 2014


Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/31670

to review the following change.

Change subject: vm: ensure valid Vm._dom before domDependentInit
......................................................................

vm: ensure valid Vm._dom before domDependentInit

If we reach the _domDependentInit stage of a VM
creation, for every possible flow, including
recovery, we must guarantee that the _dom
attribute is not None, thus VDSM and libvirt
are connected.

Quite a lot of errors could be ignored on
recovery, but this cannot, ever.

This patch moves the '_dom is not None'
check inside _domDependentInit, and makes
it mandatory for each flow.

Change-Id: Ie2840b066260790d4f3079a9cd34f87a70e96891
Signed-off-by: Francesco Romani <fromani at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/30146
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 15 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/70/31670/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 729b469..20cd5e3 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1825,7 +1825,10 @@
 
 
 class MissingLibvirtDomainError(Exception):
-    pass
+    def __init__(self, reason=vmexitreason.LIBVIRT_DOMAIN_MISSING):
+        super(Exception, self).__init__(
+            self, vmexitreason.exitReasons.get(reason, 'Missing VM'))
+        self.reason = reason
 
 
 class Vm(object):
@@ -2272,9 +2275,13 @@
                     # behaviors on VM start/destroy, because the tuning can be
                     # done automatically according to its statistical data.
                     self.cif.ksmMonitor.adjust()
+            except MissingLibvirtDomainError:
+                # we cannot continue without a libvirt domain object,
+                # not even on recovery, to avoid state desync or worse
+                # split-brain scenarios.
+                raise
             except Exception as e:
-                # we cannot continue without a libvirt domain object
-                # to avoid state desync or worse split-brain scenarios.
+                # as above
                 if isinstance(e, libvirt.libvirtError) and \
                    e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
                     raise MissingLibvirtDomainError()
@@ -2304,13 +2311,13 @@
 
             self.recovering = False
             self.saveState()
-        except MissingLibvirtDomainError:
+        except MissingLibvirtDomainError as e:
             # we cannot ever deal with this error, not even on recovery.
             self.setDownStatus(
                 self.conf.get(
                     'exitCode', ERROR),
                 self.conf.get(
-                    'exitReason', vmexitreason.LIBVIRT_DOMAIN_MISSING),
+                    'exitReason', e.reason),
                 self.conf.get(
                     'exitMessage', ''))
             self.recovering = False
@@ -3220,6 +3227,9 @@
                 pass
             raise Exception('destroy() called before Vm started')
 
+        if self._dom is None:
+            raise MissingLibvirtDomainError(vmexitreason.LIBVIRT_START_FAILED)
+
         self._getUnderlyingVmInfo()
         self._getUnderlyingVmDevicesInfo()
         self._updateAgentChannels()
@@ -3336,9 +3346,6 @@
                 hooks.after_device_create(dev._deviceXML, self.conf,
                                           dev.custom)
 
-        if not self._dom:
-            self.setDownStatus(ERROR, vmexitreason.LIBVIRT_START_FAILED)
-            return
         self._domDependentInit()
 
     def _updateDevices(self, devices):


-- 
To view, visit http://gerrit.ovirt.org/31670
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie2840b066260790d4f3079a9cd34f87a70e96891
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list