Change in vdsm[master]: Support for GLUSTERFS_DOMAIN

deepakcs at linux.vnet.ibm.com deepakcs at linux.vnet.ibm.com
Mon Jan 14 09:43:15 UTC 2013


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 <deepakcs at linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Deepak C Shetty <deepakcs at linux.vnet.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim at redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce at linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh at us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list