Change in vdsm[master]: gluster: additional parsing of host UUID in verb glusterVolu...

Hello Bala.FA,

I'd like you to do a code review.  Please visit

http://gerrit.ovirt.org/22693

to review the following change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

gluster: additional parsing of host UUID in verb glusterVolumesList

This patch adds parsing of host UUID for all the bricks in the
verb glusterVolumesList.

Change-Id: I9057f3aea0c0ed8fb4d4ec26eaf66fe80ec581e2
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1038988
Signed-off-by: ndarshan 
---
M vdsm/gluster/cli.py
1 file changed, 2 insertions(+), 2 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/22693/1

diff --git a/vdsm/gluster/cli.py b/vdsm/gluster/cli.py
index 167f01c..44d82dc 100644
--- a/vdsm/gluster/cli.py
+++ b/vdsm/gluster/cli.py
@@ -353,10 +353,10 @@
             value['transportType'] = [TransportType.RDMA]
         else:
             value['transportType'] = [TransportType.TCP, TransportType.RDMA]
-        value['bricks'] = []
+        value['bricks'] = {}
         value['options'] = {}
         for b in el.findall('bricks/brick'):
-            value['bricks'].append(b.text)
+            value['bricks'][b.text] = b.get('uuid')
         for o in el.findall('options/option'):
             value['options'][o.find('name').text] = o.find('value').text
         volumes[value['volumeName']] = value

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 1: Verified-1

Build Failed

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

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

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

Bala.FA has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 1: Code-Review-1

(1 comment)

....................................................
File vdsm/gluster/cli.py
Line 352:         elif transportType == '1':
Line 353:             value['transportType'] = [TransportType.RDMA]
Line 354:         else:
Line 355:             value['transportType'] = [TransportType.TCP, TransportType.RDMA]
Line 356:         value['bricks'] = {}
This breaks backward compatibility.  Please fix it
Line 357:         value['options'] = {}
Line 358:         for b in el.findall('bricks/brick'):
Line 359:             value['bricks'][b.text] = b.get('uuid')
Line 360:         for o in el.findall('options/option'):

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

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

Build Failed

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

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

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

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 2: Code-Review-1

(2 comments)

....................................................
Commit Message
Line 6: 
Line 7: gluster: additional parsing of host UUID in verb glusterVolumesList
Line 8: 
Line 9: This patch adds parsing of host UUID for all the bricks in the
Line 10: verb glusterVolumesList.
Should you update the schema, too?
Line 11: 
Line 12: Change-Id: I9057f3aea0c0ed8fb4d4ec26eaf66fe80ec581e2
Line 13: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1038988

....................................................
File tests/gluster_cli_tests.py
Line 115:             {'music': {'brickCount': '2',
Line 116:                        'bricks': ['192.168.122.2:/tmp/music-b1',
Line 117:                                   '192.168.122.2:/tmp/music-b2'],
Line 118:                        'distCount': '2',
Line 119:                        'hostUuid': {'192.168.122.2:/tmp/music-b1': '04eb591b-2fd3-489e-a22c-5d342a3c713d',
Please keep shorter lines.
Line 120:                                     '192.168.122.2:/tmp/music-b2': '04eb591b-2fd3-489e-a22c-5d342a3c713d'},
Line 121:                        'options': {'auth.allow': '*'},
Line 122:                        'replicaCount': '2',
Line 123:                        'stripeCount': '1',

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 3: Verified-1

Build Failed

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

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

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 4:

Build Successful

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

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

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

Kanagaraj M has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 4:

Can you please add the new brick representation with host-uuid to commit msg?

Sahina Bose has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/22693/4/vdsm/gluster/cli.py
File vdsm/gluster/cli.py:

Line 360:             value['bricks'].append(b.text)
Line 361:         for o in el.findall('options/option'):
Line 362:             value['options'][o.find('name').text] = o.find('value').text
Line 363:         for d in el.findall('bricks/brick'):
Line 364:             brickDetail = {}
Can this be added only if hostUuid is found in the xml?
Line 365:             brickDetail['name'] = d.find('name').text
Line 366:             brickDetail['hostUuid'] = d.find('hostUuid').text
Line 367:             value['bricksInfo'].append(brickDetail)
Line 368:         volumes[value['volumeName']] = value

Darshan N has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/22693/4/vdsm/gluster/cli.py
File vdsm/gluster/cli.py:

Line 360:             value['bricks'].append(b.text)
Line 361:         for o in el.findall('options/option'):
Line 362:             value['options'][o.find('name').text] = o.find('value').text
Line 363:         for d in el.findall('bricks/brick'):
Line 364:             brickDetail = {}
> Can this be added only if hostUuid is found in the xml?

Done
Line 365:             brickDetail['name'] = d.find('name').text
Line 366:             brickDetail['hostUuid'] = d.find('hostUuid').text
Line 367:             value['bricksInfo'].append(brickDetail)
Line 368:         volumes[value['volumeName']] = value

Darshan N has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 2:

(2 comments)

http://gerrit.ovirt.org/#/c/22693/2//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: gluster: additional parsing of host UUID in verb glusterVolumesList
Line 8: 
Line 9: This patch adds parsing of host UUID for all the bricks in the
Line 10: verb glusterVolumesList.
> Should you update the schema, too?

Done
Line 11: 
Line 12: Change-Id: I9057f3aea0c0ed8fb4d4ec26eaf66fe80ec581e2
Line 13: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1038988

http://gerrit.ovirt.org/#/c/22693/2/tests/gluster_cli_tests.py
File tests/gluster_cli_tests.py:

Line 115:             {'music': {'brickCount': '2',
Line 116:                        'bricks': ['192.168.122.2:/tmp/music-b1',
Line 117:                                   '192.168.122.2:/tmp/music-b2'],
Line 118:                        'distCount': '2',
Line 119:                        'hostUuid': {'192.168.122.2:/tmp/music-b1': '04eb591b-2fd3-489e-a22c-5d342a3c713d',
> Please keep shorter lines.

Done
Line 120:                                     '192.168.122.2:/tmp/music-b2': '04eb591b-2fd3-489e-a22c-5d342a3c713d'},
Line 121:                        'options': {'auth.allow': '*'},
Line 122:                        'replicaCount': '2',
Line 123:                        'stripeCount': '1',

Aravinda VK has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 5: Code-Review-1

(1 comment)

One minor code comment. Rest of the changes looks good.

http://gerrit.ovirt.org/#/c/22693/5/vdsm/gluster/cli.py
File vdsm/gluster/cli.py:

Line 367:             try:
Line 368:                 brickDetail['name'] = d.find('name').text
Line 369:                 brickDetail['hostUuid'] = d.find('hostUuid').text
Line 370:                 value['bricksInfo'].append(brickDetail)
Line 371:             except:
1. Please catch exact exception.
2. Use break instead of pass, so that you can eliminate looping all brick with pass.
Line 372:                 pass
Line 373:         volumes[value['volumeName']] = value
Line 374:     return volumes
Line 375:

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 5: Verified-1

Build Failed

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

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

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 6:

Build Successful

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

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

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

Darshan N has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 5:

(1 comment)

http://gerrit.ovirt.org/#/c/22693/5/vdsm/gluster/cli.py
File vdsm/gluster/cli.py:

Line 367:             try:
Line 368:                 brickDetail['name'] = d.find('name').text
Line 369:                 brickDetail['hostUuid'] = d.find('hostUuid').text
Line 370:                 value['bricksInfo'].append(brickDetail)
Line 371:             except:
> Please catch exact exception.

Done
Line 372:                 pass
Line 373:         volumes[value['volumeName']] = value
Line 374:     return volumes
Line 375:

Darshan N has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 6: Verified+1

Aravinda VK has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 6: Code-Review+1

Darshan N has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/22693/1/vdsm/gluster/cli.py
File vdsm/gluster/cli.py:

Line 352:         elif transportType == '1':
Line 353:             value['transportType'] = [TransportType.RDMA]
Line 354:         else:
Line 355:             value['transportType'] = [TransportType.TCP, TransportType.RDMA]
Line 356:         value['bricks'] = {}
> This breaks backward compatibility.  Please fix it

Done
Line 357:         value['options'] = {}
Line 358:         for b in el.findall('bricks/brick'):
Line 359:             value['bricks'][b.text] = b.get('uuid')
Line 360:         for o in el.findall('options/option'):

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 7:

Build Successful

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

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

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

Darshan N has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 7: Verified+1

Aravinda VK has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 7: Code-Review+1

Sahina Bose has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 7: Code-Review+1

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

Patch Set 7: Code-Review+2

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: gluster: additional parsing of host UUID in verb glusterVolumesList
......................................................................

gluster: additional parsing of host UUID in verb glusterVolumesList

This patch adds parsing of host UUID for all the bricks in the
verb glusterVolumesList.
This information is stored in bricksInfo which is list of dictionary
where each dictionary has name and hostUUID.
sample:
---------------------------------------------------------------
     'bricksInfo': [{
                     'name': '192.168.122.2:/tmp/t_b1',
                     'hostUuid': '04eb591b-2fd3-489e-a22c-5d342a3c713d'
                     },
                    {
                     'name': '192.168.122.2:/tmp/t_b2',
                     'hostUuid': '04eb591b-2fd3-489e-a22c-5d342a3c713d'
                     }]
---------------------------------------------------------------

Change-Id: I9057f3aea0c0ed8fb4d4ec26eaf66fe80ec581e2
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1038988
Signed-off-by: ndarshan 
Reviewed-on: http://gerrit.ovirt.org/22693
Reviewed-by: Aravinda VK 
Reviewed-by: Sahina Bose 
Reviewed-by: Dan Kenigsberg 
---
M tests/gluster_cli_tests.py
M vdsm/gluster/cli.py
M vdsm/gluster/vdsmapi-gluster-schema.json
3 files changed, 52 insertions(+), 7 deletions(-)

Approvals:
  Aravinda VK: Looks good to me, but someone else must approve
  Darshan N: Verified
  Dan Kenigsberg: Looks good to me, approved
  Sahina Bose: Looks good to me, but someone else must approve