Bala.FA has posted comments on this change. Change subject: gluster Geo-replication: command to create and delete a geo-replication session ......................................................................
Patch Set 10: Code-Review-1 (9 comments) .................................................... Commit Message Line 3: AuthorDate: 2013-08-05 09:59:19 +0530 Line 4: Commit: ndarshan <[email protected]> Line 5: CommitDate: 2013-08-20 14:21:08 +0530 Line 6: Line 7: gluster Geo-replication: command to create and delete a geo-replication session 1. shorten the prefix by 'gluster:' 2. split this patch into two a) for create b) for delete Line 8: Line 9: geo-replication create first executes the command to create a public key file which Line 10: will have public keys of all the hosts of source cluster.Next it pushes the file to Line 11: destination cluster and with password less communication geo-replication session is .................................................... File client/vdsClientGluster.py Line 440: Line 441: def do_glusterVolumeGeoRepDelete(self, args): Line 442: params = self._eqSplit(args) Line 443: try: Line 444: masterVolName = params.get('masterVolName', '') Use volumeName instead of masterVolName as its all about volume available locally. Line 445: slaveHost = params.get('slaveHost', '') Line 446: slaveVolName = params.get('slaveVolName', '') Line 447: except: Line 448: raise ValueError Line 442: params = self._eqSplit(args) Line 443: try: Line 444: masterVolName = params.get('masterVolName', '') Line 445: slaveHost = params.get('slaveHost', '') Line 446: slaveVolName = params.get('slaveVolName', '') can we have remoteHost/remoteVolumeName than slaveHost/slaveVolName? Line 447: except: Line 448: raise ValueError Line 449: if not (masterVolName and slaveHost and slaveVolName): Line 450: raise ValueError .................................................... File vdsm/gluster/api.py Line 287: status = self.svdsmProxy.glusterServicesGet(serviceNames) Line 288: return {'services': status} Line 289: Line 290: @exportAsVerb Line 291: def volumeGeoRepCreate(self, masterVolName, slaveHost, 1. I think this is geo-rep session create. Please add session in the name 2. have volumeName, remoteHost, remoteVolume etc than master/slave 3. functionality of this verb is a) check passwordless-ssh is there between localhost and remoteHost. This requires http://gerrit.ovirt.org/8375 b) if false return error else continue c) call cli:gsec_create d) if success, call glusterVolumeGeoRepCreate Line 292: slaveVolName, force=False, options=None): Line 293: status = self.svdsmProxy.glusterVolumeGeoRepCreate(masterVolName, Line 294: slaveHost, Line 295: slaveVolName, Line 292: slaveVolName, force=False, options=None): Line 293: status = self.svdsmProxy.glusterVolumeGeoRepCreate(masterVolName, Line 294: slaveHost, Line 295: slaveVolName, Line 296: force=False) just pass force, no default to be set here Line 297: return {'geo-rep': status} Line 298: Line 299: @exportAsVerb Line 300: def volumeGeoRepDelete(self, masterVolName, slaveHost, Line 296: force=False) Line 297: return {'geo-rep': status} Line 298: Line 299: @exportAsVerb Line 300: def volumeGeoRepDelete(self, masterVolName, slaveHost, same here Line 301: slaveVolName, options=None): Line 302: status = self.svdsmProxy.glusterVolumeGeoRepDelete(masterVolName, Line 303: slaveHost, Line 304: slaveVolName) .................................................... File vdsm/gluster/cli.py Line 905: if rc: Line 906: raise ge.GlusterGeoRepPublicKeyFileCreationFailedException(rc, Line 907: out, err) Line 908: path = out.partition('/') Line 909: if not os.path.isfile('/' + path): 1. make this function as one-to-one mapping a) change name of the function to systemExecuteGsecCreate b) make this function available publically 2. I would suggest to return well known filepath than parsing stdout. Also please open a bug in glusterfs to return xml output for this command as tracker. Line 910: raise ge.GlusterGeoRepPublicKeyFileDoesNotExistException(rc, out, err) Line 911: else: Line 912: return True Line 913: .................................................... File vdsm/gluster/exception.py Line 491: code = 4560 Line 492: message = "Gluster Geo-Replication Exception" Line 493: Line 494: Line 495: class GlusterGeoRepPublicKeyFileCreateFailedException(GlusterGeoRepException GlusterGeoRepSecCreateFailedException may be? Line 496: ): Line 497: code = 4561 Line 498: message = "Creation of public key file failed" Line 499: Line 497: code = 4561 Line 498: message = "Creation of public key file failed" Line 499: Line 500: Line 501: class GlusterGeoRepPublicKeyFileDoesNotExistException(GlusterGeoRepException): I don't think this exception needed Line 502: code = 4562 Line 503: message = "Public key file does not exist" Line 504: Line 505: -- To view, visit http://gerrit.ovirt.org/17644 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8c979a89ce11a1622819c474b59dcf088733594 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ndarshan <[email protected]> Gerrit-Reviewer: Aravinda VK <[email protected]> Gerrit-Reviewer: Bala.FA <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Timothy Asir <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: ndarshan <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
