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

[Martin Polednik]

Martin Polednik has uploaded a new change for review.

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

vdsm: add check for hostdev passthrough to caps

Currently only for intel + will not be 100% accurate source

Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Signed-off-by: Martin Polednik <mpoledni(a)redhat.com&gt;
---
M vdsm/caps.py
1 file changed, 27 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/30471/1

diff --git a/vdsm/caps.py b/vdsm/caps.py
index 0993939..2b0aa21 100644
--- a/vdsm/caps.py
+++ b/vdsm/caps.py
@@ -246,6 +246,32 @@
     return None
 
 
+def _iommuPresent(cmdline):
+    if 'intel_iommu' in cmdline:
+        if 'intel_iommu=off' not in cmdline:
+            return 'yes'
+
+    return 'no'
+
+
+def _getIommuSupport():
+    try:
+        import systemd.journal
+        reader = systemd.journal.Reader()
+        reader.this_boot()
+        reader.add_match(_TRANSPORT='kernel')
+        for index, entry in enumerate(reader):
+            if 'Command line:' in entry['MESSAGE']:
+                return _iommuPresent(entry['MESSAGE'])
+    except:
+        with open('/var/log/dmesg', 'r') as fd:
+            for index, line in enumerate(fd):
+                if 'Command line:' in line:
+                    return _iommuPresent(line)
+
+    return 'no'
+
+
 @utils.memoized
 def getLiveMergeSupport():
     """
@@ -642,6 +668,7 @@
     caps['autoNumaBalancing'] = getAutoNumaBalancingInfo()
 
     caps['selinux'] = _getSELinux()
+    caps['hostdevSupport'] = _getIommuSupport()
 
     liveSnapSupported = _getLiveSnapshotSupport(targetArch)
     if liveSnapSupported is not None:


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 1:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10342/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11127/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11284/ : FAILURE

-- 
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 2:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10403/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11188/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11345/ : FAILURE

-- 
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 <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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 <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Martin Polednik]

Martin Polednik has posted comments on this change.

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


Patch Set 2:

(2 comments)

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 
thanks for the suggestion, actually makes sense;

about checking for enabled - iommu can be anbled via =on but also =igfx_off for example,
therefore as long as its there and not off we can assume that it's configured to work
Line 255: 
Line 256: 
Line 257: def _getIommuSupport():
Line 258:     try:


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 nee 
great suggestion, was looking for something like that
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 <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Martin Polednik]

Martin Polednik has posted comments on this change.

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


Patch Set 3: Verified+1

-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 3:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11120/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12062/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11909/ : SUCCESS

-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 4:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11183/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12125/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11972/ : SUCCESS

-- 
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(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Martin Polednik]

Martin Polednik has posted comments on this change.

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


Patch Set 4:

agreed with comments, but if you look in the history this split was done in order to allow
for testing the logic as I couldn't figure out how to do the testing of top level
itself (open + readline calls)

-- 
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(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 5:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11280/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12222/ : ABORTED

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12069/ : SUCCESS

-- 
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

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


Patch Set 5: Code-Review-1

(4 comments)

http://gerrit.ovirt.org/#/c/30471/5//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2014-07-19 01:31:13 +0200
Line 4: Commit:     Martin Polednik <mpolednik(a)redhat.com&gt;
Line 5: CommitDate: 2014-09-03 14:33:20 +0200
Line 6: 
Line 7: vdsm: add check for hostdev passthrough to caps
vdsm prefix is superfluous.
Line 8: 
Line 9: Not perfectly accurate source but the support for passthrough can be
Line 10: obtained through cmdline of dmesg, or systemd journal in fedora.
Line 11: This indicates whether user *wanted* to enable the support, not if


Line 8: 
Line 9: Not perfectly accurate source but the support for passthrough can be
Line 10: obtained through cmdline of dmesg, or systemd journal in fedora.
Line 11: This indicates whether user *wanted* to enable the support, not if
Line 12: it is fully supported by hardware and/or configuration.
isn't there a saner way, by reading sysfs?
Line 13: 
Line 14: Change-Id: I789f93679740e87b2f5a88351261bc58852990d4


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

Line 254:     return False
Line 255: 
Line 256: 
Line 257: def _has_iommu_support():
Line 258:     with open('/proc/cmdline', 'r') as fd:
the return value of open() is a file, not a file descriptor. So 'fd' is a
misleading name.
Line 259:         cmdline = fd.readline()
Line 260:         if _has_iommu_support_in_cmdline(cmdline):
Line 261:             return 'yes'
Line 262:         else:


Line 257: def _has_iommu_support():
Line 258:     with open('/proc/cmdline', 'r') as fd:
Line 259:         cmdline = fd.readline()
Line 260:         if _has_iommu_support_in_cmdline(cmdline):
Line 261:             return 'yes'
better return plane booleans
Line 262:         else:
Line 263:             return 'no'
Line 264: 
Line 265: 


-- 
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 6:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12923/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12765/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11974/ : FAILURE

-- 
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

Francesco Romani has posted comments on this change.

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


Patch Set 6: Code-Review+1

(1 comment)

Comments from Dan and from me are been addressed. Thanks!

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

Line 303: 
Line 304: def _has_iommu_support():
Line 305:     with open('/proc/cmdline', 'r') as f:
Line 306:         cmdline = f.readline()
Line 307:         return _has_iommu_support_in_cmdline(cmdline)
we're gonna need an utility function for this task, but this is definitely out of
scope of this patch.
Line 308: 
Line 309: 
Line 310: @utils.memoized
Line 311: 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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

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

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

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


Patch Set 6:

(2 comments)

http://gerrit.ovirt.org/#/c/30471/6//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: Not perfectly accurate source but the support for passthrough can be
Line 10: obtained through cmdline. This indicates whether user *wanted* to
Line 11: enable the support, not if it is fully supported by hardware and/or
Line 12: configuration.
Sorry for not asking this before, but why do we want to know that? What would happen if
iommu is disabled in bios? Would attachments fail graciously?

Why would an ovirt user want to explicitly disable iommu?
Line 13: 
Line 14: Change-Id: I789f93679740e87b2f5a88351261bc58852990d4


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

Line 294: 
Line 295: 
Line 296: def _has_iommu_support_in_cmdline(cmdline):
Line 297:     if 'iommu' in cmdline and ('intel_iommu=off' not in cmdline
and
Line 298:                                'iommu=off' not in cmdline):
this would return True if cmdline has the string "biommu=officer". That's
not likely, but if we HAVE to parse the command line, let us do it properly (use
shlex.split()).
Line 299:         return True
Line 300: 
Line 301:     return False
Line 302: 


-- 
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

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

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

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


Patch Set 6: Code-Review-1

-- 
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

automation(a)ovirt.org has posted comments on this change.

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


Patch Set 7:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

[Martin Polednik]

Martin Polednik has posted comments on this change.

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


Patch Set 7: Verified+1

-- 
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 7:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/15583/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14781/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15752/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created_staging/559/ : FAILURE

-- 
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

Francesco Romani has posted comments on this change.

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


Patch Set 7:

(1 comment)

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

Line 691:     caps['numaNodeDistance'] = getNumaNodeDistance()
Line 692:     caps['autoNumaBalancing'] = getAutoNumaBalancingInfo()
Line 693: 
Line 694:     caps['selinux'] = _getSELinux()
Line 695:     caps['iommuSupport'] = str(_has_iommu_support())
str(_has_iommu_support()).lower().

Probably Engine doesn't care, but let's be compliant.
Line 696: 
Line 697:     liveSnapSupported = _getLiveSnapshotSupport(targetArch)
Line 698:     if liveSnapSupported is not None:
Line 699:         caps['liveSnapshot'] = str(liveSnapSupported).lower()


-- 
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

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

Francesco Romani has posted comments on this change.

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


Patch Set 7:

one last step and that should be it.

-- 
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

automation(a)ovirt.org has posted comments on this change.

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


Patch Set 8:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

[Martin Polednik]

Martin Polednik has posted comments on this change.

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


Patch Set 8: Verified+1

Manually verified via cmdline and getCaps.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

Francesco Romani has posted comments on this change.

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


Patch Set 8: Code-Review+1

addressed comments both from Dan and me, and looks good to me

(though for a bit about other possible /proc/cmdline misdetection, and can't find
something _realistic_, so let's go ahead)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 8:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16107/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/15307/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/16277/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created_staging/1083/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

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


Patch Set 8: Code-Review-1

(1 comment)

setting -1 for visibility; the request is only for a commit message change.

https://gerrit.ovirt.org/#/c/30471/8//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2015-02-26 16:10:35 +0100
Line 6: 
Line 7: hostdev: add check for hostdev passthrough to caps
Line 8: 
Line 9: Detecting whether the user has enabled iommu can be done through
Could you give me (in the commit message) a little more background about this?

* what is Fedora and EL7's default for this?
* is there not a safer way to check this on a running kernel (kernel command line can
change)
* what is the effect of unsupported iommu?
Line 10: checking kernel cmdline for occurence of iommu (AMD) or intel_iommu
Line 11: (well, Intel). This patch exposes this fact in caps, making it
Line 12: possible for engine to warn user about possible inconsistence
Line 13: or impossibility of using hostdev passthrough.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

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

[Martin Polednik]

Martin Polednik has posted comments on this change.

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


Patch Set 8:

(1 comment)

https://gerrit.ovirt.org/#/c/30471/8//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2015-02-26 16:10:35 +0100
Line 6: 
Line 7: hostdev: add check for hostdev passthrough to caps
Line 8: 
Line 9: Detecting whether the user has enabled iommu can be done through
>  Could you give me (in the commit message) a little more background
> about th 
* EL7 ships with iommu not compiled in kernel (therefore requiring this)
* not as far as I know
* no hostdev support
Line 10: checking kernel cmdline for occurence of iommu (AMD) or intel_iommu
Line 11: (well, Intel). This patch exposes this fact in caps, making it
Line 12: possible for engine to warn user about possible inconsistence
Line 13: or impossibility of using hostdev passthrough.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

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

[Martin Polednik]

Martin Polednik has abandoned this change.

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


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server

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

automation(a)ovirt.org has posted comments on this change.

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


Patch Set 8:

* Update tracker::IGNORE, no Bug-Url found

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I789f93679740e87b2f5a88351261bc58852990d4
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com&gt;
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No