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 <dnarayan(a)redhat.com>
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 <dnarayan(a)redhat.com>
Gerrit-Reviewer: Aravinda VK <avishwan(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: ndarshan <dnarayan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes