Change in vdsm[master]: vm: abort vm start if can't read pid

fromani at redhat.com fromani at redhat.com
Fri Dec 12 12:54:56 UTC 2014


Francesco Romani has uploaded a new change for review.

Change subject: vm: abort vm start if can't read pid
......................................................................

vm: abort vm start if can't read pid

If for whatever (quite unlikely) cause
the PID can't be read, or it is invalid,
abort the creation of the VM.

Change-Id: I1b80baba14caded2fb3ef74451498751347323ea
Bug-Url: https://bugzilla.redhat.com/1128458
Signed-off-by: Francesco Romani <fromani at redhat.com>
---
M tests/vmTests.py
M tests/vmfakelib.py
M vdsm/virt/vm.py
3 files changed, 61 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/36129/1

diff --git a/tests/vmTests.py b/tests/vmTests.py
index 24c3893..3f3d898 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -1748,6 +1748,32 @@
                                 lambda: self._getAllDomains('novdsm'))]):
             self.assertFalse(vm.getVDSMDomains())
 
+    def testGetPidNoFile(self):
+        with MonkeyPatchScope([(vm, 'supervdsm',
+                                fake.SuperVdsm(exception=IOError))]):
+            with fake.VM() as testvm:
+                self.assertRaises(vm.MissingVmPidError, testvm._getPid)
+
+    def testGetPidBadFile(self):
+        with MonkeyPatchScope([(vm, 'supervdsm',
+                                fake.SuperVdsm(exception=ValueError))]):
+            with fake.VM() as testvm:
+                self.assertRaises(vm.MissingVmPidError, testvm._getPid)
+
+    @permutations([[-1], [0]])
+    def testGetPidBadFileContent(self, pid):
+        with MonkeyPatchScope([(vm, 'supervdsm',
+                                fake.SuperVdsm(pid=pid))]):
+            with fake.VM() as testvm:
+                self.assertRaises(vm.MissingVmPidError, testvm._getPid)
+
+    def testGetPidSuccess(self):
+        pid = 42
+        with MonkeyPatchScope([(vm, 'supervdsm',
+                                fake.SuperVdsm(pid=pid))]):
+            with fake.VM() as testvm:
+                self.assertEqual(testvm._getPid(), pid)
+
 
 class TestVmStatusTransitions(TestCaseBase):
     @slowtest
diff --git a/tests/vmfakelib.py b/tests/vmfakelib.py
index 83c5b7a..c5b3861 100644
--- a/tests/vmfakelib.py
+++ b/tests/vmfakelib.py
@@ -181,13 +181,19 @@
             yield fake
 
 
-class SuperVdsm:
-    def __init__(self):
-        pass
+class SuperVdsm(object):
+    def __init__(self, exception=None, pid=42):
+        self._exception = exception
+        self._pid = pid
 
     def getProxy(self):
         return self
 
+    def getVmPid(self, vmname):
+        if self._exception:
+            raise self._exception()
+        return self._pid
+
     def getVcpuNumaMemoryMapping(self, vmName):
         return {0: [0, 1], 1: [0, 1], 2: [0, 1], 3: [0, 1]}
 
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 43f5790..deba16e 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1300,10 +1300,21 @@
     pass
 
 
-class MissingLibvirtDomainError(Exception):
+class VmStartupError(Exception):
+    pass
+
+
+class MissingLibvirtDomainError(VmStartupError):
     def __init__(self, reason=vmexitreason.LIBVIRT_DOMAIN_MISSING):
         super(Exception, self).__init__(
             self, vmexitreason.exitReasons.get(reason, 'Missing VM'))
+        self.reason = reason
+
+
+class MissingVmPidError(VmStartupError):
+    def __init__(self, reason=vmexitreason.LIBVIRT_START_FAILED):
+        super(Exception, self).__init__(
+            self, vmexitreason.exitReasons.get(reason, 'Missing VM PID'))
         self.reason = reason
 
 
@@ -1759,10 +1770,9 @@
                 self._vmCreationEvent.set()
                 try:
                     self._run()
-                except MissingLibvirtDomainError:
-                    # we cannot continue without a libvirt domain object,
-                    # not even on recovery, to avoid state desync or worse
-                    # split-brain scenarios.
+                except VmStartupError:
+                    # we cannot continue on this state, not even on recovery,
+                    # to avoid state desync or worse split-brain scenarios.
                     raise
                 except Exception as e:
                     # as above
@@ -1793,7 +1803,7 @@
 
             self.recovering = False
             self.saveState()
-        except MissingLibvirtDomainError as e:
+        except VmStartupError as e:
             # we cannot ever deal with this error, not even on recovery.
             self.setDownStatus(
                 self.conf.get(
@@ -2694,7 +2704,8 @@
             self.conf['pauseCode'] = self._initTimePauseCode
             if self._initTimePauseCode == 'ENOSPC':
                 self.cont()
-        self.conf['pid'] = self._getPid()
+
+        self.conf['pid'] = str(self._getPid())
 
         nice = int(self.conf.get('nice', '0'))
         nice = max(min(nice, 19), 0)
@@ -4346,12 +4357,16 @@
                     if dev['type'] == GRAPHICS_DEVICES))
 
     def _getPid(self):
-        pid = '0'
+        pid = 0
         try:
             vmName = self.conf['vmName'].encode('utf-8')
-            pid = supervdsm.getProxy().getVmPid(vmName)
-        except Exception:
-            pass
+            pid = int(supervdsm.getProxy().getVmPid(vmName))
+            if pid <= 0:
+                self.log.error('read invalid pid: %i', pid)
+                raise MissingVmPidError()
+        except (IOError, ValueError):
+            self.log.error('cannot read pid')
+            raise MissingVmPidError()
         return pid
 
     def _updateDomainDescriptor(self):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1b80baba14caded2fb3ef74451498751347323ea
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>


More information about the vdsm-patches mailing list