Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-03 Thread apahim 
Amador Pahim has uploaded a new change for review.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..

Skip netdev probe from sessions missing iscsi sysfs (i.e. using hardware iSCSI)

In https://gerrit.ovirt.org/#/c/31534/, we introduced the
iface.net_ifacename probe for active sessions. To do so, we had to use
the sessionID (S) to access the iscsi sysfs, looking for
/sys/devices/platform/hostN/sessionS to then use the probed hostN into the
path /sys/class/iscsi_host/hostN/netdev, where netdev contains the value for
iface.net_ifacename, if any.

As we didn't expect /sys/devices/platform/hostN/sessionS to be missing, the
adopted policy was to raise an exception if that happens.

Now we learned the hard way that there is a valid case for missing
/sys/devices/platform/hostN/sessionS, which is using hardware iSCSI (qla4xxx).

This patch is fixing the code to allow missing
/sys/devices/platform/hostN/sessionS.

Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Signed-off-by: Amador Pahim 
---
M vdsm/storage/iscsi.py
1 file changed, 8 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/54/38354/1

diff --git a/vdsm/storage/iscsi.py b/vdsm/storage/iscsi.py
index e7c25a0..fed236d 100644
--- a/vdsm/storage/iscsi.py
+++ b/vdsm/storage/iscsi.py
@@ -105,6 +105,7 @@
 sessionID - the iSCSI session ID.
 Returns:
 - iSCSI host path - e.g. '/sys/class/iscsi_host/host17/'
+- None if host session is not present (i.e. hardware iSCSI)
 """
 
 pattern = '/sys/devices/platform/host*/session%s' % sessionID
@@ -112,13 +113,18 @@
 host = os.path.basename(os.path.dirname(path))
 return '/sys/class/iscsi_host/' + host
 
-raise OSError(errno.ENOENT, "No iscsi_host for session [%s]" % sessionID)
+return None
 
 
 def readSessionInfo(sessionID):
 iscsi_session = getIscsiSessionPath(sessionID)
 iscsi_connection = getIscsiConnectionPath(sessionID)
+
 iscsi_host = getIscsiHostPath(sessionID)
+if iscsi_host is not None:
+netdev = os.path.join(iscsi_host, "netdev")
+else:
+netdev = None
 
 if not os.path.isdir(iscsi_session) or not os.path.isdir(iscsi_connection):
 raise OSError(errno.ENOENT, "No such session")
@@ -133,14 +139,12 @@
 paddr = os.path.join(iscsi_connection, "persistent_address")
 pport = os.path.join(iscsi_connection, "persistent_port")
 
-netdev = os.path.join(iscsi_host, "netdev")
-
 res = []
 for fname in (targetname, iface, tpgt, user, passwd, paddr, pport, netdev):
 try:
 with open(fname, "r") as f:
 res.append(f.read().strip())
-except (OSError, IOError):
+except (OSError, IOError, TypeError):
 res.append("")
 
 iqn, iface, tpgt, username, password, ip, port, netdev = res


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-04 Thread amureini 
Allon Mureinik has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 2: Code-Review+1

(1 comment)

https://gerrit.ovirt.org/#/c/38354/2//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2015-03-03 14:30:12 -0300
Line 6: 
Line 7: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
Line 8: 
Line 9: In https://gerrit.ovirt.org/#/c/31534/, we introduced the
Better to use the commit id (a78793935e09893c66bb42e752c6a7af3a4b4db5) - this 
way the commit message is understable from the command line, without having to 
switch context to open a browser.
Line 10: iface.net_ifacename probe for active sessions. To do so, we had to use
Line 11: the sessionID (S) to access the iscsi sysfs, looking for
Line 12: /sys/devices/platform/hostN/sessionS to then use the probed hostN into 
the
Line 13: path /sys/class/iscsi_host/hostN/netdev, where netdev contains the 
value for


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-04 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 1:

Build Failed 

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

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

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-04 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 2:

Build Failed 

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

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

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-04 Thread mlipchuk 
Maor Lipchuk has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-04 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 3:

Build Failed 

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

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

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-05 Thread apahim 
Amador Pahim has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-05 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 3: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/38354/3/vdsm/storage/iscsi.py
File vdsm/storage/iscsi.py:

Line 143: for fname in (targetname, iface, tpgt, user, passwd, paddr, 
pport, netdev):
Line 144: try:
Line 145: with open(fname, "r") as f:
Line 146: res.append(f.read().strip())
Line 147: except (OSError, IOError, TypeError):
netdev=None is not a surprise here. We should not just swallow all TypeError to 
avoid it.

Please skip opening the filename if it is none to be None.

if fname is None:
  res.append("")
  continue
Line 148: res.append("")
Line 149: 
Line 150: iqn, iface, tpgt, username, password, ip, port, netdev = res
Line 151: port = int(port)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-05 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 2:

(5 comments)

https://gerrit.ovirt.org/#/c/38354/2/vdsm/storage/iscsi.py
File vdsm/storage/iscsi.py:

Line 142
Line 143
Line 144
Line 145
Line 146
This is not related to your change, but using same temporaries for different 
types of data (path, contents of path) in the same function is evil.


Line 104: Arguments:
Line 105: sessionID - the iSCSI session ID.
Line 106: Returns:
Line 107: - iSCSI host path - e.g. '/sys/class/iscsi_host/host17/'
Line 108: - None if host session is not present (i.e. hardware iSCSI)
How do we know that the missing path means "using hardware iscsi", and not 
"session was disconnected"?

Do we have some other sysfs attribute that can tell us that we should not 
expect this path?
Line 109: """
Line 110: 
Line 111: pattern = '/sys/devices/platform/host*/session%s' % sessionID
Line 112: for path in glob.iglob(pattern):


Line 112: for path in glob.iglob(pattern):
Line 113: host = os.path.basename(os.path.dirname(path))
Line 114: return '/sys/class/iscsi_host/' + host
Line 115: 
Line 116: return None
It is more useful to raise here and handle the error in the caller.
Line 117: 
Line 118: 
Line 119: def readSessionInfo(sessionID):
Line 120: iscsi_session = getIscsiSessionPath(sessionID)


Line 123: iscsi_host = getIscsiHostPath(sessionID)
Line 124: if iscsi_host is not None:
Line 125: netdev = os.path.join(iscsi_host, "netdev")
Line 126: else:
Line 127: netdev = None
You can handle the OSError here:

try:
iscsi_host = getIscsiHostPath(sessionID)
netdev = os.path.join(iscsi_host, "netdev")
except OSError as e:
if e.errno != errno.ENOENT:
raise
netdev = None

But the OSError we get is not a real error, we can make this code nicer by 
raising module private error:

try:
build netdev path...
except NoSessionPath:
   netdev = None
Line 128: 
Line 129: if not os.path.isdir(iscsi_session) or not 
os.path.isdir(iscsi_connection):
Line 130: raise OSError(errno.ENOENT, "No such session")
Line 131: 


Line 143: for fname in (targetname, iface, tpgt, user, passwd, paddr, 
pport, netdev):
Line 144: try:
Line 145: with open(fname, "r") as f:
Line 146: res.append(f.read().strip())
Line 147: except (OSError, IOError, TypeError):
This code will silently hide None value for any of the names (targetname, 
iface, ...), while you are trying to handle None netdev.

Even if this could only fail when netdev is None, this is bad idea. You don't 
try to use None (open(fname, ...)) and depend on the fact that it raises 
TypeError; you should check explicitly for None and handle it.

The correct way would be to extract a function for reading path content, so we 
can write:

for path in (values that must exits):
append content at path

if netdev is not None:
append content at path

We probably need a helper to append contents of file at path.
Line 148: res.append("")
Line 149: 
Line 150: iqn, iface, tpgt, username, password, ip, port, netdev = res
Line 151: port = int(port)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-05 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 3: Code-Review-1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-05 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 3:

Please see my comments in version 2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-09 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 4:

Build Failed 

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

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

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_el_gerrit/15671/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-09 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 5:

Build Successful 

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

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

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_el_gerrit/15672/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-11 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 5: Code-Review-1

(5 comments)

https://gerrit.ovirt.org/#/c/38354/5//COMMIT_MSG
Commit Message:

Line 19: Now we learned the hard way that there is a valid case for missing
Line 20: /sys/devices/platform/hostN/sessionS, which is using hardware iSCSI 
(qla4xxx).
Line 21: 
Line 22: This patch is fixing the code to allow missing
Line 23: /sys/devices/platform/hostN/sessionS.
This is wrong. It should fix the code so when using hardware iscsi, we do fail 
when this path does not exists.

There is probably additional check that we can do to verify this this is 
hardware iscsi.

Can you point us to documentation about this?
Line 24: 
Line 25: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1197292
Line 26: Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0


https://gerrit.ovirt.org/#/c/38354/5/vdsm/storage/iscsi.py
File vdsm/storage/iscsi.py:

Line 125: iscsi_connection = getIscsiConnectionPath(sessionID)
Line 126: 
Line 127: try:
Line 128: iscsi_host = getIscsiHostPath(sessionID)
Line 129: netdev = os.path.join(iscsi_host, "netdev")
This line cannot raise NoSessionPath - so you should use else block (see bellow)
Line 130: except NoSessionPath:
Line 131: netdev = None
Line 132: 
Line 133: if not os.path.isdir(iscsi_session) or not 
os.path.isdir(iscsi_connection):


Line 126: 
Line 127: try:
Line 128: iscsi_host = getIscsiHostPath(sessionID)
Line 129: netdev = os.path.join(iscsi_host, "netdev")
Line 130: except NoSessionPath:
You did not address my concern about no host path - how do we know that this is 
caused by hardware iscsi and not some error in our code/system?
Line 131: netdev = None
Line 132: 
Line 133: if not os.path.isdir(iscsi_session) or not 
os.path.isdir(iscsi_connection):
Line 134: raise OSError(errno.ENOENT, "No such session")


Line 127: try:
Line 128: iscsi_host = getIscsiHostPath(sessionID)
Line 129: netdev = os.path.join(iscsi_host, "netdev")
Line 130: except NoSessionPath:
Line 131: netdev = None
else:
netdev = None
Line 132: 
Line 133: if not os.path.isdir(iscsi_session) or not 
os.path.isdir(iscsi_connection):
Line 134: raise OSError(errno.ENOENT, "No such session")
Line 135: 


Line 146: res = []
Line 147: for fname in (targetname, iface, tpgt, user, passwd, paddr, 
pport, netdev):
Line 148: if fname is None:
Line 149: res.append("")
Line 150: continue
Are we ok with None value in any other value than netdev?

Since this patch is fixing the issue with netdev, we should handle only None 
netdev here.
Line 151: try:
Line 152: with open(fname, "r") as f:
Line 153: res.append(f.read().strip())
Line 154: except (OSError, IOError):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-15 Thread automation 
automat...@ovirt.org has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 1:

* 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/38354
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-15 Thread automation 
automat...@ovirt.org has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 2:

* Update tracker::#1197292::OK
* Check Bug-Url::OK
* Check Public Bug::#1197292::OK, public bug
* Check Product::#1197292::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 
ovirt-3.2)
* 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/38354
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-15 Thread automation 
automat...@ovirt.org has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 3:

* Update tracker::#1197292::OK
* Check Bug-Url::OK
* Check Public Bug::#1197292::OK, public bug
* Check Product::#1197292::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 
ovirt-3.2)
* 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/38354
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-15 Thread automation 
automat...@ovirt.org has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 4:

* Update tracker::#1197292::OK
* Check Bug-Url::OK
* Check Public Bug::#1197292::OK, public bug
* Check Product::#1197292::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 
ovirt-3.2)
* 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/38354
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-15 Thread automation 
automat...@ovirt.org has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 5:

* Update tracker::#1197292::OK
* Check Bug-Url::OK
* Check Public Bug::#1197292::OK, public bug
* Check Product::#1197292::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 
ovirt-3.2)
* 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/38354
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-30 Thread apahim 
Amador Pahim has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 5:

Sorry for the slowness. I don't have a reproducer here and QA hardware is 
mostly not available. If you have a hardware for this, please let me know.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-03-30 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/38354/5/vdsm/storage/iscsi.py
File vdsm/storage/iscsi.py:

Line 126: 
Line 127: try:
Line 128: iscsi_host = getIscsiHostPath(sessionID)
Line 129: netdev = os.path.join(iscsi_host, "netdev")
Line 130: except NoSessionPath:
> You did not address my concern about no host path - how do we know that thi
Not having session path seems to be good check, but we need to look at some 
machines with hardware iscsi to be sure.
Line 131: netdev = None
Line 132: 
Line 133: if not os.path.isdir(iscsi_session) or not 
os.path.isdir(iscsi_connection):
Line 134: raise OSError(errno.ENOENT, "No such session")


Line 127: try:
Line 128: iscsi_host = getIscsiHostPath(sessionID)
Line 129: netdev = os.path.join(iscsi_host, "netdev")
Line 130: except NoSessionPath:
Line 131: netdev = None
> else:
I think I meant:

else:
netdev = os.path.join(iscsi_host, "netdev")
Line 132: 
Line 133: if not os.path.isdir(iscsi_session) or not 
os.path.isdir(iscsi_connection):
Line 134: raise OSError(errno.ENOENT, "No such session")
Line 135: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-04-02 Thread automation 
automat...@ovirt.org has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 6:

* 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/38354
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Amit Aviram 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-04-02 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 6:

Build Started (2/2) -> 
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17623/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Amit Aviram 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-04-02 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 6:

Build Started (1/2) -> 
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17449/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Amit Aviram 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-04-02 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 6:

Build Successful 

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

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Amit Aviram 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-04-02 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 6:

(5 comments)

https://gerrit.ovirt.org/#/c/38354/6/vdsm/storage/iscsi.py
File vdsm/storage/iscsi.py:

Line 108: """
Line 109: path = os.path.realpath(getIscsiSessionPath(sessionID))
Line 110: if not os.path.exists(path):
Line 111: raise OSError(errno.ENOENT, "No such session %r" %
Line 112:   sessionID)
This should fit on one line.
Line 113: 
Line 114: # As the realpath of the iscsi session is:
Line 115: # /sys/devices/pci.../.../host*/session*/iscsi_session/session*
Line 116: # for hardware iSCSI, or:


Line 115: # /sys/devices/pci.../.../host*/session*/iscsi_session/session*
Line 116: # for hardware iSCSI, or:
Line 117: # /sys/devices/platform/host*/session*/iscsi_session/session*
Line 118: # for iSCSI, We are retreiving the host's name from the 4th from
Line 119: # last element in the path.
The text is little bit confusing - lets replace with something like this:

# Session path depends on the iSCSI implementation:
# Hardware iSCSI:
#   /sys/devices/pci*/*/*/host0/session7/iscsi_session/session7
# Software iSCSI:
#   /sys/devices/platform/host5/session8/iscsi_session/session8

In both case we retrive the host from the same location (4th element from 
right).

I suggest before to use ... to shorten the path, but this may confuse people 
since .. is the shortcut for parrent directory, so lets use *.
Line 120: host = path.rsplit(os.sep, 4)[-4]
Line 121: 
Line 122: if not host.startswith('host'):
Line 123: raise RuntimeError('Unexpected iSCSI path analyzed %s' % path)


Line 117: # /sys/devices/platform/host*/session*/iscsi_session/session*
Line 118: # for iSCSI, We are retreiving the host's name from the 4th from
Line 119: # last element in the path.
Line 120: host = path.rsplit(os.sep, 4)[-4]
Line 121: 
Lets remove the empty line.
Line 122: if not host.startswith('host'):
Line 123: raise RuntimeError('Unexpected iSCSI path analyzed %s' % path)
Line 124: return '/sys/class/iscsi_host/' + host
Line 125: 


Line 119: # last element in the path.
Line 120: host = path.rsplit(os.sep, 4)[-4]
Line 121: 
Line 122: if not host.startswith('host'):
Line 123: raise RuntimeError('Unexpected iSCSI path analyzed %s' % path)
I don't know what is iSCSI path. Also "analyzed" is not adding much value for 
the reader of the log.

We like to see the exact string that does not match. Using %r will show it like 
this: '/sys/platform...' instead of /sys/platform.

Use:

"Unexpected session path %r"

Finally lets add  empty line bellow this block.
Line 124: return '/sys/class/iscsi_host/' + host
Line 125: 
Line 126: 
Line 127: def readSessionInfo(sessionID):


Line 140: 
Line 141: paddr = os.path.join(iscsi_connection, "persistent_address")
Line 142: pport = os.path.join(iscsi_connection, "persistent_port")
Line 143: 
Line 144: # iscsi host is available only when the session exists.
iscsi host -> iscsi_host
Line 145: iscsi_host = getIscsiHostPath(sessionID)
Line 146: netdev = os.path.join(iscsi_host, "netdev")
Line 147: 
Line 148: res = []


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Amit Aviram 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-04-02 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/38354/6//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2015-03-03 14:10:00 -0300
Line 4: Commit: Amit Aviram 
Line 5: CommitDate: 2015-04-02 17:49:19 +0300
Line 6: 
Line 7: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
This patch does not do this now; please describe what this patch does in the 
current version.
Line 8: 
Line 9: In https://gerrit.ovirt.org/#/c/31534/, we introduced the
Line 10: iface.net_ifacename probe for active sessions. To do so, we had to use
Line 11: the sessionID (S) to access the iscsi sysfs, looking for


Line 19: Now we learned the hard way that there is a valid case for missing
Line 20: /sys/devices/platform/hostN/sessionS, which is using hardware iSCSI 
(qla4xxx).
Line 21: 
Line 22: This patch is fixing the code to allow missing
Line 23: /sys/devices/platform/hostN/sessionS.
Lets replace with something like this:

In commit a78793935e (Configure iSCSI iface.net_ifacename), we introduced
the iface.net_ifacename probe for active sessions. We use the sessionID to
locate /sys/class/iscs_host/host* and read the netdev attribue.

However, we looked up the session in /sys/devices/platform/host*
which is correct only for software iSCSI. For hardware iSCSI, the session is
located in /sys/devices/pci*/*/*/*/host*.

This patch locates the host using the real iscsi_session path, which works 
for
both software and hardware iSCSI.
Line 24: 
Line 25: Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Amit Aviram 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Skip netdev probe from sessions missing iscsi sysfs (i.e. us...

2015-04-04 Thread amureini 
Allon Mureinik has posted comments on this change.

Change subject: Skip netdev probe from sessions missing iscsi sysfs (i.e. using 
hardware iSCSI)
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/38354/6//COMMIT_MSG
Commit Message:

Line 21: 
Line 22: This patch is fixing the code to allow missing
Line 23: /sys/devices/platform/hostN/sessionS.
Line 24: 
Line 25: Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Amit - please add yourself as a signed-off


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f273fddff2235f15197c6689de5e6374fda6f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Amit Aviram 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches