Dan Kenigsberg has posted comments on this change. Change subject: vm: add extra recovery checking for ppc ......................................................................
Patch Set 2: Code-Review-1 (3 comments) http://gerrit.ovirt.org/#/c/31241/2/tests/clientifTests.py File tests/clientifTests.py: Line 19: # Line 20: Line 21: import logging Line 22: import os.path Line 23: import xml.dom.minidom as MD elsewhere in the code we import only parseString. MD, in all caps, is a thorn in my eye. Line 24: Line 25: from testlib import VdsmTestCase as TestCaseBase Line 26: from testlib import temporaryPath Line 27: from monkeypatch import MonkeyPatch http://gerrit.ovirt.org/#/c/31241/2/vdsm/clientIF.py File vdsm/clientIF.py: Line 489: self.log.error("domId: %s is dead", vm.UUIDString()) Line 490: else: Line 491: raise Line 492: else: Line 493: if _hasVDSMSysinfo(vmdom): Apparently, I was not clear enough. There is no need to maintain duplicate code, one for x86_64 and the other for ppc, where hasVDSMChannels() works for both! There was no special reason for 2c665623b089ab34c7f7f6a5349d8687bc1a88d6 to depend on smbios when implementing isVDSMVm(). We might as well depended on hasVDSMChannels() instead. Line 494: return True Line 495: # else no sysinfo in xml, fallback to less reliable guest agent Line 496: # channel detection (mostly PPC) Line 497: try: Line 497: try: Line 498: if hasVDSMChannels(vmdom): Line 499: return True Line 500: except IndexError: Line 501: # recovery must not fail when is this expected (please add to comment)? any reason not to log details when it does? Line 502: pass Line 503: return False Line 504: Line 505: def _getVDSMVms(self): -- To view, visit http://gerrit.ovirt.org/31241 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic0cc57129e9c8e6545f9a947329adf1f9e82648f Gerrit-PatchSet: 2 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: [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
