6:52 a.m.
Hello Ayal Baron, Timothy Asir, Saggi Mizrahi, Federico Simoncelli, Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/7617
to review the following change.
Change subject: glusterHostsList verb returns local hostname as IP address or FQDN
......................................................................
glusterHostsList verb returns local hostname as IP address or FQDN
Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Signed-off-by: Bala.FA <barumuga(a)redhat.com>
---
M vdsm/gluster/cli.py
M vdsm/gluster/exception.py
2 files changed, 5 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/7617/1
diff --git a/vdsm/gluster/cli.py b/vdsm/gluster/cli.py
index dcd8eda..659eef3 100644
--- a/vdsm/gluster/cli.py
+++ b/vdsm/gluster/cli.py
@@ -732,7 +732,8 @@
"""
rc, out, err = _execGluster(_getGlusterPeerCmd() + ["status"])
if rc:
- raise ge.GlusterHostListFailedException(rc, out, err)
+ raise ge.GlusterHostsListFailedException(rc, out, err)
else:
- return _parsePeerStatus(out, _getGlusterHostName(),
+ return _parsePeerStatus(out,
+ _getLocalIpAddress() or _getGlusterHostName(),
_getGlusterUuid(), HostStatus.CONNECTED)
diff --git a/vdsm/gluster/exception.py b/vdsm/gluster/exception.py
index 3b8a978..abe2ffb 100644
--- a/vdsm/gluster/exception.py
+++ b/vdsm/gluster/exception.py
@@ -364,6 +364,6 @@
message = "Remove host failed"
-class GlusterHostListFailedException(GlusterHostException):
+class GlusterHostsListFailedException(GlusterHostException):
code = 4407
- message = "Host list failed"
+ message = "Hosts list failed"
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
3:32 a.m.
New subject: Change in vdsm[master]: glusterHostsList verb uses gluster cli xml output.
Dan Kenigsberg has posted comments on this change.
Change subject: glusterHostsList verb uses gluster cli xml output.
......................................................................
Patch Set 4: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
3:32 a.m.
New subject: Change in vdsm[master]: glusterHostsList verb uses gluster cli xml output.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: glusterHostsList verb uses gluster cli xml output.
......................................................................
glusterHostsList verb uses gluster cli xml output.
This eliminates standard output text parsing.
Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Signed-off-by: Bala.FA <barumuga(a)redhat.com>
---
M tests/gluster_cli_tests.py
M vdsm/gluster/cli.py
M vdsm/gluster/exception.py
3 files changed, 74 insertions(+), 50 deletions(-)
Approvals:
Timothy Asir: Verified; Looks good to me, but someone else must approve
Dan Kenigsberg: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
2:57 a.m.
New subject: Change in vdsm[master]: glusterHostsList verb uses gluster cli xml output.
Timothy Asir has posted comments on this change.
Change subject: glusterHostsList verb uses gluster cli xml output.
......................................................................
Patch Set 4: Verified; Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
1:51 a.m.
New subject: Change in vdsm[master]: glusterHostsList verb uses gluster cli xml output.
Dan Kenigsberg has posted comments on this change.
Change subject: glusterHostsList verb uses gluster cli xml output.
......................................................................
Patch Set 4: Looks good to me, but someone else must approve
would you please find another reviewer, hopefully with gluster know-how?
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
9:51 p.m.
New subject: Change in vdsm[master]: glusterHostsList verb uses gluster cli xml output.
Bala.FA has posted comments on this change.
Change subject: glusterHostsList verb uses gluster cli xml output.
......................................................................
Patch Set 3: (3 inline comments)
....................................................
File vdsm/gluster/cli.py
Line 541: return hostList
Line 542:
Line 543:
Line 544: @exportToSuperVdsm
Line 545: def peerStatus():
'gluster peer status' gluster gli command lists servers in the gluster cluster
except the host where we run the command. But the engine side, its required to give all
servers. This is handled by adding current host detail by default. If you feel moving
this local host addition to api.py, I can do that.
Line 546: """
Line 547: Returns:
Line 548: [{'hostname': HOSTNAME, 'uuid': UUID, 'status':
STATE}, ...]
Line 549: """
Line 553: except ge.GlusterCmdFailedException, e:
Line 554: raise ge.GlusterHostsListFailedException(rc=e.rc, err=e.err)
Line 555: try:
Line 556: return _parsePeerStatus(xmltree,
Line 557: _getLocalIpAddress() or _getGlusterHostName(),
In gluster sync functionality in engine side, its unable to identify a host with its fqdn
name, but can do with any one of the ip address of the host. Based on this,
_getLocalIpAddress() prefer than _getGlusterHostName()
Yes. This is not related to xml. I will move it to separate patch.
Line 558: _getGlusterUuid(), HostStatus.CONNECTED)
Line 559: except:
Line 555: try:
Line 556: return _parsePeerStatus(xmltree,
Line 557: _getLocalIpAddress() or _getGlusterHostName(),
Line 558: _getGlusterUuid(), HostStatus.CONNECTED)
Line 559: except:
Done
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
4:20 p.m.
New subject: Change in vdsm[master]: glusterHostsList verb uses gluster cli xml output.
Dan Kenigsberg has posted comments on this change.
Change subject: glusterHostsList verb uses gluster cli xml output.
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/gluster/cli.py
Line 541: return hostList
Line 542:
Line 543:
Line 544: @exportToSuperVdsm
Line 545: def peerStatus():
hmmm, it is not very nice. this verb assume that the client and server sits on the same
host. I suppose it would make more sense for this verb to receive the address of the
server, and use _getLocalIpAddress() or _getGlusterHostName() only as a default.
Line 546: """
Line 547: Returns:
Line 548: [{'hostname': HOSTNAME, 'uuid': UUID, 'status':
STATE}, ...]
Line 549: """
Line 553: except ge.GlusterCmdFailedException, e:
Line 554: raise ge.GlusterHostsListFailedException(rc=e.rc, err=e.err)
Line 555: try:
Line 556: return _parsePeerStatus(xmltree,
Line 557: _getLocalIpAddress() or _getGlusterHostName(),
would you explain why you prefer getLocalIpAddress on getGlusterHostName? this change may
fit into a separate patch - is it related to the move to xml?
Line 558: _getGlusterUuid(), HostStatus.CONNECTED)
Line 559: except:
Line 555: try:
Line 556: return _parsePeerStatus(xmltree,
Line 557: _getLocalIpAddress() or _getGlusterHostName(),
Line 558: _getGlusterUuid(), HostStatus.CONNECTED)
Line 559: except:
"except" is way too wide to be correct. assume a network error while in
_getGlusterHostName(). or a typo hiding somewhere. you do not want to swallow everything.
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
8:30 a.m.
New subject: Change in vdsm[master]: glusterHostsList verb uses gluster cli xml output.
Bala.FA has posted comments on this change.
Change subject: glusterHostsList verb uses gluster cli xml output.
......................................................................
Patch Set 3: Verified
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
3:04 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: glusterHostsList verb returns local hostname as IP address or FQDN
......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/789/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
9:56 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: glusterHostsList verb returns local hostname as IP address or FQDN
......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/774/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
4269
days inactive
4280
days old
vdsm-patches@lists.fedorahosted.org
9 comments
4 participants
participants (4)
-
barumuga＠redhat.com -
Dan Kenigsberg -
Jenkins CI RO -
tjeyasin＠redhat.com