Change in vdsm[master]: storage: Extend the API for IPv6 iSCSI targets

Dominik Holler has uploaded a new change for review.

Change subject: storage: Extend the API for IPv6 iSCSI targets
......................................................................

storage: Extend the API for IPv6 iSCSI targets

A new parameter is introduced in the API, to explicitly indicate the
capability of the client to handle IPv6 iSCSI targets. This parameter
is used to present IPv6 iSCSI targets only to clients, which has this
capability.

Change-Id: Iecb45e326d9c551fb636f309f9e945023143dd9c
Bug-Url: https://bugzilla.redhat.com/1308306
Signed-off-by: Dominik Holler <dholler(a)redhat.com&gt;
---
M lib/api/vdsm-api.yml
M lib/vdsm/rpc/bindingxmlrpc.py
M vdsm/API.py
M vdsm/storage/hsm.py
4 files changed, 18 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/94/65794/1

diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml
index f311b17..d0e5a20 100644
--- a/lib/api/vdsm-api.yml
+++ b/lib/api/vdsm-api.yml
@@ -7713,6 +7713,11 @@
         description: The password associated with the given username
         name: password
         type: string
+
+    -   defaultvalue: false
+        description: If true, the returned list includes porals on IPv6
+        name: ipv6Targets
+        type: boolean
     return:
         description: 'A list of discovered targets in the form:
             ''<host>:<port>,<tpgt><iqn>'''
diff --git a/lib/vdsm/rpc/bindingxmlrpc.py b/lib/vdsm/rpc/bindingxmlrpc.py
index b8bb3f6..f747304 100644
--- a/lib/vdsm/rpc/bindingxmlrpc.py
+++ b/lib/vdsm/rpc/bindingxmlrpc.py
@@ -984,8 +984,10 @@
 
     def iscsiDiscoverSendTargets(self, con, options=None):
         con = protect_passwords(con)
+        ipv6Targets = con['ipv6Targets'] if 'ipv6Targets' in con else
False
         iscsiConn = API.ISCSIConnection(con['connection'], con['port'],
-                                        con['user'], con['password'])
+                                        con['user'], con['password'],
+                                        ipv6Targets)
         return iscsiConn.discoverSendTargets()
 
     def vgCreate(self, name, devlist, force=False):
diff --git a/vdsm/API.py b/vdsm/API.py
index f000297..da80afe 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -1012,18 +1012,20 @@
 
 
 class ISCSIConnection(APIBase):
-    ctorArgs = ['host', 'port', 'user', 'password']
+    ctorArgs = ['host', 'port', 'user', 'password',
'ipv6Targets']
 
-    def __init__(self, host, port, user="", password=""):
+    def __init__(self, host, port, user="", password="",
ipv6Targets=False):
         APIBase.__init__(self)
         self._host = host
         self._port = port
         self._user = user
         self._pass = password
+        self._ipv6Targets = ipv6Targets
 
     def discoverSendTargets(self):
         params = {'connection': self._host, 'port': self._port,
-                  'user': self._user, 'password': self._pass}
+                  'user': self._user, 'password': self._pass,
+                  'ipv6Targets': self._ipv6Targets}
         return self._irs.discoverSendTargets(params)
 
 
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index e472b1c..9d5f97d 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -2915,6 +2915,7 @@
         """
         ip = con['connection']
         port = int(con['port'])
+        ipv6Targets = con['ipv6Targets']
         username = con['user']
         password = con['password']
         if username == "":
@@ -2937,9 +2938,10 @@
         fullTargets = []
         partialTargets = []
         for target in targets:
-            fullTargets.append(iscsi.getTargetString(target.portal,
-                               target.tpgt, target.iqn))
-            partialTargets.append(target.iqn)
+            if ipv6Targets or not target.portal.startswith("["):
+                fullTargets.append(iscsi.getTargetString(target.portal,
+                                   target.tpgt, target.iqn))
+                partialTargets.append(target.iqn)
 
         return dict(targets=partialTargets, fullTargets=fullTargets)
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iecb45e326d9c551fb636f309f9e945023143dd9c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler <dholler(a)redhat.com&gt;

gerrit-hooks has posted comments on this change.

Change subject: storage: Extend the API for IPv6 iSCSI targets
......................................................................


Patch Set 1:

* Update Tracker::#1308306::OK, status: NEW
* Check Bug-Url::OK
* Check Public Bug::#1308306::OK, public bug
* Check Product::IGNORE, not relevant for branch: master
* Check TM::IGNORE, not relevant for branch: master
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iecb45e326d9c551fb636f309f9e945023143dd9c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler <dholler(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Edward Haas has posted comments on this change.

Change subject: storage: Extend the API for IPv6 iSCSI targets
......................................................................


Patch Set 1:

(2 comments)

Just some naming comments.

https://gerrit.ovirt.org/#/c/65794/1/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

PS1, Line 7719: ipv6Targets
The name is not hinting that this is a boolean.
Maybe something like: ipv6_enabled / ipv6_support etc.. with or without the
'target' included.

It mainly feels strange when used in the code later on.


https://gerrit.ovirt.org/#/c/65794/1/lib/vdsm/rpc/bindingxmlrpc.py
File lib/vdsm/rpc/bindingxmlrpc.py:

PS1, Line 987: con['ipv6Targets'] if 'ipv6Targets' in con else False
I think this will fit here: con.get('ipv6Targets', False)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iecb45e326d9c551fb636f309f9e945023143dd9c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler <dholler(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

gerrit-hooks has posted comments on this change.

Change subject: storage: Extend the API for IPv6 iSCSI targets
......................................................................


Patch Set 2:

* Update Tracker::#1308306::OK, status: NEW
* Check Bug-Url::OK
* Check Public Bug::#1308306::OK, public bug
* Check Product::IGNORE, not relevant for branch: master
* Check TM::IGNORE, not relevant for branch: master
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iecb45e326d9c551fb636f309f9e945023143dd9c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler <dholler(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: storage: Extend the API for IPv6 iSCSI targets
......................................................................


Patch Set 3:

* Update Tracker::#1308306::OK, status: NEW
* Check Bug-Url::OK
* Check Public Bug::#1308306::OK, public bug
* Check Product::IGNORE, not relevant for branch: master
* Check TM::IGNORE, not relevant for branch: master
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iecb45e326d9c551fb636f309f9e945023143dd9c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler <dholler(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Extend the API for IPv6 iSCSI targets
......................................................................


Patch Set 3:

(2 comments)

Partial review.

https://gerrit.ovirt.org/#/c/65794/3/lib/vdsm/rpc/bindingxmlrpc.py
File lib/vdsm/rpc/bindingxmlrpc.py:

Line 986:         con = protect_passwords(con)
Line 987:         ipv6_enabled = con.get('ipv6_enabled', False)
Line 988:         iscsiConn = API.ISCSIConnection(con['connection'],
con['port'],
Line 989:                                         con['user'],
con['password'],
Line 990:                                         ipv6_enabled)
xmlrpc is deprecated, we don't add new features here. Please remove this change.
Line 991:         return iscsiConn.discoverSendTargets()
Line 992: 
Line 993:     def vgCreate(self, name, devlist, force=False):
Line 994:         vg = API.LVMVolumeGroup(self.cif)


https://gerrit.ovirt.org/#/c/65794/3/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 2937:         # iscsiadm changes the output we can handle it gracefully
Line 2938:         fullTargets = []
Line 2939:         partialTargets = []
Line 2940:         for target in targets:
Line 2941:             if ipv6_enabled or not target.portal.startswith("["):
This would be more clear as:

    if ipv6_enabled or target.is_ipv6():
Line 2942:                 fullTargets.append(
Line 2943:                     iscsi.getTargetString(target.portal, target.tpgt,
target.iqn))
Line 2944:                 partialTargets.append(target.iqn)
Line 2945: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iecb45e326d9c551fb636f309f9e945023143dd9c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler <dholler(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

gerrit-hooks has posted comments on this change.

Change subject: storage: Extend the API for IPv6 iSCSI targets
......................................................................


Patch Set 4:

* Update Tracker::#1308306::OK, status: POST
* Check Bug-Url::OK
* Check Public Bug::#1308306::OK, public bug
* Check Product::IGNORE, not relevant for branch: master
* Check TM::IGNORE, not relevant for branch: master
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iecb45e326d9c551fb636f309f9e945023143dd9c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler <dholler(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: storage: Extend the API for IPv6 iSCSI targets
......................................................................


Patch Set 5:

* Update Tracker::#1308306::OK, status: POST
* Check Bug-Url::OK
* Check Public Bug::#1308306::OK, public bug
* Check Product::IGNORE, not relevant for branch: master
* Check TM::IGNORE, not relevant for branch: master
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iecb45e326d9c551fb636f309f9e945023143dd9c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler <dholler(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: storage: Extend the API for IPv6 iSCSI targets
......................................................................


Patch Set 8: Code-Review+1

Nir, anything left for you to approve this storage patch?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iecb45e326d9c551fb636f309f9e945023143dd9c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler <dholler(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dominik Holler <dholler(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No