Change in vdsm[master]: vdsm: add check for hostdev passthrough to caps

fromani at redhat.com fromani at redhat.com
Fri Aug 29 09:19:53 UTC 2014


Francesco Romani has posted comments on this change.

Change subject: vdsm: add check for hostdev passthrough to caps
......................................................................


Patch Set 4: Code-Review-1

(2 comments)

a few minor comments inside.

Please try to test the top-level function (_getIommuSupport) instead of the private helper.

-1 for visibility.

http://gerrit.ovirt.org/#/c/30471/4/vdsm/caps.py
File vdsm/caps.py:

Line 250:     if 'iommu' in cmdline and ('intel_iommu=off' not in cmdline and
Line 251:                                'iommu=off' not in cmdline):
Line 252:         return 'yes'
Line 253: 
Line 254:     return 'no'
What about returning a plain boolean and doing the needed conversion down on line 659?

I think that would be (slightly) better.
Line 255: 
Line 256: 
Line 257: def _getIommuSupport():
Line 258:     with open('/proc/cmdline', 'r') as fd:


Line 256: 
Line 257: def _getIommuSupport():
Line 258:     with open('/proc/cmdline', 'r') as fd:
Line 259:         cmdline = fd.readline()
Line 260:         _checkCmdlineIommuSupport(cmdline)
you miss a 'return' here.
Line 261: 
Line 262: 
Line 263: @utils.memoized
Line 264: def getLiveMergeSupport():


-- 
To view, visit http://gerrit.ovirt.org/30471
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik 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