Change in vdsm[master]: gluster: performs peer detach using host UUID if provided

danken at redhat.com danken at redhat.com
Sun Nov 10 00:10:16 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: gluster: performs peer detach using host UUID if provided
......................................................................


Patch Set 2: Code-Review-1

(7 comments)

....................................................
File vdsm/gluster/api.py
Line 212:         self.svdsmProxy.glusterPeerProbe(hostName)
Line 213: 
Line 214:     @exportAsVerb
Line 215:     def hostRemove(self, hostName, force=False, options={}):
Line 216:         host = None
the code would become simpler if you keep using the hostName variable, and override it with the name acquired by hostUuid.
Line 217:         hostUuid = options.get('hostUuid')
Line 218: 
Line 219:         # get hostname of given host uuid
Line 220:         if hostUuid:


Line 218: 
Line 219:         # get hostname of given host uuid
Line 220:         if hostUuid:
Line 221:             for hostInfo in self.svdsmProxy.glusterPeerStatus():
Line 222:                 if(hostInfo['uuid'] == hostUuid):
it's not C here ;-), drop the parenthesis.
Line 223:                     host = hostInfo['hostname']
Line 224:                     break
Line 225:             if not host:
Line 226:                 log.warn("Host with UUID:%s not found " % hostUuid)


Line 221:             for hostInfo in self.svdsmProxy.glusterPeerStatus():
Line 222:                 if(hostInfo['uuid'] == hostUuid):
Line 223:                     host = hostInfo['hostname']
Line 224:                     break
Line 225:             if not host:
it would be nicer to drop the assignment of line 216, and do an "else" clause for the "for" loop, logging the warning.
Line 226:                 log.warn("Host with UUID:%s not found " % hostUuid)
Line 227:                 host = hostName
Line 228:         else:
Line 229:             host = hostName


....................................................
File vdsm/gluster/vdsmapi-gluster-schema.json
Line 919:  'data': {'hostName': 'str'},
Line 920:  'returns': 'bool'}
Line 921: 
Line 922: ##
Line 923: # @HostOptions:
RemoveHostOptions, maybe.
Line 924: #
Line 925: # List of host options.
Line 926: #
Line 927: # @hostUuid:  The Uuid of the host


Line 923: # @HostOptions:
Line 924: #
Line 925: # List of host options.
Line 926: #
Line 927: # @hostUuid:  The Uuid of the host
state that it has precedence over hostName.
Line 928: #
Line 929: # Since: 4.10.3
Line 930: ##
Line 931: {'type': 'HostOptions',


Line 925: # List of host options.
Line 926: #
Line 927: # @hostUuid:  The Uuid of the host
Line 928: #
Line 929: # Since: 4.10.3
it's 4.14 by now.
Line 930: ##
Line 931: {'type': 'HostOptions',
Line 932:  'data': {'hostUuid': 'UUID'}}
Line 933: 


Line 942: #
Line 943: # Since: 4.10.3
Line 944: ##
Line 945: {'command': {'class': 'GlusterHost', 'name': 'remove'},
Line 946:  'data': {'hostName': 'str', 'force': 'bool', 'options': 'HostOptions'}
"options" are optional. state this with an *.
Line 947:  'returns': 'bool'}
Line 948: 
Line 949: ##
Line 950: # @HostStatus:


-- 
To view, visit http://gerrit.ovirt.org/20648
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idfff258ccc43c276ca7b9c0472dacac2fefce64a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Aravinda VK <avishwan at redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list