Deepak C Shetty has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 1: No score
(8 inline comments)
Replied to federico's comments. Thanks federico for the quick review. Would appreciate your response to my comments/qeustions asked in response to yours.
.................................................... File vdsm/libvirtvm.py Line 991: # Determine if its network type agree
Line 1000: def blockDev(self): 2 ways to do it.
1) In _updateVolumes, i can change the order of checking... <check for networkdevice> elseif <check for blk device>, that ways if its network, blk device wont be called itself.
2) Other way is to change the blockdev property function, and check for network type, if not then call utils.....
Which one is preferable ? or should i do both ?
Line 1003: self._blockDev = utils.isBlockDevice(self.path['path']) self.path is created due to prepareImage returning 'path' as a key for drive dict. Changing the key would call for even more changes across this file, hence retained it.
Line 1993: capacity, alloc, physical = self._dom.blockInfo(d.path, 0) Hmm, i was wondering if instead of making 'path' a dict, how about introducing 'volInfo' as a new key in drive dict returned from the prepareImage ? That would avoid makign changes everywhere where .path is being used, what do you think ?
.................................................... File vdsm/storage/fileSD.py Line 43: OPTIONS = "OPTIONS" I feel keeping this generic helps, since we are in fileSD.py. Calling it gluster_options won't make sense in the generic fileSD class, hence I kept it generic. Keeping it generic also means, Gluster and later some other can use it as they need. Making it specific would mean the domain will have to do extra work to add their specific xxx_options, we can avoid that.
.................................................... File vdsm/storage/hsm.py Line 128: sd.GLUSTERFS_DOMAIN: 'posixfs' } Yes, because I want to re-use the MountConnection. In fact to put it other way.. I want to do the same stuff that PosixFS does ( mount the remotePath and create domain on it). Do you see problems with using PosixFS ?
.................................................... File vdsm/storage/volume.py Line 294: clsName, "shareVolumeRollback", [dstPath])) Sorry, did not get this comment
Line 807: def getVolumePath(self): This is not un-used.
-- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com