Hello Ayal Baron, Bala.FA, Saggi Mizrahi, Dan Kenigsberg,
I'd like you to do a code review. Please visit
to review the following change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
gluster: Add gluster volume geo-replication configuration feature
Configure geo-replication options between the hosts specified by MASTER and SLAVE. This provides listing, add, remove & update geo-rep configurations.
New verbs: * glusterVolumeGeoRepConfigList - return value structure: {{NAME: VALUE, writable: BOOL, 'description': DESCRIPTION}... } * glusterVolumeGeoRepConfigGet - return value structure: {NAME: VALUE, writable: BOOL, 'description': DESCRIPTION} * glusterVolumeGeoRepConfigSet * glusterVolumeGeoRepConfigDelete
Change-Id: I9c43f0950bbaa215cfe22ba18cc02e5c5851c347 Signed-off-by: Timothy Asir tjeyasin@redhat.com --- M client/vdsClientGluster.py M vdsm/gluster/api.py M vdsm/gluster/cli.py M vdsm/gluster/exception.py M vdsm/gluster/vdsmapi-gluster-schema.json 5 files changed, 471 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/03/18303/1
diff --git a/client/vdsClientGluster.py b/client/vdsClientGluster.py index 0e8ebd1..8258c26 100644 --- a/client/vdsClientGluster.py +++ b/client/vdsClientGluster.py @@ -453,6 +453,70 @@ pp.pprint(status) return status['status']['code'], status['status']['message']
+ def do_glusterVolumeGeoRepConfigList(self, args): + params = self._eqSplit(args) + masterVolName = params.get('masterVolName', '') + slaveHost = params.get('slaveHost', '') + slaveVolName = params.get('slaveVolName', '') + if not(masterVolName and slaveHost and slaveVolName): + raise ValueError + + status = self.s.glusterVolumeGeoRepConfigList(masterVolName, + slaveHost, + slaveVolName) + pp.pprint(status) + return status['status']['code'], status['status']['message'] + + def do_glusterVolumeGeoRepConfigSet(self, args): + params = self._eqSplit(args) + masterVolName = params.get('masterVolName', '') + slaveHost = params.get('slaveHost', '') + slaveVolName = params.get('slaveVolName', '') + key = params.get('key', '') + value = params.get('value', '') + if not(masterVolName and slaveHost and slaveVolName and key and value): + raise ValueError + + status = self.s.glusterVolumeGeoRepConfigSet(masterVolName, + slaveHost, + slaveVolName, + key, + value) + pp.pprint(status) + return status['status']['code'], status['status']['message'] + + def do_glusterVolumeGeoRepConfigGet(self, args): + params = self._eqSplit(args) + masterVolName = params.get('masterVolName', '') + slaveHost = params.get('slaveHost', '') + slaveVolName = params.get('slaveVolName', '') + key = params.get('key', '') + if not(masterVolName and slaveHost and slaveVolName and key): + raise ValueError + + status = self.s.glusterVolumeGeoRepConfigGet(masterVolName, + slaveHost, + slaveVolName, + key) + pp.pprint(status) + return status['status']['code'], status['status']['message'] + + def do_glusterVolumeGeoRepConfigDelete(self, args): + params = self._eqSplit(args) + masterVolName = params.get('masterVolName', '') + slaveHost = params.get('slaveHost', '') + slaveVolName = params.get('slaveVolName', '') + key = params.get('key', '') + if not(masterVolName and slaveHost and slaveVolName and key): + raise ValueError + + status = self.s.glusterVolumeGeoRepConfigDelete(masterVolName, + slaveHost, + slaveVolName, + key) + pp.pprint(status) + return status['status']['code'], status['status']['message'] +
def getGlusterCmdDict(serv): return \ @@ -754,5 +818,56 @@ '<slave_host_name>is remote slave host name or ip\n\t' '<slave_volume_name>existing volume name in the slave node', 'Delete the geo-replication session' + )), + 'glusterVolumeGeoRepConfigList': ( + serv.do_glusterVolumeGeoRepConfigList, + ('masterVolName=<master_volume_name> slaveHost=<slave_host> ' + 'slaveVolName=<slave_volume_name>\n\t' + '<master_volume_name> is an existing volume name in ' + 'the master node\n\t' + '<slave_host> is slave host name\n\t' + '<slave_volume_name> is an existing volume name ' + 'in the slave node', + 'list volume geo-replication configurations' + )), + 'glusterVolumeGeoRepConfigSet': ( + serv.do_glusterVolumeGeoRepConfigSet, + ('masterVolName=<master_volume_name> slaveHost=<slave_host> ' + 'slaveVolName=<slave_volume_name> keyName=<key> ' + 'value=<value>\n\t' + '<master_volume_name> is an existing volume name ' + 'in the master node\n\t' + '<slave_host> is slave host name\n\t' + '<slave_volume_name> is an existing volume name in ' + 'the slave node\n\t' + '<key> is the key name\n\t' + '<value> is the key value', + 'set volume geo-replication configuration' + )), + 'glusterVolumeGeoRepConfigGet': ( + serv.do_glusterVolumeGeoRepConfigGet, + ('masterVolName=<master_volume_name> slaveHost=<slave_host> ' + 'slaveVolName=<slave_volume_name> keyName=<key> ' + 'value=<value>\n\t' + '<master_volume_name> is an existing volume name in ' + 'the master node\n\t' + '<slave_host> is slave host name\n\t' + '<slave_volume_name> is an existing volume name in ' + 'the slave node\n\t' + '<key> is the key name', + 'get volume geo-replication configuration' + )), + 'glusterVolumeGeoRepConfigDelete': ( + serv.do_glusterVolumeGeoRepConfigDelete, + ('masterVolName=<master_volume_name> slaveHost=<slave_host> ' + 'slaveVolName=<slave_volume_name> keyName=<key> ' + 'value=<value>\n\t' + '<master_volume_name> is an existing volume name in ' + 'the master node\n\t' + '<slave_host> is slave host name\n\t' + '<slave_volume_name> is an existing volume name in ' + 'the slave node\n\t' + '<key> is the key name', + 'Delete volume geo-replication configuration' )) } diff --git a/vdsm/gluster/api.py b/vdsm/gluster/api.py index d65a8a2..6a58ece 100644 --- a/vdsm/gluster/api.py +++ b/vdsm/gluster/api.py @@ -304,6 +304,47 @@ slaveVolName) return {'geo-rep': status}
+ @exportAsVerb + def volumeGeoRepConfigList(self, masterVolName, slaveHost, slaveVolName, + options=None): + status = self.svdsmProxy.glusterVolumeGeoRepConfigList(masterVolName, + slaveHost, + slaveVolName) + return {'geoRepConfig': status} + + @exportAsVerb + def volumeGeoRepConfigSet(self, masterVolName, slaveHost, slaveVolName, + key, value, options=None): + status = self.svdsmProxy.glusterVolumeGeoRepConfigSet(masterVolName, + slaveHost, + slaveVolName, + key, + value) + return {'geoRepSet': status} + + @exportAsVerb + def volumeGeoRepConfigGet(self, masterVolName, slaveHost, slaveVolName, + key, options=None): + + fp = open("/tmp/file1.txt", "w") + fp.write("entering geo rep config get") + fp.close() + status = self.svdsmProxy.glusterVolumeGeoRepConfigGet(masterVolName, + slaveHost, + slaveVolName, + key) + return {'geoRepGet': status} + + @exportAsVerb + def volumeGeoRepConfigDelete(self, masterVolName, slaveHost, slaveVolName, + key, options=None): + + status = self.svdsmPorxy.glusterVolumeGeoRepConfigDelete(masterVolName, + slaveHost, + slaveVolName, + key) + return {'geoRepDelete': status} +
def getGlusterMethods(gluster): l = [] diff --git a/vdsm/gluster/cli.py b/vdsm/gluster/cli.py index 4529a44..d16d88f 100644 --- a/vdsm/gluster/cli.py +++ b/vdsm/gluster/cli.py @@ -73,6 +73,95 @@ RDMA = 'RDMA'
+class GeoRepConf: + EDITABLE = 'True' + READONLY = 'False' + + +GEOREP_CONFIG = { + 'gluster_log_file': ( + 'glusterLogFile', + 'Path to the geo-replication glusterfs log file', + GeoRepConf.EDITABLE), + 'gluster_log_level': ( + 'glusterLogLevel', + 'The log level for glusterfs processes', + GeoRepConf.EDITABLE), + 'log_file': ( + 'logFile', + 'The path to the geo-replication log file', + GeoRepConf.EDITABLE), + 'ssh_command': ( + 'sshCommand', + 'Command to connect to the remote machine (the default is ssh)', + GeoRepConf.EDITABLE), + 'rsync_command': ( + 'rsyncCommand', + 'rsync command for syncronizing (default - rsync)', + GeoRepConf.EDITABLE), + 'volume_id': ( + 'volumeID', + 'Volume ID which can be use to delete the existing master UID ' + 'for the intermediate/slave node', + GeoRepConf.EDITABLE), + 'timeout': ( + 'timeout', + 'The timeout period', GeoRepConf.EDITABLE), + 'sync_jobs': ( + 'syncJobs', + 'The number of simultaneous files/directories ' + 'that can be synchronized', + GeoRepConf.EDITABLE), + 'ignore_deletes': ( + 'ignoreDeletes', + 'delete files only in master and will not trigger a delete ' + 'operation in the slave. Hence, the slave will remain as a ' + 'superset of the master and can be used to recover the master ' + 'in case of any crash or accidental delete', + GeoRepConf.EDITABLE), + 'checkpoint': ( + 'checkPoint', + 'Sets the checkpoint with the given value (label). If the value is ' + 'set as now, then the current time will be used as value (label)', + GeoRepConf.EDITABLE), + 'gluster_command_dir': ( + 'glusterCommandDir', + 'gluster command path', + GeoRepConf.READONLY), + 'gluster_params': ( + 'glusterParams', + 'gluster parameters', + GeoRepConf.READONLY), + 'pid_file': ( + 'pidFile', + 'geo replication session pid file', + GeoRepConf.READONLY), + 'session_owner': ( + 'sessionOwner', + 'session owner uuid', + GeoRepConf.READONLY), + 'socketdir': ( + 'socketDir', + 'socket directory', + GeoRepConf.READONLY), + 'special_sync_mode': ( + 'specialSyncMode', + 'special sync mode', + GeoRepConf.READONLY), + 'state_detail_file': ( + 'stateDetailFile', + 'state detail file', + GeoRepConf.READONLY), + 'state_file': ( + 'stateFile', + 'state file', + GeoRepConf.READONLY), + 'working_dir': ( + 'workingDir', + 'working directory', + GeoRepConf.READONLY)} + + def _execGluster(cmd): return utils.execCmd(cmd)
@@ -938,3 +1027,89 @@ return True except ge.GlusterCmdFailedException as e: raise ge.GlusterGeoRepDeletionFailedException(rc=e.rc, err=e.err) + + +def _parseVolumeGeoRepConfigListXml(tree): + config = {} + + for el in tree.findall('geoRepConfig'): + try: + key = el.find('name').text + config[GEOREP_CONFIG[key]] = { + 'value': el.find('value').text, + 'description': GEOREP_CONFIG[key][1], + 'editable': GEOREP_CONFIG[key][2]} + except KeyError: + pass # omitting unwanted items + return config + + +def _parseVolumeGeoRepConfigList(out): + config = {} + + for line in out: + k, v = line.split(':') + try: + config[GEOREP_CONFIG[k][0]] = {'value': v.strip(), + 'description': GEOREP_CONFIG[k][1], + 'editable': GEOREP_CONFIG[k][2]} + except KeyError: + pass # omitting unwanted items + return config + + +@makePublic +def volumeGeoRepConfigList(masterVolName, slaveHost, slaveVolName): + """Returns + {{OPTIONNAME: VALUE, + 'writable': BOOL, + 'description': DESCRIPTION}... } + """ + command = _getGlusterVolCmd() + ["geo-replication", masterVolName, + "%s::%s" % (slaveHost, slaveVolName), + "config"] + + try: + xmltree = _execGlusterXml(command) + return _parseVolumeGeoRepConfigListXml(xmltree) + except ge.GlusterCmdFailedException as e: + rc, out, err = _execGluster(command) + if rc: + raise ge.GlusterGeoRepConfigListFailedException(rc, out, err) + return _parseVolumeGeoRepConfigList(out) + + +@makePublic +def volumeGeoRepConfigSet(masterVolName, slaveHost, slaveVolName, key, value): + command = _getGlusterVolCmd() + ["geo-replication", masterVolName, + "%s::%s" % (slaveHost, slaveVolName), + "config", key, value] + rc, out, err = _execGluster(command) + if rc: + raise ge.GlusterGeoRepConfigSetFailedException(rc, out, err) + return True + + +@makePublic +def volumeGeoRepConfigGet(masterVolName, slaveHost, slaveVolName, key): + command = _getGlusterVolCmd() + ["geo-replication", masterVolName, + "%s::%s" % (slaveHost, slaveVolName), + "config", key] + rc, out, err = _execGluster(command) + if rc: + raise ge.GlusterGeoRepConfigGetFailedException(rc, out, err) + + return {key: {'name': out.strip(), + 'description': GEOREP_CONFIG[key][1], + 'editable': GEOREP_CONFIG[key][2]}} + + +@makePublic +def volumeGeoRepConfigDelete(masterVolName, slaveHost, slaveVolName, key): + command = _getGlusterVolCmd() + ["geo-replication", masterVolName, + "%s::%s" % (slaveHost, slaveVolName), + "config", "!%s" % key] + rc, out, err = _execGluster(command) + if rc: + raise ge.GlusterGeoRepConfigDeleteFailedException(rc, out, err) + return True diff --git a/vdsm/gluster/exception.py b/vdsm/gluster/exception.py index 0e98c06..846a70e 100644 --- a/vdsm/gluster/exception.py +++ b/vdsm/gluster/exception.py @@ -511,3 +511,23 @@ class GlusterGeoRepDeletionFailedException(GlusterGeoRepException): code = 4564 message = "Geo Rep session deletion failed" + + +class GlusterGeoRepConfigListFailedException(GlusterVolumeException): + code = 4165 + message = "Get volume geo-replication config list failed" + + +class GlusterGeoRepConfigSetFailedException(GlusterVolumeException): + code = 4166 + message = "Set volume geo-replication config failed" + + +class GlusterGeoRepConfigGetFailedException(GlusterVolumeException): + code = 4167 + message = "Get volume geo-replication config failed" + + +class GlusterGeoRepConfigDeleteFailedException(GlusterVolumeException): + code = 4168 + message = "Delete volume geo-replication config failed" diff --git a/vdsm/gluster/vdsmapi-gluster-schema.json b/vdsm/gluster/vdsmapi-gluster-schema.json index e4dad64..da4177e 100644 --- a/vdsm/gluster/vdsmapi-gluster-schema.json +++ b/vdsm/gluster/vdsmapi-gluster-schema.json @@ -414,3 +414,123 @@ {'command': {'class': 'GlusterGeoRep', 'name': 'delete'}, 'data': {'masterVolName': 'str', 'slaveHost': 'str', 'slaveVolName': 'str'}, 'returns': 'bool'} + +## +# @GeoRepConfig: +# +# Geo replication config details. +# +# @optionname: Config option name +# +# @editable: Editable option or not +# +# @description: Option details +# +# Since: 4.10.3 +## +{'type': 'GeoRepConfig', + 'data': {'optionname': 'str', 'editable': 'bool', 'description': 'str'}} + +## +# @GlusterGeoRep.configList: +# +# List Geo Replication configuration +# +# @mastervolname: is an existing volume name in the master node +# +# @slavehost: is remote slave host name or ip +# +# @slavevolname: is an available existing volume name in the slave node +# +# Returns: +# List of geo replication configurations +# +# Since: 4.10.3 +## +{'command': {'class': 'GlusterGeoRep', 'name': 'geoRepConfigList'}, + 'data': {'mastervolname': 'str', 'slavehost': 'str', 'slavevolname': 'str'}, + 'returns': 'GeoRepConfig'} + +## +# @GlusterGeoRep.configSet: +# +# Set Geo Replication config option +# +# @mastervolname: is an existing volume name in the master node +# +# @slavehost: is remote slave host name or ip +# +# @slavevolname: is an available existing volume name in the slave node +# +# @key: valid configuration option name +# +# @value: value to the option +# +# Returns: +# True if it sets value to the option successfully +# +# Since: 4.10.3 +## +{'command': {'class': 'GlusterGeoRep', 'name': 'geoRepConfigSet'}, + 'data': {'mastervolname': 'str', 'slavehost': 'str', 'slavevolname': 'str', 'key': 'str', 'value': 'str'}, + 'returns': 'bool'} + +## +# @GeoRepConfig: +# +# Geo replication config details. +# +# @optionname: Config option name +# +# @editable: Editable option or not +# +# @description: Option details +# +# Since: 4.10.3 +## +{'type': 'GeoRepConfig', + 'data': {'optionname': 'str', 'editable': 'bool', 'description': 'str'}} + +## +# @GlusterVolume.geoRepConfigGet: +# +# Get value of the Geo Replication config option +# +# @mastervolname: is an existing volume name in the master node +# +# @slavehost: is remote slave host name or ip +# +# @slavevolname: is an available existing volume name in the slave node +# +# @key: valid configuration option name +# +# Returns: +# The value of the Geo Replication config option +# +# Since: 4.10.3 +## +{'command': {'class': 'GlusterGeoRep', 'name': 'geoRepConfigGet'}, + 'data': {'mastervolname': 'str', 'slavehost': 'str', 'slavevolname': 'str', 'key': 'str'}, + 'returns': 'GeoRepConfig'} + +## +# @GlusterVolume.geoRepConfigDelete: +# +# Delete Geo Replication config option +# +# @mastervolname: is an existing volume name in the master node +# +# @slavehost: is remote slave host name or ip +# +# @slavevolname: is an available existing volume name in the slave node +# +# @key: valid configuration option name +# +# Returns: +# True if it deletes the option successfully +# +# Since: 4.10.3 +## +{'command': {'class': 'GlusterGeoRep', 'name': 'geoRepConfigDelete'}, + 'data': {'mastervolname': 'str', 'slavehost': 'str', 'slavevolname': 'str', 'key': 'str'}, + 'returns': 'bool'}
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4024/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3135/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3942/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3135/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3942/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4027/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3135/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3942/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4030/ : FAILURE
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)
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)
ndarshan has posted comments on this change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
Patch Set 1:
(1 comment)
.................................................... File vdsm/gluster/vdsmapi-gluster-schema.json Line 431: {'type': 'GeoRepConfig', Line 432: 'data': {'optionname': 'str', 'editable': 'bool', 'description': 'str'}} Line 433: Line 434: ## Line 435: # @GlusterGeoRep.configList: I feel "GlusterVolume.geoRepConfigList" can be used here and for configset Line 436: # Line 437: # List Geo Replication configuration Line 438: # Line 439: # @mastervolname: is an existing volume name in the master node
Bala.FA has posted comments on this change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
Patch Set 1:
(1 comment)
.................................................... File vdsm/gluster/cli.py Line 159: 'working_dir': ( Line 160: 'workingDir', Line 161: 'working directory', Line 162: GeoRepConf.READONLY)} Line 163: As this is very similar to volume set help xml, I would prefer 1) file a bug in glusterfs for geo-rep set help xml for this.
2) till the bug get fixed, have the expected xml in a file and load it at the time of module load and have a clear comment about this is temporarily
3) Use the xml object wherever applicable Line 164: Line 165: def _execGluster(cmd): Line 166: return utils.execCmd(cmd) Line 167:
Itamar Heim has posted comments on this change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
Patch Set 1:
pig
Itamar Heim has posted comments on this change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
Patch Set 1:
ping...
Itamar Heim has abandoned this change.
Change subject: gluster: Add gluster volume geo-replication configuration feature ......................................................................
Abandoned
abandoning - old. no reply. not touched for a while. please restore if relevant
vdsm-patches@lists.fedorahosted.org