Michal Skrivanek has posted comments on this change. Change subject: recovery: try to restore VMs from recovery files ......................................................................
Patch Set 6: (2 comments) 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 from recovery not recovered already and succeed recoverVm() is actually a reason for alert as well. "It should never happen". Maybe add a verbose log to isVDSMVm in this case…there should not be any/many nonVDSM VMs….and if they are we do want to know about them 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 they will recover correctly. I wouldn't mind being silent about the happy path 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
