Dan Kenigsberg has posted comments on this change. Change subject: recovery: try to restore VMs from recovery files ......................................................................
Patch Set 6: Code-Review-1 (5 comments) very partial review http://gerrit.ovirt.org/#/c/25276/6/tests/functional/recoveryTests.py File tests/functional/recoveryTests.py: Line 1: # Line 2: # Copyright 2014 Red Hat, Inc. How about naming this file vmRecoveryTests? just "recovery" is too opaque imo. Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by Line 6: # the Free Software Foundation; either version 2 of the License, or Line 56: global _initramfsPath Line 57: global _tmpinitramfs Line 58: _initramfsPath = _detectBootImages(_initramfsPaths) Line 59: if _initramfsPath is None: Line 60: _initramfsPath = _genInitramfs() Why do we need a running guest for these tests? _genInitramfs() is soo slow. Line 61: _tmpinitramfs = True Line 62: Line 63: Line 64: def tearDownModule(): Line 100: kernelPath=_kernelPath, Line 101: initramfsPath=_initramfsPath) Line 102: vmId = vm.start() Line 103: self._waitForStartup(vmId, VM_MINIMAL_UPTIME) Line 104: status, msg, stats = self.vdsm.getVmList(vmId) Please run TWO VMs: kill one of them when Vdsm is still up, and verify that its Down state and exist reason is conserved over restart. The second Vm can be killed while Vdsm was stopped (the current code). Line 105: Line 106: deployUtil.setService('vdsmd', 'stop') Line 107: Line 108: os.kill(int(stats['pid']), signal.SIGTERM) Line 102: vmId = vm.start() Line 103: self._waitForStartup(vmId, VM_MINIMAL_UPTIME) Line 104: status, msg, stats = self.vdsm.getVmList(vmId) Line 105: Line 106: deployUtil.setService('vdsmd', 'stop') deployUtil is a piece of untrustworthy cr*p. Avoid it at all costs, and particularly when vdsm.tool.service.service_start() exists. Line 107: Line 108: os.kill(int(stats['pid']), signal.SIGTERM) Line 109: Line 110: deployUtil.setService('vdsmd', 'start') Line 107: Line 108: os.kill(int(stats['pid']), signal.SIGTERM) Line 109: Line 110: deployUtil.setService('vdsmd', 'start') Line 111: time.sleep(5) # looking for a better way to do this while time() - start < 5 and vdsm.tool.service.service_status(): sleep(1) Line 112: Line 113: self.setUp() Line 114: self.assertEqual(self._getVmStatus(vmId)['status'], 'Down') Line 115: -- To view, visit http://gerrit.ovirt.org/25276 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id495f6047ba658c2b04da19bd7bf76425b3b9659 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
