Change in vdsm[master]: [RFC] storage: Configure multipath using local interfaces

Amador Pahim has uploaded a new change for review.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

[RFC] storage: Configure multipath using local interfaces

Currently is not possible to achieve multipath using one target IP
and two or more local interfaces. iscsiadm is always executed with
default interface.
This patch takes advantage of pre-existent irs iscsi_default_ifaces
configuration key to inform to vdsm the interfaces to be used creating
paths to the target.
This is the initial patch, hoping for comments and concerns.
For now, let me keep some links to explain what is the goal:

- iscsi multipath mannual configuration:
https://gist.github.com/apahim/7434734

- current vdsm implementation:
https://gist.github.com/apahim/7434764

- vdsm results after this patch:
https://gist.github.com/apahim/7434836

Change-Id: I488c0a1401606ac40a5ee4ef3d816f53b1d22785
Bug-Url: http://bugzilla.redhat.com/753541
Signed-off-by: Amador Pahim 
---
M vdsm/storage/hsm.py
M vdsm/storage/iscsiadm.py
2 files changed, 54 insertions(+), 41 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/21189/1

diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 15fd9fa..13ff51f 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -147,7 +147,11 @@

def _BCInitiatorNameResolve(ifaceName):
     if not ifaceName:
-        return iscsi.IscsiInterface('default')
+        ifaces = config.get('irs', 'iscsi_default_ifaces').split(',')
+        if ifaces and 'iser' not in ifaces:
+            return iscsi.IscsiInterface(ifaces)
+        else:
+            return iscsi.IscsiInterface(['default'])

for iface in iscsi.iterateIscsiInterfaces():
         if iface.name == ifaceName:
diff --git a/vdsm/storage/iscsiadm.py b/vdsm/storage/iscsiadm.py
index 9bb27ea..30b811f 100644
--- a/vdsm/storage/iscsiadm.py
+++ b/vdsm/storage/iscsiadm.py
@@ -229,70 +229,79 @@

def node_new(iface, portal, targetName):
-    rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", iface, "-p",
-                            portal, "--op=new"])
-    if rc == 0:
-        return
+    for i in iface:
+        if not iface_exists(i):
+            iface_new(i)

-    if not iface_exists(iface):
-        raise IscsiInterfaceDoesNotExistError(iface)
+        rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", i, "-p",
+                                portal+",1", "--op=new"])

-    raise IscsiNodeError(rc, out, err)
+        if rc != 0:
+            raise IscsiNodeError(rc, out, err)
+
+    return

def node_update(iface, portal, targetName, key, value, hideValue=False):
-    rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", iface, "-p",
-                            portal, "-n", key, "-v", value, "--op=update"],
-                           hideValue)
-    if rc == 0:
-        return
+    for i in iface:
+        if not iface_exists(i):
+            iface_new(i)

-    if not iface_exists(iface):
-        raise IscsiInterfaceDoesNotExistError(iface)
+        rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", i, "-p",
+                               portal+",1", "-n", key, "-v",
+                               value, "--op=update"], hideValue)

-    raise IscsiNodeError(rc, out, err)
+        if rc != 0:
+            raise IscsiNodeError(rc, out, err)
+
+    return

def node_delete(iface, portal, targetName):
-    rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", iface, "-p",
-                            portal, "--op=delete"])
-    if rc == 0:
-        return
+    for i in iface:
+        if iface_exists(i):
+            iface_delete(i)

-    if not iface_exists(iface):
-        raise IscsiInterfaceDoesNotExistError(iface)
+        rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", i, "-p",
+                               portal+",1", "--op=delete"])
+        if rc != 0:
+            raise IscsiNodeError(rc, out, err)

-    raise IscsiNodeError(rc, out, err)
+    return

def node_disconnect(iface, portal, targetName):
-    rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", iface, "-p",
-                            portal, "-u"])
-    if rc == 0:
-        return
+    for i in iface:
+        if not iface_exists(i):
+            raise IscsiInterfaceDoesNotExistError(i)

-    if not iface_exists(iface):
-        raise IscsiInterfaceDoesNotExistError(iface)
+        rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I",
+                               iface, "-p", portal+",1", "-u"])

-    if rc == ISCSI_ERR_OBJECT_NOT_FOUND:
-        raise IscsiSessionNotFound(iface, portal, targetName)
+        if rc == ISCSI_ERR_OBJECT_NOT_FOUND:
+            raise IscsiSessionNotFound(iface, portal, targetName)

-    raise IscsiNodeError(rc, out, err)
+        if rc != 0:
+            raise IscsiNodeError(rc, out, err)
+
+    return

def node_login(iface, portal, targetName):
-    rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", iface, "-p",
-                            portal, "-l"])
-    if rc == 0:
-        return
+    for i in iface:
+        if not iface_exists(i):
+            raise IscsiInterfaceDoesNotExistError(i)

-    if not iface_exists(iface):
-        raise IscsiInterfaceDoesNotExistError(iface)
+        rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", i, "-p",
+                               portal+",1", "-l"])

-    if rc == ISCSI_ERR_LOGIN_AUTH_FAILED:
-        raise IscsiAuthenticationError(rc, out, err)
+        if rc == ISCSI_ERR_LOGIN_AUTH_FAILED:
+            raise IscsiAuthenticationError(rc, out, err)

-    raise IscsiNodeError(rc, out, err)
+        if rc != 0:
+            raise IscsiNodeError(rc, out, err)
+
+    return

def session_rescan_async():

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4576/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5376/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5455/ : SUCCESS

Sergey Gotliv has posted comments on this change.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

Patch Set 1: Code-Review-1

(2 comments)

Did you try to use VDSM that way:

# vdsClient 0 connectStorageServer 3 98da1408-948d-4cab-9a8b-418914be9f07 \
  connection=vm-rhsrv1.in1.bytenix.com,iqn=iqn.1994-05.com.redhat:ovirt1,portal=1,id=6cec6a56-8e1b-41bd-ac9d-c3866124389a,port=3260,initiatorName=ovirtmgmt

where initiatorName is the iface you want to use?

I am not convinced at this moment that we need to change something in VDSM in order to achieve the goal described in that RFE.

Let's dusscuss.

....................................................
File vdsm/storage/hsm.py
Line 149:     if not ifaceName:
Line 150:         ifaces = config.get('irs', 'iscsi_default_ifaces').split(',')
Line 151:         if ifaces and 'iser' not in ifaces:
Line 152:             return iscsi.IscsiInterface(ifaces)
Line 153:         else:
This code is probably not working, right?
Line 154:             return iscsi.IscsiInterface(['default'])
Line 155: 
Line 156:     for iface in iscsi.iterateIscsiInterfaces():
Line 157:         if iface.name == ifaceName:

....................................................
File vdsm/storage/iscsiadm.py
Line 228:     raise IscsiDiscoverdbError(rc, out, err)
Line 229: 
Line 230: 
Line 231: def node_new(iface, portal, targetName):
Line 232:     for i in iface:
This method should receive exactly one interface, if you want to call it with multiple interfaces, please, call this method in the loop in the caller method.

Same is true for all changes in that module.
Line 233:         if not iface_exists(i):
Line 234:             iface_new(i)
Line 235: 
Line 236:         rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", i, "-p",

Sergey Gotliv has posted comments on this change.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

Patch Set 1:

Just to correct my previous comment,

initiatorName is the network interface.

Amador Pahim has posted comments on this change.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

Patch Set 1:

(2 comments)

....................................................
File vdsm/storage/hsm.py
Line 149:     if not ifaceName:
Line 150:         ifaces = config.get('irs', 'iscsi_default_ifaces').split(',')
Line 151:         if ifaces and 'iser' not in ifaces:
Line 152:             return iscsi.IscsiInterface(ifaces)
Line 153:         else:
It's working.
Line 154:             return iscsi.IscsiInterface(['default'])
Line 155: 
Line 156:     for iface in iscsi.iterateIscsiInterfaces():
Line 157:         if iface.name == ifaceName:

....................................................
File vdsm/storage/iscsiadm.py
Line 228:     raise IscsiDiscoverdbError(rc, out, err)
Line 229: 
Line 230: 
Line 231: def node_new(iface, portal, targetName):
Line 232:     for i in iface:
Done
Line 233:         if not iface_exists(i):
Line 234:             iface_new(i)
Line 235: 
Line 236:         rc, out, err = _runCmd(["-m", "node", "-T", targetName, "-I", i, "-p",

Amador Pahim has posted comments on this change.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

Patch Set 2: Code-Review-1

Keeping -1 while we discuss whole solution.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

Patch Set 2: Code-Review-1 Verified-1

Build Failed

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4608/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5408/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5487/ : FAILURE

Amador Pahim has posted comments on this change.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

Patch Set 3: Code-Review-1

Keeping -1 while we discuss the whole solution.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

Patch Set 3:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4609/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5409/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5488/ : SUCCESS

Saggi Mizrahi has posted comments on this change.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

Patch Set 3:

One of SaggiMizrahi's automated scripts discovered this patch might require his approval.
Please wait until he had time to check it out.

Amador Pahim has abandoned this change.

Change subject: [RFC] storage: Configure multipath using local interfaces
......................................................................

Abandoned