Francesco Romani has posted comments on this change. Change subject: recovery: try to restore VMs from recovery files ......................................................................
Patch Set 6: (7 comments) 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? right, will fix. 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 will amend the code to not use this. 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 Done 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 par I missed this! Thanks, will fix! 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(): Thanks, will fix! Line 112: Line 113: self.setUp() Line 114: self.assertEqual(self._getVmStatus(vmId)['status'], 'Down') Line 115: http://gerrit.ovirt.org/#/c/25276/6/vdsm/clientIF.py File vdsm/clientIF.py: Line 418: Line 419: # we do this to safely handle VMs which disappeared Line 420: # from the host while VDSM was down/restarting Line 421: for vmId in self._getVDSMVmsFromRecovery(): Line 422: if not self._recoverVm(vmId): > see below. Here I'd be a bit more verbose. The case when we get an extra VM Right. Will add a warning() here. Line 423: self.log.info('expected VM %s from recovery file but' Line 424: 'missing, reported as Down', vmId) Line 425: Line 426: while (self._enabled and Line 505: for f in os.listdir(constants.P_VDSM_RUN): Line 506: vmId, fileType = os.path.splitext(f) Line 507: if fileType == ".recovery": Line 508: if vmId in self.vmContainer: Line 509: self.log.info('vm %s already recovered', vmId) > hm, on many VMs this may be a bit too much if we assume most of the times t Yes, this makes sense to me. Will fix. Line 510: else: Line 511: vms.append(vmId) Line 512: return vms Line 513: -- 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
