Bala.FA has posted comments on this change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
Patch Set 1: Code-Review-1
(7 comments)
.................................................... Commit Message Line 18: 'description': DESCRIPTION}... } Line 19: * glusterVolumeGeoRepConfigGet Line 20: - return value structure: Line 21: {NAME: VALUE, Line 22: writable: BOOL, Please fix whitespace Line 23: 'description': DESCRIPTION} Line 24: * glusterVolumeGeoRepConfigSet Line 25: * glusterVolumeGeoRepConfigDelete Line 26:
.................................................... File client/vdsClientGluster.py Line 858: 'get volume geo-replication configuration' Line 859: )), Line 860: 'glusterVolumeGeoRepConfigDelete': ( Line 861: serv.do_glusterVolumeGeoRepConfigDelete, Line 862: ('masterVolName=<master_volume_name> slaveHost=<slave_host> ' Use volumeName, remoteHost, remoteVolume instead of masterVolName, slaveHost, slaveVolName everywhere Line 863: 'slaveVolName=<slave_volume_name> keyName=<key> ' Line 864: 'value=<value>\n\t' Line 865: '<master_volume_name> is an existing volume name in ' Line 866: 'the master node\n\t'
.................................................... File vdsm/gluster/api.py Line 319: slaveHost, Line 320: slaveVolName, Line 321: key, Line 322: value) Line 323: return {'geoRepSet': status} This is not matter of boolean? Line 324: Line 325: @exportAsVerb Line 326: def volumeGeoRepConfigGet(self, masterVolName, slaveHost, slaveVolName, Line 327: key, options=None):
Line 327: key, options=None): Line 328: Line 329: fp = open("/tmp/file1.txt", "w") Line 330: fp.write("entering geo rep config get") Line 331: fp.close() remove this debugging code Line 332: status = self.svdsmProxy.glusterVolumeGeoRepConfigGet(masterVolName, Line 333: slaveHost, Line 334: slaveVolName, Line 335: key)
Line 342: status = self.svdsmPorxy.glusterVolumeGeoRepConfigDelete(masterVolName, Line 343: slaveHost, Line 344: slaveVolName, Line 345: key) Line 346: return {'geoRepDelete': status} return is not a boolean? Line 347: Line 348: Line 349: def getGlusterMethods(gluster): Line 350: l = []
.................................................... File vdsm/gluster/cli.py Line 159: 'working_dir': ( Line 160: 'workingDir', Line 161: 'working directory', Line 162: GeoRepConf.READONLY)} Line 163: I am not able to understand why you maintain config list. What if this list grows/shrinks depending on glusterfs version? What if some config turns into read-only or writable?
I think this should be outside of vdsm Line 164: Line 165: def _execGluster(cmd): Line 166: return utils.execCmd(cmd) Line 167:
Line 1104: 'editable': GEOREP_CONFIG[key][2]}} Line 1105: Line 1106: Line 1107: @makePublic Line 1108: def volumeGeoRepConfigDelete(masterVolName, slaveHost, slaveVolName, key): is it deleting the conf or resetting the conf? Line 1109: command = _getGlusterVolCmd() + ["geo-replication", masterVolName, Line 1110: "%s::%s" % (slaveHost, slaveVolName), Line 1111: "config", "!%s" % key] Line 1112: rc, out, err = _execGluster(command)