Francesco Romani has posted comments on this change.

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


Patch Set 2:

(3 comments)

I find strange that libvirt or even somewhere on /sys or /proc
does not signal the availability of IOMMU

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

Line 250:     if 'intel_iommu' in cmdline:
Line 251:         if 'intel_iommu=off' not in cmdline or 'iommu=off' not in 
cmdline:
Line 252:             return 'yes'
Line 253: 
Line 254:     return 'no'
why not just

return 'no' if 'iommu=off' in cmdline else 'yes'

IIUC utils.tobool('iommu=off' not in cmdline) could work as well, need to check

However, I'm not sure about this negative check, because I read that sometimes 
IOMMU need to be explicitely *enabled*.
Line 255: 
Line 256: 
Line 257: def _getIommuSupport():
Line 258:     try:


Line 255: 
Line 256: 
Line 257: def _getIommuSupport():
Line 258:     try:
Line 259:         import systemd.journal
Not sure about this. I'd rather go read /proc/cmdline , which is guaranteed to 
be here.
Line 260:         reader = systemd.journal.Reader()
Line 261:         reader.this_boot()
Line 262:         reader.add_match(_TRANSPORT='kernel')
Line 263:         for index, entry in enumerate(reader):


Line 266:     except:
Line 267:         with open('/var/log/dmesg', 'r') as fd:
Line 268:             for index, line in enumerate(fd):
Line 269:                 if 'Command line:' in line:
Line 270:                     return _iommuPresent(line)
same here, probably /proc/cmdline is simpler and provides just what you need.
Line 271: 
Line 272:     return 'no'
Line 273: 
Line 274: 


-- 
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <[email protected]>
Gerrit-Reviewer: Francesco Romani <[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

Reply via email to