Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN
......................................................................
Patch Set 12: (7 inline comments)
@fsimonce, addressed your comments and posted some replies. Thanks for the
comprehensive review. Pls have a look.
Reg. the comment on the arch... is there a better way to go ?
....................................................
File vdsm/storage/glusterSD.py
Line 18: return glusterVolume.GlusterVolume
Line 19:
Line 20: @staticmethod
Line 21: def findDomainPath(sdUUID):
Line 22: for tmpSdUUID, domainPath in
fileSD.scanDomains(sd.GLUSTERSD_DIR +
Done
Line 23: "/*"):
Line 24: if tmpSdUUID == sdUUID and
mount.isMounted(os.path.join(domainPath,
Line 25: "..")):
Line 26: return domainPath
....................................................
File vdsm/storage/glusterVolume.py
Line 1: from volume import VmVolumeInfo
Line 2: import fileVolume
Line 3: from sdc import sdCache
Line 4: import string
Done
Line 5: import supervdsm as svdsm
Line 6:
Line 7:
Line 8: class GlusterVolume(fileVolume.FileVolume):
Line 15: """
Line 16: Send info to represent Gluster volume as a network block device
Line 17: """
Line 18: rpath = sdCache.produce(self.sdUUID).getRemotePath()
Line 19: rpath_list = string.rsplit(rpath, ":")
Done
Line 20: volfileServer = rpath_list[0]
Line 21: volname = rpath_list[1]
Line 22:
Line 23: # Volume transport to Libvirt transport mapping
Line 20: volfileServer = rpath_list[0]
Line 21: volname = rpath_list[1]
Line 22:
Line 23: # Volume transport to Libvirt transport mapping
Line 24: VOLUME_TRANS_MAP = {'TCP': 'tcp', 'RDMA': 'rdma'}
Right now, only this function needs it, so i kept it inside the func.
If more functions are added and they need it, then we can move it at the module
or class level as needed. I think for now its fine. Hope you agree ?
Line 25:
Line 26: # Extract the volume's transport using gluster cli
Line 27: svdsmProxy = svdsm.getProxy()
Line 28: volInfo = svdsmProxy.glusterVolumeInfo(volname)
Line 31: # Use default port
Line 32: volPort = "0"
Line 33:
Line 34: imgFilePath = self.getVolumePath()
Line 35: imgFilePath_list = string.rsplit(imgFilePath, "/")
Done
Line 36: imgFileRelPath = string.join(imgFilePath_list[4:], "/")
Line 37:
Line 38: glusterPath = volname + '/' + imgFileRelPath
Line 39:
Line 32: volPort = "0"
Line 33:
Line 34: imgFilePath = self.getVolumePath()
Line 35: imgFilePath_list = string.rsplit(imgFilePath, "/")
Line 36: imgFileRelPath = string.join(imgFilePath_list[4:], "/")
Changed this too to avoid using string module.
Line 37:
Line 38: glusterPath = volname + '/' + imgFileRelPath
Line 39:
Line 40: return {'volType': VmVolumeInfo.TYPE_NETWORK, 'path':
glusterPath,
....................................................
File vdsm/storage/volume.py
Line 868: volume to the VM in a different way than the standard 'path'
way.
Line 869: """
Line 870: # By default, send path
Line 871: return {'volType': VmVolumeInfo.TYPE_PATH,
Line 872: 'path': self.getVolumePath()}
I don't think so. It seems to be the case, if you look at it from volume.py's
perspective. But if u look at it from glusterVolume.py perspective, path
returned from getVolumePath and ['path'] returned from getVmVolumeInfo are
different. The former path is used for the domain side of VDSM, while the
latter is used to tell qemu how to access the gluster image using a gluster
specific path ( not the domain path). Till now, VDSM libvirtvm didn't have
support for network based device support, so path was same ( domain and qemu
side). But with gluster, they are diff. In fact the ['path'] returned from
prepareImage should be more aptly called volDomPath and ['path'] returned from
getVmVolumeInfo is for VM purpose only.
In other words, if any derived class overrides getVmVolumeInfo and sends a diff
['path'] which is what glusterVolume.py does, then the volume.py's
self.volumePath is not accessible, post return from prepareImage.
getVolumePath is used to represent the volume's path from a domain point of
view.
getVmVolumeInfo['path'] is used to represent the volume's path from a VM
(libvirtvm) perspective.
Let me know if you still think its redundant ?
Line 873:
Line 874: def getMetaParam(self, key):
Line 875: """
Line 876: Get a value of a specific key
--
To view, visit http://gerrit.ovirt.org/6856
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ac37da88625f20d148beaf53bb6371c15b33ad7
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Deepak C Shetty <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Bala.FA <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Deepak C Shetty <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Royce Lv <[email protected]>
Gerrit-Reviewer: Ryan Harper <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
