Timothy Asir has posted comments on this change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
Patch Set 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, Done 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> ' Done 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} Yes, I will modify this 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() Done 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} yes, i will change this to simply return 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: Gluster does not provide description or access details (like readonly or editable) of an config option. That is the reason i added here. If gluster (cli) introduce a new option, this will simply bypass it unless we update this list. Deletion of any config option from gluster will not affect here. However, if there is any addition or deletion in config option will be treated as a change in gluster cli xml which needs to be taken care.
How do you think about having two different list for keeping description and access details separately. So that we avoid keeping the access detail flag for every option.
Like,
geoRepConfDesc = {'working_dir':"..", ...} geoRepConfReadOnly = ['working_dir', 'state_file', ...] for option in optionList: # getDesc(option) ... if option in geoRepConfReadOnly: ... 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): its reset only. I will change this function name like 'volumeGeoRepConfigReset' or something and add option to reset list of given config options. 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)