Change in vdsm[master]: vm: add extra recovery checking for ppc

danken at redhat.com danken at redhat.com
Fri Aug 8 10:31:35 UTC 2014


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 <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list