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