Nir Soffer has posted comments on this change.

Change subject: cpuinfo: add predicates for x86 and ppc platform detection
......................................................................


Patch Set 1:

(3 comments)

Nice, but the translation from old "in POWER" to new "isppc()" is not always 
correct.

https://gerrit.ovirt.org/#/c/50894/1/vdsm/caps.py
File vdsm/caps.py:

Line 97:     def is_ppc(cls, arch):
Line 98:         return arch in cls.POWER
Line 99: 
Line 100:     @classmethod
Line 101:     def is_x86(cls, arch):
Do we supprot x86 and x86_64? seems that we support only x86_64, so better call 
this predicate is_x86_64()
Line 102:         return arch == cls.X86_64
Line 103: 
Line 104: 
Line 105: class CpuInfo(object):


Line 416:     if Architecture.is_x86(arch):
Line 417:         arch = 'x86'
Line 418: 
Line 419:     # Same goes for ppc64le
Line 420:     if Architecture.is_ppc(arch):
This will match now both PPC64 and PPC64LE - is this the intent? If it is, 
please fix the comment about "Same goes for ppc64le".
Line 421:         arch = 'ppc64'
Line 422: 
Line 423:     architectureElement = None
Line 424: 


https://gerrit.ovirt.org/#/c/50894/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 1668:         
domxml._appendAgentDevice(self._qemuguestSocketFile.decode('utf-8'),
Line 1669:                                   _QEMU_GA_DEVICE_NAME)
Line 1670:         domxml.appendInput()
Line 1671: 
Line 1672:         if caps.Architecture.is_ppc(self.arch):
Previously this matched only PPC64, not it will match also PPC64LE - is this 
the intent?
Line 1673:             domxml.appendEmulator()
Line 1674: 
Line 1675:         self._appendDevices(domxml)
Line 1676: 


-- 
To view, visit https://gerrit.ovirt.org/50894
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0981367436ddfbfabfd3484019b96831e31f9609
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpoled...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to