Dan Kenigsberg has posted comments on this change. Change subject: vm: detect migration completed on recovery ......................................................................
Patch Set 9: Code-Review-1 (4 comments) minor questions http://gerrit.ovirt.org/#/c/28511/9//COMMIT_MSG Commit Message: Line 17: Line 18: To handle this situation, we Line 19: - connect early to the domain, on recovery. We know Line 20: this is safe because on recovery we iterate on the very Line 21: domain list libvirt provided to us moments before, but in what way this is "safe"? it might be only a problematic choice of words. I think that you mean is that - now, we wait for VMs from a list produced by the former vdsm process, which may have died hour ago. - with this patch, the list of VMs is much more fresh, limiting the danger of needless wait. am I right? Line 22: so the domain will be present Line 23: - inspect the domain state *before* waitingr migration Line 24: termination, and skip the wait if the domain is detected Line 25: running. Line 19: - connect early to the domain, on recovery. We know Line 20: this is safe because on recovery we iterate on the very Line 21: domain list libvirt provided to us moments before, Line 22: so the domain will be present Line 23: - inspect the domain state *before* waitingr migration waitingr -> waiting for Line 24: termination, and skip the wait if the domain is detected Line 25: running. Line 26: Line 27: A nice side-effect, this patch also tries to clarify a http://gerrit.ovirt.org/#/c/28511/9/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3010: if self.recovering: Line 3011: self._dom = NotifyingVirDomain( Line 3012: self._connection.lookupByUUIDString(self.id), Line 3013: self._timeoutExperienced) Line 3014: elif 'migrationDest' in self.conf: that's simply "not initDomain", which I find clearer. Line 3015: pass # self._dom will be None until migration ends. Line 3016: elif 'restoreState' in self.conf: Line 3017: fromSnapshot = self.conf.get('restoreFromSnapshot', False) Line 3018: srcDomXML = self.conf.pop('_srcDomXML') Line 3587: try: Line 3588: if self._isDomainRunning(): Line 3589: waitMigration = False Line 3590: self.log.info('migration completed while recovering!') Line 3591: except (libvirt.libvirtError, AttributeError): what is this evil AttributeError? due to self._dom being set to None asynchronously? Line 3592: self.log.exception('migration failed while recovering!') Line 3593: self.setDownStatus(ERROR, vmexitreason.MIGRATION_FAILED) Line 3594: return Line 3595: if waitMigration: -- To view, visit http://gerrit.ovirt.org/28511 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I451c2a940693842e9bf7c63ccc117e75026bb11b Gerrit-PatchSet: 9 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
