Deepak C Shetty has uploaded a new change for review.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
[RFC] Support for GLUSTERFS_DOMAIN
This patch introduces a new storage domain of type GLUSTERFS_DOMAIN, which uses gluster as the storage backend.
In GLUSTERFS_DOMAIN, vdsm creates the storage domain by mounting the gluster volume (akin to nfs mounting export path). VMs created using this domain exploit the QEMU's gluster block backend. Instead of accessing the vmdisk as a file path, it accesses the vmdisk as a network disk device, served by gluster server/volume.
This patch attempts to re-use nfsSD core logic to support domain of type GLUSTERFS_DOMAIN.
Also has some changes to pass pep8 checks
Change-Id: I9ac37da88625f20d148beaf53bb6371c15b33ad7 Signed-off-by: Deepak C Shetty deepakcs@linux.vnet.ibm.com --- M vdsm.spec.in M vdsm/API.py M vdsm/BindingXMLRPC.py M vdsm/libvirtvm.py M vdsm/storage/Makefile.am M vdsm/storage/fileSD.py A vdsm/storage/glusterVolume.py M vdsm/storage/hsm.py M vdsm/storage/nfsSD.py M vdsm/storage/sd.py M vdsm/storage/volume.py M vdsm/vm.py 12 files changed, 124 insertions(+), 34 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/6856/1 -- To view, visit http://gerrit.ovirt.org/6856 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I9ac37da88625f20d148beaf53bb6371c15b33ad7 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com
Deepak C Shetty has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 1: Verified
I have verified till the point the libvirt XML is created. libvirt XML is still under works. Haven't yet tested end2end, since qemu and libvirt changes for support gluster as a network disk device is WIP.
Once libvirt and qemu support is frozen, i can post a final patch.
-- 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
Federico Simoncelli has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(11 inline comments)
.................................................... File vdsm/libvirtvm.py Line 88: self._vm._dom.blockInfo(vmDrive.path, 0) Did you make sure that this still works with your changes?
Line 991: # Determine if its network type For consistency I'd make this a property as blockDev, something like:
@property def networkDev(self): return self.path['volType'] == "network"
Line 1000: def blockDev(self): We might want to take advantage of volType to skip the utils.isBlockDevice check if we already know that it's not a block device.
Line 1003: self._blockDev = utils.isBlockDevice(self.path['path']) This looks like a repetition:
self.path['path']
You may want to consider an other name, eg: self.volInfo['path']
Line 1521: if drv.path == diskParams['path']: Did you make sure that this still works with your changes?
Line 1875: mergeStatus['path'] = mergeDrive.path Did you make sure that this still works with your changes?
Line 1993: capacity, alloc, physical = self._dom.blockInfo(d.path, 0) Did you make sure that this still works with your changes? ...and so on for all the other .path present in this file and in vm.py (if any).
.................................................... File vdsm/storage/fileSD.py Line 43: OPTIONS = "OPTIONS" I'm not sure if we want to keep this generic as "OPTIONS" or make it specific for what you need. We'll reevaluate later.
.................................................... File vdsm/storage/hsm.py Line 128: sd.GLUSTERFS_DOMAIN: 'posixfs' } Are you sure that this should be 'posixfs'?
.................................................... File vdsm/storage/volume.py Line 294: clsName, "shareVolumeRollback", [dstPath])) Unrelated. As all the other ones.
Line 807: def getVolumePath(self): If this became unused just squash it with getVmVolumeInfo.
-- 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
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
Deepak C Shetty has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 1: (8 inline comments)
I have posted v2. Pls provide your review comments.
.................................................... File vdsm/libvirtvm.py Line 88: self._vm._dom.blockInfo(vmDrive.path, 0) Done
Line 991: # Determine if its network type Done
Line 1000: def blockDev(self): Done
Line 1003: self._blockDev = utils.isBlockDevice(self.path['path']) Done
Line 1521: if drv.path == diskParams['path']: Done
Line 1875: mergeStatus['path'] = mergeDrive.path Done
Line 1993: capacity, alloc, physical = self._dom.blockInfo(d.path, 0) Done. For all the .path changes, I avoided them, by defining new key in drive dict by name 'vmVolInfo'. Posted v2 with the same.
.................................................... File vdsm/storage/volume.py Line 294: clsName, "shareVolumeRollback", [dstPath])) Do you mean this change is unrelated to the patch. Yes, but i did it for pep8 compliance
-- 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 Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/27/
-- 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: 2 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 Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 2: Fails
Build Failed
http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/27/ : ABORTED
-- 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: 2 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 Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 2: No score
Build Started http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/394/
-- 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: 2 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 Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/394/ : ABORTED
-- 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: 2 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 Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(9 inline comments)
Few things to revise.
.................................................... File vdsm/libvirtvm.py Line 982: self.truesize = int(kwargs.get('truesize', '0')) Line 983: self.apparentsize = int(kwargs.get('apparentsize', '0')) Line 984: self.name = self._makeName() Line 985: Line 986: if self.device in ("cdrom", "floppy") or self.networkDev: Even if it works and it makes sense... this looks misplaced (mixing the device type with the device backend). Try to move this in blockDev. Line 987: self._blockDev = False Line 988: else: Line 989: self._blockDev = None Line 990:
Line 992: Line 993: @property Line 994: def networkDev(self): Line 995: try: Line 996: res = self.vmVolInfo['volType'] == "network" return self.vmVolInfo['volType'] == "network" Line 997: except AttributeError: Line 998: # To handle legacy and removable drives. Line 999: res = False Line 1000: return res
Line 995: try: Line 996: res = self.vmVolInfo['volType'] == "network" Line 997: except AttributeError: Line 998: # To handle legacy and removable drives. Line 999: res = False return False Line 1000: return res Line 1001: Line 1002: @property Line 1003: def blockDev(self):
Line 1000: return res Line 1001: Line 1002: @property Line 1003: def blockDev(self): Line 1004: if self._blockDev is None: maybe you can put your network check here. Line 1005: try: Line 1006: self._blockDev = utils.isBlockDevice(self.path) Line 1007: except: Line 1008: self.log.debug("Unable to determine if the path '%s' is a "
.................................................... File vdsm/storage/fileSD.py Line 39: import supervdsm Line 40: import mount Line 41: Line 42: REMOTE_PATH = "REMOTE_PATH" Line 43: OPTIONS = "OPTIONS" Probably REMOTE_INFO is better. Line 44: Line 45: FILE_SD_MD_FIELDS = sd.SD_MD_FIELDS.copy() Line 46: # TBD: Do we really need this key? Line 47: FILE_SD_MD_FIELDS[REMOTE_PATH] = (str, str)
Line 121: self.imageGarbageCollector() Line 122: self._registerResourceNamespaces() Line 123: Line 124: @classmethod Line 125: def _prepareMetadata(cls, domPath, sdUUID, domainName, domClass, remotePath, storageType, version, Since you're at it you could make this line pep8 compliant. Line 126: options): Line 127: """ Line 128: Prepare all domain's special volumes and metadata Line 129: """
.................................................... File vdsm/storage/hsm.py Line 2757: }) Line 2758: Line 2759: chain.append(volInfo) Line 2760: Line 2761: return {'path': chain[-1]['path'], 'vmVolInfo': chain[-1]['vmVolInfo'], 'chain': chain} what about only 'info' so that it can be reused for any kind of data. Line 2762: Line 2763: @public Line 2764: def teardownImage(self, sdUUID, spUUID, imgUUID, volUUID=None): Line 2765: """
.................................................... File vdsm/storage/volume.py Line 290: Line 291: try: Line 292: vars.task.pushRecovery( Line 293: task.Recovery("Share volume rollback: %s" % dstPath, clsModule, Line 294: clsName, "shareVolumeRollback", [dstPath])) Since you're not already touching these lines even if it's pep8 you shouldn't be fixing them. You can send a separate patch to fix all the pep8 problems. But as long as I see volume.py should be already passing pep8. Line 295: Line 296: self._share(dstImgPath) Line 297: Line 298: except Exception, e:
Line 819: volume to the VM in a different way than the standard 'path' way. Line 820: """ Line 821: # By default, send path Line 822: vmVolInfo = {'volType': 'path', 'path': self.getVolumePath()} Line 823: return vmVolInfo return {'volType': 'path', 'path': self.getVolumePath()} Line 824: Line 825: def getMetaParam(self, key): Line 826: """ Line 827: 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Shu Ming has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 2: (1 inline comment)
.................................................... File vdsm/storage/glusterVolume.py Line 39: 'protocol': 'gluster', 'volPort': opt_volport, Line 40: 'volTransport': opt_voltrans, Line 41: 'volfileServer': volfileServer} Line 42: Line 43: return vmVolInfo You may return {'volType': 'network', 'path': glusterPath, 'protocol': 'gluster', 'volPort': opt_volport, 'volTransport': opt_voltrans, 'volfileServer': volfileServer}
-- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 2: (1 inline comment)
.................................................... File vdsm/storage/glusterVolume.py Line 39: 'protocol': 'gluster', 'volPort': opt_volport, Line 40: 'volTransport': opt_voltrans, Line 41: 'volfileServer': volfileServer} Line 42: Line 43: return vmVolInfo I think its the same, does it really matter ? i feel what I have done is more legible :)
-- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 2: (10 inline comments)
.................................................... File vdsm/libvirtvm.py Line 982: self.truesize = int(kwargs.get('truesize', '0')) Line 983: self.apparentsize = int(kwargs.get('apparentsize', '0')) Line 984: self.name = self._makeName() Line 985: Line 986: if self.device in ("cdrom", "floppy") or self.networkDev: Done. Line 987: self._blockDev = False Line 988: else: Line 989: self._blockDev = None Line 990:
Line 992: Line 993: @property Line 994: def networkDev(self): Line 995: try: Line 996: res = self.vmVolInfo['volType'] == "network" Done Line 997: except AttributeError: Line 998: # To handle legacy and removable drives. Line 999: res = False Line 1000: return res
Line 995: try: Line 996: res = self.vmVolInfo['volType'] == "network" Line 997: except AttributeError: Line 998: # To handle legacy and removable drives. Line 999: res = False Done Line 1000: return res Line 1001: Line 1002: @property Line 1003: def blockDev(self):
Line 1000: return res Line 1001: Line 1002: @property Line 1003: def blockDev(self): Line 1004: if self._blockDev is None: Done Line 1005: try: Line 1006: self._blockDev = utils.isBlockDevice(self.path) Line 1007: except: Line 1008: self.log.debug("Unable to determine if the path '%s' is a "
.................................................... File vdsm/storage/fileSD.py Line 39: import supervdsm Line 40: import mount Line 41: Line 42: REMOTE_PATH = "REMOTE_PATH" Line 43: OPTIONS = "OPTIONS" Done Line 44: Line 45: FILE_SD_MD_FIELDS = sd.SD_MD_FIELDS.copy() Line 46: # TBD: Do we really need this key? Line 47: FILE_SD_MD_FIELDS[REMOTE_PATH] = (str, str)
Line 121: self.imageGarbageCollector() Line 122: self._registerResourceNamespaces() Line 123: Line 124: @classmethod Line 125: def _prepareMetadata(cls, domPath, sdUUID, domainName, domClass, remotePath, storageType, version, Done Line 126: options): Line 127: """ Line 128: Prepare all domain's special volumes and metadata Line 129: """
.................................................... File vdsm/storage/glusterVolume.py Line 39: 'protocol': 'gluster', 'volPort': opt_volport, Line 40: 'volTransport': opt_voltrans, Line 41: 'volfileServer': volfileServer} Line 42: Line 43: return vmVolInfo Done
.................................................... File vdsm/storage/hsm.py Line 2757: }) Line 2758: Line 2759: chain.append(volInfo) Line 2760: Line 2761: return {'path': chain[-1]['path'], 'vmVolInfo': chain[-1]['vmVolInfo'], 'chain': chain} Done. 'vmVolInfo': chain[-1]['vmVolInfo'] --> 'info':chain[-1]['volInfo'] Made associated changes in prepareImage and clientIF.py - Done This also causes class Drive in libvirtvm.py attribute name change.... drive['vmVolInfo'] --> drive['volumeInfo'] and is related changes -- Done Line 2762: Line 2763: @public Line 2764: def teardownImage(self, sdUUID, spUUID, imgUUID, volUUID=None): Line 2765: """
.................................................... File vdsm/storage/volume.py Line 290: Line 291: try: Line 292: vars.task.pushRecovery( Line 293: task.Recovery("Share volume rollback: %s" % dstPath, clsModule, Line 294: clsName, "shareVolumeRollback", [dstPath])) Well it was not, hence i did the pep8 changes. Reverted them back in the next patch. Done. Line 295: Line 296: self._share(dstImgPath) Line 297: Line 298: except Exception, e:
Line 819: volume to the VM in a different way than the standard 'path' way. Line 820: """ Line 821: # By default, send path Line 822: vmVolInfo = {'volType': 'path', 'path': self.getVolumePath()} Line 823: return vmVolInfo Done Line 824: Line 825: def getMetaParam(self, key): Line 826: """ Line 827: 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 2: (1 inline comment)
.................................................... File vdsm/storage/volume.py Line 290: Line 291: try: Line 292: vars.task.pushRecovery( Line 293: task.Recovery("Share volume rollback: %s" % dstPath, clsModule, Line 294: clsName, "shareVolumeRollback", [dstPath])) I also undid all the unrelated pep8 changes.. will be posting next patch Line 295: Line 296: self._share(dstImgPath) Line 297: Line 298: except Exception, e:
-- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/650/ : SUCCESS
-- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 3: Verified
This patch has been verified end2end using the proposed libvirt (WIP) and QEMU changes on my test machine. Able to create a VM using GLUSTERFS_DOMAIN as the storage domain.
-- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(2 inline comments)
.................................................... Commit Message Line 3: AuthorDate: 2012-07-31 18:41:21 +0530 Line 4: Commit: Deepak C Shetty deepakcs@linux.vnet.ibm.com Line 5: CommitDate: 2012-08-24 14:22:57 +0530 Line 6: Line 7: [RFC] Support for GLUSTERFS_DOMAIN You can leave the [RFC] at this stage. Line 8: Line 9: This patch introduces a new storage domain of type Line 10: GLUSTERFS_DOMAIN, which uses gluster as the storage backend. Line 11:
.................................................... File vdsm/libvirtvm.py Line 999: return False Line 1000: Line 1001: @property Line 1002: def blockDev(self): Line 1003: if self._blockDev is None and not self.networkDev: With this check you risk to return None (self._blockDev is None by default). Line 1004: try: Line 1005: self._blockDev = utils.isBlockDevice(self.path) Line 1006: except: Line 1007: self.log.debug("Unable to determine if the path '%s' is a "
-- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: [RFC] Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 3: No score
(2 inline comments)
.................................................... Commit Message Line 3: AuthorDate: 2012-07-31 18:41:21 +0530 Line 4: Commit: Deepak C Shetty deepakcs@linux.vnet.ibm.com Line 5: CommitDate: 2012-08-24 14:22:57 +0530 Line 6: Line 7: [RFC] Support for GLUSTERFS_DOMAIN Done Line 8: Line 9: This patch introduces a new storage domain of type Line 10: GLUSTERFS_DOMAIN, which uses gluster as the storage backend. Line 11:
.................................................... File vdsm/libvirtvm.py Line 999: return False Line 1000: Line 1001: @property Line 1002: def blockDev(self): Line 1003: if self._blockDev is None and not self.networkDev: My bad. Done. Line 1004: try: Line 1005: self._blockDev = utils.isBlockDevice(self.path) Line 1006: except: Line 1007: self.log.debug("Unable to determine if the path '%s' is a "
-- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has abandoned this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 4: Abandoned
Sorry for the bad post. Forgot to fix a pygflakes issue in nfSD.py, will post a new one.
-- To view, visit http://gerrit.ovirt.org/6856 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: abandon Gerrit-Change-Id: I9ac37da88625f20d148beaf53bb6371c15b33ad7 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has restored this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 4: Restored
Didn't knew abandoning a patchset abandons the whole change itself !... Restoring the change, so that I can submit v5.
-- To view, visit http://gerrit.ovirt.org/6856 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: restore Gerrit-Change-Id: I9ac37da88625f20d148beaf53bb6371c15b33ad7 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/659/ : SUCCESS
-- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 4:
Pls ignore this patchset 4, forgot to fix pyflakes issue before submitting. Not sure how to abandon only patchset 4 in gerrit, hence this msg.
Posted patchset 5, hopefully the final !
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 5: Verified
Tested end2end with proposed libvirt (WIP) and related QEMU changes. Able to create a VM using GLUSTERFS_DOMAIN as the storage domain type in VDSM.
-- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Saggi Mizrahi has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 5: I would prefer that you didn't submit this
(4 inline comments)
I also don't like you adding a another metadata entry. Is it really necessary, I remember talking to you about how that is problematic.
.................................................... Commit Message Line 18: Line 19: This patch attempts to re-use nfsSD core logic to support Line 20: domain of type GLUSTERFS_DOMAIN. Line 21: Line 22: v2: No need to keep the changelog in the commit message Line 23: Using different key drive['vmVolInfo'] instead of 'path' Line 24: Addressed most of federico's comments Line 25: v3: Line 26: Addressed federico's comments
.................................................... File vdsm/storage/blockSD.py Line 469: t = time.time() Line 470: misc.readfile(lvm.lvPath(self.sdUUID, sd.METADATA), 4096) Line 471: return time.time() - t Line 472: Line 473: How is removing that method related? Line 474: def getVolumeClass(self): Line 475: """ Line 476: Return a type specific volume generator object Line 477: """
.................................................... File vdsm/storage/hsm.py Line 124: # FCP domain shouldn't even be on the list but VDSM use to just Line 125: # accept this type as iscsi so we are stuck with it Line 126: sd.FCP_DOMAIN: 'iscsi', Line 127: sd.POSIXFS_DOMAIN: 'posixfs', Line 128: sd.GLUSTERFS_DOMAIN: 'posixfs' } Why is this here? Line 129: Line 130: def _BCInitiatorNameResolve(ifaceName): Line 131: if not ifaceName: Line 132: return iscsi.IscsiInterface('default')
.................................................... File vdsm/storage/nfsSD.py Line 122: def getVolumeClass(self): Line 123: """ Line 124: Return a type specific volume generator object Line 125: """ Line 126: if self.getMetaParam(sd.DMDK_TYPE) == sd.GLUSTERFS_DOMAIN: Use an accessor Line 127: return glusterVolume.GlusterVolume Line 128: else: Line 129: return fileSD.FileStorageDomain.getVolumeClass(self) Line 130:
-- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 5: No score
(4 inline comments)
@Saggi, Per our IRC chat, I discussed with the gluster folks and it seems I can live w/o worrying about transport and port as inputs, thus there shouldn't be the need to persist them, and hence no need of REMOTE_INFO MD key. I will send a updated patch soon.
.................................................... Commit Message Line 18: Line 19: This patch attempts to re-use nfsSD core logic to support Line 20: domain of type GLUSTERFS_DOMAIN. Line 21: Line 22: v2: Done Line 23: Using different key drive['vmVolInfo'] instead of 'path' Line 24: Addressed most of federico's comments Line 25: v3: Line 26: Addressed federico's comments
.................................................... File vdsm/storage/blockSD.py Line 469: t = time.time() Line 470: misc.readfile(lvm.lvPath(self.sdUUID, sd.METADATA), 4096) Line 471: return time.time() - t Line 472: Line 473: In discussion with federico, it was decided to unify the produceVolume, hence this was removed. Its related because for the newly introduced glusterVolume class, won't get instantiated, if called via produceVolume, as produceVolume was not asking getVolumeClass, it was hardcoded. So when I changed produceVolume to work via getVolumeClass, it made sense to unify them too. Line 474: def getVolumeClass(self): Line 475: """ Line 476: Return a type specific volume generator object Line 477: """
.................................................... File vdsm/storage/hsm.py Line 124: # FCP domain shouldn't even be on the list but VDSM use to just Line 125: # accept this type as iscsi so we are stuck with it Line 126: sd.FCP_DOMAIN: 'iscsi', Line 127: sd.POSIXFS_DOMAIN: 'posixfs', Line 128: sd.GLUSTERFS_DOMAIN: 'posixfs' } Because I want to re-use the nfsSD core logic, that is done in PosixFS. Line 129: Line 130: def _BCInitiatorNameResolve(ifaceName): Line 131: if not ifaceName: Line 132: return iscsi.IscsiInterface('default')
.................................................... File vdsm/storage/nfsSD.py Line 122: def getVolumeClass(self): Line 123: """ Line 124: Return a type specific volume generator object Line 125: """ Line 126: if self.getMetaParam(sd.DMDK_TYPE) == sd.GLUSTERFS_DOMAIN: Accessor ? Line 127: return glusterVolume.GlusterVolume Line 128: else: Line 129: return fileSD.FileStorageDomain.getVolumeClass(self) Line 130:
-- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 5: (1 inline comment)
.................................................... File vdsm/storage/nfsSD.py Line 122: def getVolumeClass(self): Line 123: """ Line 124: Return a type specific volume generator object Line 125: """ Line 126: if self.getMetaParam(sd.DMDK_TYPE) == sd.GLUSTERFS_DOMAIN: Done Line 127: return glusterVolume.GlusterVolume Line 128: else: Line 129: return fileSD.FileStorageDomain.getVolumeClass(self) Line 130:
-- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 6: Verified
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 6:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/754/ : SUCCESS
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 6: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/storage/nfsSD.py Line 121: def getVolumeClass(self): Line 122: """ Line 123: Return a type specific volume generator object Line 124: """ Line 125: if self.getType() == sd.GLUSTERFS_DOMAIN: Please move this to a glusterSD module (GlusterStorageDomain). Line 126: return glusterVolume.GlusterVolume Line 127: else: Line 128: return fileSD.FileStorageDomain.getVolumeClass(self) Line 129:
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Itamar Heim has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 6:
what are pros/cons of using the block vs. file based approach (in any case, i assume we need this type of storage domain for doing the gluster part directly from qemu).
expected engine changes to support this? feature page?
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Bala.FA has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 6: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/storage/glusterVolume.py Line 21: VOLUME_TRANS_MAP = {'TCP': 'socket', 'RDMA': 'rdma'} Line 22: Line 23: # Extract the volume's transport using gluster cli Line 24: svdsmProxy = svdsm.getProxy() Line 25: volInfo = svdsmProxy.glusterVolumeInfo(volname) glusterVolumeInfo (and other gluster methods) are available only if vdsm-gluster is available in the system. If no gluster module, there will be an exception thrown here.
Its better to check gluster module availability something like
try: from gluster import cli as gcli _glusterEnabled = True except ImportError: _glusterEnabled = False Line 26: volTrans = VOLUME_TRANS_MAP[volInfo[volname]['transportType'][0]] Line 27: Line 28: # Use default port Line 29: volPort = "0"
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 6: No score
@Itamar, By "block Vs file" if you mean block backend of qemu Vs using the image as a file, the adv is that we bypass the fuse overhead present in the latter case.
I am not the right person for engine side of changes. I hope people like Shireesh who have done lot of Gluster related oVirt work will be better choice ?
I will open feature wiki page once my VDSM gluster domain patch is thru.
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 6: (2 inline comments)
.................................................... File vdsm/storage/glusterVolume.py Line 21: VOLUME_TRANS_MAP = {'TCP': 'socket', 'RDMA': 'rdma'} Line 22: Line 23: # Extract the volume's transport using gluster cli Line 24: svdsmProxy = svdsm.getProxy() Line 25: volInfo = svdsmProxy.glusterVolumeInfo(volname) As discussed on IRC, i will post the next patch w/o considering this. There are packaging issues that need to be resolved which fsimonce prefered to discuss over the next patch. Line 26: volTrans = VOLUME_TRANS_MAP[volInfo[volname]['transportType'][0]] Line 27: Line 28: # Use default port Line 29: volPort = "0"
.................................................... File vdsm/storage/nfsSD.py Line 121: def getVolumeClass(self): Line 122: """ Line 123: Return a type specific volume generator object Line 124: """ Line 125: if self.getType() == sd.GLUSTERFS_DOMAIN: Done. This involved creating new domain, connection classes and changes in cache module to be able to find this domain. Line 126: return glusterVolume.GlusterVolume Line 127: else: Line 128: return fileSD.FileStorageDomain.getVolumeClass(self) Line 129:
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 7: Verified
(1 inline comment)
Veriried end2end by creating a a VM backed by Gluster domain.
.................................................... File vdsm/storage/glusterVolume.py Line 20: # Volume transport to Libvirt transport mapping Line 21: VOLUME_TRANS_MAP = {'TCP': 'socket', 'RDMA': 'rdma'} Line 22: Line 23: # Extract the volume's transport using gluster cli Line 24: # FIXME: How to handle the vdsm-gluster rpm dependency ? Based on the irc discussion, there needs to be some agreement on how to handle the absence of vdsm-gluster rpm. Per fsimonce, it was suggested to move cli and exception py files from vdsm-gluster to vdsm generic rpm. I am willing to make the changes based on what the community agrees to, thus the FIXME in the code above. Line 25: svdsmProxy = svdsm.getProxy() Line 26: volInfo = svdsmProxy.glusterVolumeInfo(volname) Line 27: volTrans = VOLUME_TRANS_MAP[volInfo[volname]['transportType'][0]] Line 28:
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Igor Lvovsky has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 7: (1 inline comment)
.................................................... File vdsm/storage/volume.py Line 823: Derived classes can use this if they want to represent the Line 824: volume to the VM in a different way than the standard 'path' way. Line 825: """ Line 826: # By default, send path Line 827: return {'volType': 'path', 'path': self.getVolumePath()} what is it means: 'volType': 'path' ? Line 828: Line 829: def getMetaParam(self, key): Line 830: """ Line 831: 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 7: No score
(1 inline comment)
.................................................... File vdsm/storage/volume.py Line 823: Derived classes can use this if they want to represent the Line 824: volume to the VM in a different way than the standard 'path' way. Line 825: """ Line 826: # By default, send path Line 827: return {'volType': 'path', 'path': self.getVolumePath()} It means that the volume is represented by filesystem path, versus network type Its equivalent to libvirt's <disk type=file> Vs <disk type=network> Line 828: Line 829: def getMetaParam(self, key): Line 830: """ Line 831: 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 7: (1 inline comment)
In general it looks good... a minor comment.
.................................................... File vdsm/storage/volume.py Line 823: Derived classes can use this if they want to represent the Line 824: volume to the VM in a different way than the standard 'path' way. Line 825: """ Line 826: # By default, send path Line 827: return {'volType': 'path', 'path': self.getVolumePath()} Please find a suitable name/place for enumerating those macros eg:
class VolumeInfo(object): TYPE_PATH = "path" ...
So that then you can use:
return {'volType': VolumeInfo.TYPE_PATH, 'path': self.getVolumePath()}
etc... Line 828: Line 829: def getMetaParam(self, key): Line 830: """ Line 831: 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 7: I would prefer that you didn't submit this
(2 inline comments)
.................................................... File vdsm/storage/blockSD.py Line 469: t = time.time() Line 470: misc.readfile(lvm.lvPath(self.sdUUID, sd.METADATA), 4096) Line 471: return time.time() - t Line 472: Line 473: this change does not seems strictly gluster-related. Would you split it into a separate patch, explaining its logic and correctness? Line 474: def getVolumeClass(self): Line 475: """ Line 476: Return a type specific volume generator object Line 477: """
.................................................... File vdsm/storage/glusterVolume.py Line 20: # Volume transport to Libvirt transport mapping Line 21: VOLUME_TRANS_MAP = {'TCP': 'socket', 'RDMA': 'rdma'} Line 22: Line 23: # Extract the volume's transport using gluster cli Line 24: # FIXME: How to handle the vdsm-gluster rpm dependency ? The thing is that I would not like vdsm proper to have a hard requirement for gluster. We could move the gluster/cli.py "binding" into vdsm, but we must allow, and gracefully fail, in case gluster is not found during runtime.
A better approach would have been to make a modular framework for storage domains. But I do not have a mental blueprint for this... Line 25: svdsmProxy = svdsm.getProxy() Line 26: volInfo = svdsmProxy.glusterVolumeInfo(volname) Line 27: volTrans = VOLUME_TRANS_MAP[volInfo[volname]['transportType'][0]] Line 28:
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 7:
Regarding the packaging, in a different patch bring the gluster cli/exceptions into vdsm (modify the server part accordingly). After that is acked, rebase this patch on that.
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 7:
@Federico, http://gerrit.ovirt.org/#/c/8033/ patch posted to move cli and exception.py from vdsm-gluster to vdsm rpm. thanks.
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 8: Fails
This patchset 8 is posted after splitting the prev patchset (no. 7) into multiple different patches, as requested by dan, federico and saggie. See the parent patche(s) trail for the dep. patches.
This patch is on top of the patch which moves gluster/cli and gluster/exception.py from vdsm-gluster to vdsm rpm.
I am putting this -1verify fails, because it works when vdsm-gluster rpm is installed, but gives some exception as below, when vdsm-gluster is not installed.
Thread-49::ERROR::2012-09-21 18:43:28,715::task::833::TaskManager.Task::(_setError) Task=`1a3e49a3-fe55-4c87-b0ce-10a2b6731bd5`::Unexpected error Traceback (most recent call last): File "/usr/share/vdsm/storage/task.py", line 840, in _run return fn(*args, **kargs) File "/usr/share/vdsm/logUtils.py", line 38, in wrapper res = f(*args, **kwargs) File "/usr/share/vdsm/storage/hsm.py", line 2769, in prepareImage 'vmVolInfo': vol.getVmVolumeInfo()} File "/usr/share/vdsm/storage/glusterVolume.py", line 26, in getVmVolumeInfo volInfo = svdsmProxy.glusterVolumeInfo(volname) File "/usr/share/vdsm/supervdsm.py", line 67, in __call__ return callMethod() File "/usr/share/vdsm/supervdsm.py", line 64, in <lambda> getattr(self._supervdsmProxy._svdsm, self._funcName)(*args, AttributeError: 'AutoProxy[instance]' object has no attribute 'glusterVolumeInfo'
I am still investigating this issue.
I wanted to post this patch for review comments from the reviewers, so that they can see the patch post the split, while I investigate on the above excp.
thanx, deepak
-- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 8: No score
The above RPM dep issue, now stands resolved. Have posted patchset 3 in change # 8033 for the same.
Will post next patchset for this change, incorporating federico's comments.
-- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 8: Verified
Pls ignore prev. comment. Not posting next patch. This patchset 8 is the latest patchset, that has been verified to work fine with the below dep. patches...
http://gerrit.ovirt.org/#/c/8115/ - Unify produceVolume http://gerrit.ovirt.org/#/c/8085/ - Introduce getMountPoint for NfsStorageDomain http://gerrit.ovirt.org/#/c/8033/ - Move gluster/{cli,exception,hostname,__init__}.py to vdsm rpm
Good case:
I was able to create a GLUSTERFS_DOMAIN and create a VM off it, without vdsm-gluster rpm installed, but /usr/sbin/gluster being valid.
Then, I removed /usr/sbin/gluster and ran the script to create GLUSTERFS_DOMAIN, which does not create the VM, and gives the below exceptions in vdsm.log...
Thread-51::INFO::2012-09-25 12:33:19,430::logUtils::37::dispatcher::(wrapper) Run and protect: prepareImage(sdUUID='a1f5f2ad-76a4-49f0-9206-3aaddfb5ae3b', spUUID='c3ad1a35-c05c-4626-82a3-9fc1c46af41a', imgUUID='28c86b7d-a96f-4df9-b540-d72e72108c01', volUUID='f0b847a2-3adb-4fd9-bd34-7d96295857b7') Thread-51::DEBUG::2012-09-25 12:33:19,431::resourceManager::175::ResourceManager.Request::(__init__) ResName=`Storage.a1f5f2ad-76a4-49f0-9206-3aaddfb5ae3b`ReqID=`7f60a01e-b773-45a2-98bb-6d9ddb7d571a`::Request was made in '/usr/share/vdsm/storage/resourceManager.py' line '485' at 'registerResource' Thread-51::DEBUG::2012-09-25 12:33:19,431::resourceManager::486::ResourceManager::(registerResource) Trying to register resource 'Storage.a1f5f2ad-76a4-49f0-9206-3aaddfb5ae3b' for lock type 'shared' Thread-51::DEBUG::2012-09-25 12:33:19,431::resourceManager::528::ResourceManager::(registerResource) Resource 'Storage.a1f5f2ad-76a4-49f0-9206-3aaddfb5ae3b' is free. Now locking as 'shared' (1 active user) Thread-51::DEBUG::2012-09-25 12:33:19,431::resourceManager::212::ResourceManager.Request::(grant) ResName=`Storage.a1f5f2ad-76a4-49f0-9206-3aaddfb5ae3b`ReqID=`7f60a01e-b773-45a2-98bb-6d9ddb7d571a`::Granted request Thread-51::DEBUG::2012-09-25 12:33:19,432::task::794::TaskManager.Task::(resourceAcquired) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::_resourcesAcquired: Storage.a1f5f2ad-76a4-49f0-9206-3aaddfb5ae3b (shared) Thread-51::DEBUG::2012-09-25 12:33:19,432::task::957::TaskManager.Task::(_decref) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::ref 1 aborting False Thread-51::DEBUG::2012-09-25 12:33:19,433::fileVolume::552::Storage.Volume::(validateVolumePath) validate path for f0b847a2-3adb-4fd9-bd34-7d96295857b7 Thread-51::INFO::2012-09-25 12:33:19,447::image::357::Storage.Image::(getChain) sdUUID=a1f5f2ad-76a4-49f0-9206-3aaddfb5ae3b imgUUID=28c86b7d-a96f-4df9-b540-d72e72108c01 chain=[<storage.glusterVolume.GlusterVolume object at 0x7f22dc05af90>] MainProcess|Thread-51::ERROR::2012-09-25 12:33:19,452::supervdsmServer::72::SuperVdsm.ServerCallback::(wrapper) Error in wrapper Traceback (most recent call last): File "/usr/share/vdsm/supervdsmServer.py", line 70, in wrapper return func(*args, **kwargs) File "/usr/share/vdsm/supervdsmServer.py", line 292, in wrapper return func(*args, **kwargs) File "/usr/share/vdsm/gluster/cli.py", line 46, in wrapper return func(*args, **kwargs) File "/usr/share/vdsm/gluster/cli.py", line 172, in volumeInfo command = _getGlusterVolCmd() + ["info"] File "/usr/share/vdsm/gluster/cli.py", line 36, in _getGlusterVolCmd return [_glusterCommandPath.cmd, "--mode=script", "volume"] File "/usr/lib64/python2.7/site-packages/vdsm/utils.py", line 844, in cmd os.strerror(os.errno.ENOENT), self.name) OSError: [Errno 2] No such file or directory: 'gluster' Thread-51::ERROR::2012-09-25 12:33:19,454::task::833::TaskManager.Task::(_setError) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::Unexpected error Traceback (most recent call last): File "/usr/share/vdsm/storage/task.py", line 840, in _run return fn(*args, **kargs) File "/usr/share/vdsm/logUtils.py", line 38, in wrapper res = f(*args, **kwargs) File "/usr/share/vdsm/storage/hsm.py", line 2769, in prepareImage 'vmVolInfo': vol.getVmVolumeInfo()} File "/usr/share/vdsm/storage/glusterVolume.py", line 27, in getVmVolumeInfo volInfo = svdsmProxy.glusterVolumeInfo(volname) File "/usr/share/vdsm/supervdsm.py", line 67, in __call__ return callMethod() File "/usr/share/vdsm/supervdsm.py", line 65, in <lambda> **kwargs) File "<string>", line 2, in glusterVolumeInfo File "/usr/lib64/python2.7/multiprocessing/managers.py", line 773, in _callmethod raise convert_to_error(kind, result) OSError: [Errno 2] No such file or directory: 'gluster' Thread-51::DEBUG::2012-09-25 12:33:19,456::task::852::TaskManager.Task::(_run) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::Task._run: aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d ('a1f5f2ad-76a4-49f0-9206-3aaddfb5ae3b', 'c3ad1a35-c05c-4626-82a3-9fc1c46af41a', '28c86b7d-a96f-4df9-b540-d72e72108c01', 'f0b847a2-3adb-4fd9-bd34-7d96295857b7') {} failed - stopping task Thread-51::DEBUG::2012-09-25 12:33:19,456::task::1177::TaskManager.Task::(stop) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::stopping in state preparing (force False) Thread-51::DEBUG::2012-09-25 12:33:19,456::task::957::TaskManager.Task::(_decref) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::ref 1 aborting True Thread-51::INFO::2012-09-25 12:33:19,456::task::1134::TaskManager.Task::(prepare) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::aborting: Task is aborted: u"[Errno 2] No such file or directory: 'gluster'" - code 100 Thread-51::DEBUG::2012-09-25 12:33:19,457::task::1139::TaskManager.Task::(prepare) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::Prepare: aborted: [Errno 2] No such file or directory: 'gluster' Thread-51::DEBUG::2012-09-25 12:33:19,457::task::957::TaskManager.Task::(_decref) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::ref 0 aborting True Thread-51::DEBUG::2012-09-25 12:33:19,457::task::892::TaskManager.Task::(_doAbort) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::Task._doAbort: force False Thread-51::DEBUG::2012-09-25 12:33:19,457::resourceManager::844::ResourceManager.Owner::(cancelAll) Owner.cancelAll requests {} Thread-51::DEBUG::2012-09-25 12:33:19,458::task::568::TaskManager.Task::(_updateState) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::moving from state preparing -> state aborting Thread-51::DEBUG::2012-09-25 12:33:19,458::task::523::TaskManager.Task::(__state_aborting) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::_aborting: recover policy none Thread-51::DEBUG::2012-09-25 12:33:19,458::task::568::TaskManager.Task::(_updateState) Task=`aae72dcc-f4b8-4d0d-9dff-351dfe2bfe6d`::moving from state aborting -> state failed
-- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 8: Looks good to me, but someone else must approve
I didn't look closely but overall now the architecture is much better.
-- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 9: Verified
Submitted as part of a new topic
-- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 9: No score
* QEMU-GlusterFS integration patches are merged into QEMU upstream. See https://lists.gnu.org/archive/html/qemu-devel/2012-09/msg05199.html. This marks the base enablement needed for this patch.
* Libvirt patches for supporting GlusterFS as a network drive is currently under review @ https://www.redhat.com/archives/libvir-list/2012-October/msg00085.html.
* Curent patch is in sync with libvirt support. Based on the progress of libvirt patch, I will be updating this patch so that its in sync with the libvirt.
thanx, deepak
-- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 9: Verified
-- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Adam Litke has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 9: I would prefer that you didn't submit this
Could you add some test cases for your code? Not only will it verify correctness, but it will show those of us who are not familiar with Gluster how to test your code. Thanks!
-- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 9:
@Adam, I am not sure if i can add a test case before the libvirt changes for gluster disk support are upstream.. they are still a WIP. Also adding testcase for this means checking for gluster binaries and skipping test if they aren't present, since w/o creating a gluster volume, this can't be tested. Am i thinking right ?
-- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 10:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/433/ (1/2)
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 10:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/398/ (2/2)
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 10: I would prefer that you didn't submit this
Build Unstable
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/398/ : UNSTABLE
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/433/ : SUCCESS
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 10: Verified
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 10:
@aglitke, added glusterSD functional testcase.
-- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 11:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/597/ (2/2)
-- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 11:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/562/ (1/2)
-- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 11: Verified
fixed issues reported by jenkins
-- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 11:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/562/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/597/ : SUCCESS
-- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 12:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/686/ (2/2)
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 12:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/651/ (1/2)
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 12:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/651/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/686/ : SUCCESS
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 12: Verified
No changes wrt prev. patchset had to rebase the topic, hence this new patchset.
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 12: (6 inline comments)
Few minor comments. Marking as -1 for visibility. WRT the architecture I'm not sure if this is the way we want to go. For the rest the patch looks fine.
.................................................... 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 + os.path.join? 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 maybe we can get rid of this. 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, ":") rpath.rsplit(":") 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'} this might belong to the entire module or to the GlusterVolume class. 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, "/") imgFilePath.rsplit... Line 36: imgFileRelPath = string.join(imgFilePath_list[4:], "/") Line 37: Line 38: glusterPath = volname + '/' + imgFileRelPath Line 39:
.................................................... 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()} 'path' might be redundant since in hsm.py already returns it (line 3054). We might want to decide if it makes sense to keep it here and remove it from there. 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@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 12: I would prefer that you didn't submit this
Few minor comments. Marking as -1 for visibility. WRT the architecture I'm not sure if this is the way we want to go. For the rest the patch looks fine.
-- 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@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
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@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 12: (1 inline comment)
.................................................... 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()} Another reason, I was also not sure if in future consumers of prepareImage might still expect ['path'] to be the path in the domain. if we remove that, derived class might change vmVolInfo['path'] and that can break assumptions of prepareImage consumers. To be safe for past and future users, and for reasons mentioned above, i still feel path is needed in both places. 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@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 13:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/675/ (2/2)
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 13:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/710/ (1/2)
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 13: Verified
Addressed fsimonce's review comments. Verified
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 13:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/675/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/710/ : SUCCESS
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 13: I would prefer that you didn't submit this
(1 inline comment)
Minor issue. The rest looks fine to me.
.................................................... File vdsm/storage/glusterSD.py Line 18: return glusterVolume.GlusterVolume Line 19: Line 20: @staticmethod Line 21: def findDomainPath(sdUUID): Line 22: glusterDomPath = os.path.join(sd.GLUSTERSD_DIR + "/*") I don't understand this change. My previous comment was about using os.path.join to build the path:
os.path.join(GLUSTERSD_DIR, "*") Line 23: for tmpSdUUID, domainPath in fileSD.scanDomains(glusterDomPath): Line 24: if tmpSdUUID == sdUUID and mount.isMounted(os.path.join(domainPath, Line 25: "..")): Line 26: return domainPath
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 13: (1 inline comment)
.................................................... File vdsm/storage/glusterSD.py Line 18: return glusterVolume.GlusterVolume Line 19: Line 20: @staticmethod Line 21: def findDomainPath(sdUUID): Line 22: glusterDomPath = os.path.join(sd.GLUSTERSD_DIR + "/*") I had tried the below...
os.path.join(sd.GLUSTERSD_DIR, "*") and that was not working. After debug i found it was giving glusterSD* not glusterSD/* which was causing my findDomain to fail.
Hence i used os.path.join(sd.GLUSTERSD_DIR, "/*") Line 23: for tmpSdUUID, domainPath in fileSD.scanDomains(glusterDomPath): Line 24: if tmpSdUUID == sdUUID and mount.isMounted(os.path.join(domainPath, Line 25: "..")): Line 26: return domainPath
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 13: (2 inline comments)
.................................................... File vdsm/storage/glusterSD.py Line 18: return glusterVolume.GlusterVolume Line 19: Line 20: @staticmethod Line 21: def findDomainPath(sdUUID): Line 22: glusterDomPath = os.path.join(sd.GLUSTERSD_DIR + "/*") Done Line 23: for tmpSdUUID, domainPath in fileSD.scanDomains(glusterDomPath): Line 24: if tmpSdUUID == sdUUID and mount.isMounted(os.path.join(domainPath, Line 25: "..")): Line 26: return domainPath
Line 18: return glusterVolume.GlusterVolume Line 19: Line 20: @staticmethod Line 21: def findDomainPath(sdUUID): Line 22: glusterDomPath = os.path.join(sd.GLUSTERSD_DIR + "/*") Ok my bad.. i think i understood, instead of ',' i had a '+'. Oops ! Will post new patchset. Thanks Line 23: for tmpSdUUID, domainPath in fileSD.scanDomains(glusterDomPath): Line 24: if tmpSdUUID == sdUUID and mount.isMounted(os.path.join(domainPath, Line 25: "..")): Line 26: return domainPath
-- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/748/ (1/2)
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/713/ (2/2)
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/713/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/748/ : SUCCESS
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14: Verified
address 1 minor comment from fsimonce. Verified.
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14: Looks good to me, approved
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14: I would prefer that you didn't submit this
(4 inline comments)
mostly non-lethal stuff, but please address my questions.
.................................................... File vdsm/storage/glusterVolume.py Line 14: """ Line 15: Send info to represent Gluster volume as a network block device Line 16: """ Line 17: rpath = sdCache.produce(self.sdUUID).getRemotePath() Line 18: rpath_list = rpath.rsplit(":") having a verbatim colon in the path is quite insane, but if you are doing rsplit(), you should limit yourself to a single splitter. Line 19: volfileServer = rpath_list[0] Line 20: volname = rpath_list[1] Line 21: Line 22: # Volume transport to Libvirt transport mapping
Line 31: volPort = "0" Line 32: Line 33: imgFilePath = self.getVolumePath() Line 34: imgFilePath_list = imgFilePath.rsplit("/") Line 35: imgFileRelPath = "/".join(imgFilePath_list[4:]) where is this hard-coded "4" comes from? shouldn't it depend on @vdsmrepo@?
I'm not sure that what you do is safer than imgFilePath[len(/rhev/data-center):] Line 36: Line 37: glusterPath = volname + '/' + imgFileRelPath Line 38: Line 39: return {'volType': VmVolumeInfo.TYPE_NETWORK, 'path': glusterPath,
.................................................... File vdsm/storage/sdc.py Line 132: def _findDomain(self, sdUUID): Line 133: import blockSD Line 134: import localFsSD Line 135: import nfsSD Line 136: import glusterSD nit: keep sorted Line 137: Line 138: # The order is somewhat important, it's ordered Line 139: # by how quickly get can find the domain. For instance Line 140: # if an nfs mount is unavailable we will get stuck
.................................................... File vdsm/storage/storageServer.py Line 248: class GlusterFSConnection(MountConnection): Line 249: Line 250: def _getLocalPath(self): Line 251: return os.path.join( Line 252: self.localPathBase, "glusterSD", why not just set localPathBase += '/glusterSD' Line 253: self._remotePath.replace("_", "__").replace("/", "_")) Line 254: Line 255: Line 256: class NFSConnection(object):
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14: (4 inline comments)
@Dan, Responded to your comments. Pls take a look.
.................................................... File vdsm/storage/glusterVolume.py Line 14: """ Line 15: Send info to represent Gluster volume as a network block device Line 16: """ Line 17: rpath = sdCache.produce(self.sdUUID).getRemotePath() Line 18: rpath_list = rpath.rsplit(":") remotePath comes from the user as part of the 'spec', typically server:/export ( in case of NFS) and server:volumename (in case of gluster). I need to extract the volume name hence the need to split using ':'. IIUC, ':' will be present in NFS' getRemotePath() as well. Is there a better way of doing this w/o using ':' ?
I will use maxsplit=1 for the rsplit, since it always should be of the form server:volumename for gluster case. Line 19: volfileServer = rpath_list[0] Line 20: volname = rpath_list[1] Line 21: Line 22: # Volume transport to Libvirt transport mapping
Line 31: volPort = "0" Line 32: Line 33: imgFilePath = self.getVolumePath() Line 34: imgFilePath_list = imgFilePath.rsplit("/") Line 35: imgFileRelPath = "/".join(imgFilePath_list[4:]) I agree, hard-coding needs to be removed. Let me see how I can do that.
4: means i need to find the path relative to the start of the mount point. Basically the path to the image starting from the sdUUID/... bcos this gets fed to the qemu's gluster:// cmdline that expects a path relative to the gluster volume. Line 36: Line 37: glusterPath = volname + '/' + imgFileRelPath Line 38: Line 39: return {'volType': VmVolumeInfo.TYPE_NETWORK, 'path': glusterPath,
.................................................... File vdsm/storage/sdc.py Line 132: def _findDomain(self, sdUUID): Line 133: import blockSD Line 134: import localFsSD Line 135: import nfsSD Line 136: import glusterSD Done Line 137: Line 138: # The order is somewhat important, it's ordered Line 139: # by how quickly get can find the domain. For instance Line 140: # if an nfs mount is unavailable we will get stuck
.................................................... File vdsm/storage/storageServer.py Line 248: class GlusterFSConnection(MountConnection): Line 249: Line 250: def _getLocalPath(self): Line 251: return os.path.join( Line 252: self.localPathBase, "glusterSD", I thought using os.path.join was portable and preferred ? In fact in one of my prev. patchsets.. fsimonce had asked me to use os.path.join instead of +=.
is there any advantage of usign += here instead of the preferred os.path.join ? I am not clear.
What i am doing here is to get a glusterSD specific mnt path something like mnt/glusterSD/servername:volname. Line 253: self._remotePath.replace("_", "__").replace("/", "_")) Line 254: Line 255: Line 256: class NFSConnection(object):
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14: (4 inline comments)
hth
.................................................... File vdsm/storage/glusterVolume.py Line 14: """ Line 15: Send info to represent Gluster volume as a network block device Line 16: """ Line 17: rpath = sdCache.produce(self.sdUUID).getRemotePath() Line 18: rpath_list = rpath.rsplit(":") yeah I've meant
rpath.rsplit(":", 1) Line 19: volfileServer = rpath_list[0] Line 20: volname = rpath_list[1] Line 21: Line 22: # Volume transport to Libvirt transport mapping
Line 31: volPort = "0" Line 32: Line 33: imgFilePath = self.getVolumePath() Line 34: imgFilePath_list = imgFilePath.rsplit("/") Line 35: imgFileRelPath = "/".join(imgFilePath_list[4:]) I did not quite understand the explanation - are you trying to drop localPathBase from the imgFilePath? if so, please do it directly, without assumptions of its depth. Line 36: Line 37: glusterPath = volname + '/' + imgFileRelPath Line 38: Line 39: return {'volType': VmVolumeInfo.TYPE_NETWORK, 'path': glusterPath,
.................................................... File vdsm/storage/sdc.py Line 140: # if an nfs mount is unavailable we will get stuck Line 141: # until it times out, this should affect fetching Line 142: # of block\local domains. If for any case in the future Line 143: # this changes, please update the order. Line 144: for mod in (blockSD, localFsSD, glusterSD, nfsSD): (here too) Line 145: try: Line 146: return mod.findDomain(sdUUID) Line 147: except se.StorageDomainDoesNotExist: Line 148: pass
.................................................... File vdsm/storage/storageServer.py Line 248: class GlusterFSConnection(MountConnection): Line 249: Line 250: def _getLocalPath(self): Line 251: return os.path.join( Line 252: self.localPathBase, "glusterSD", I was completely misunderstood. My point is asking why you are overriding _getLocalPath() with an almost-exact reimplementation, and whether setting your localPathBase proprly would be cleaner. Line 253: self._remotePath.replace("_", "__").replace("/", "_")) Line 254: Line 255: Line 256: class NFSConnection(object):
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14: (4 inline comments)
@dan, pls see my comments and let me know if I have understood you correctly ?
.................................................... File vdsm/storage/glusterVolume.py Line 14: """ Line 15: Send info to represent Gluster volume as a network block device Line 16: """ Line 17: rpath = sdCache.produce(self.sdUUID).getRemotePath() Line 18: rpath_list = rpath.rsplit(":") Done Line 19: volfileServer = rpath_list[0] Line 20: volname = rpath_list[1] Line 21: Line 22: # Volume transport to Libvirt transport mapping
Line 31: volPort = "0" Line 32: Line 33: imgFilePath = self.getVolumePath() Line 34: imgFilePath_list = imgFilePath.rsplit("/") Line 35: imgFileRelPath = "/".join(imgFilePath_list[4:]) Regret if i was not clear. Given the below path for a image as an example : /rhev/data-center/mnt/glusterSD/vm-vdsm-de-1:dpkvol3/51f3411f-d507-4f78-be9c-96c245eb9273/images/280c372c-9cd5-4fa0-9dbc-238a087a1e9b/b11687e6-978e-42b7-90c3-a4bf77cde1cc
I want to extract : 51f3411f-d507-4f78-be9c-96c245eb9273/images/280c372c-9cd5-4fa0-9dbc-238a087a1e9b/b11687e6-978e-42b7-90c3-a4bf77cde1cc
since thats what is volume relative path which gets fed to qemu's gluster backend.
What do you mean by "do it directly" ? I am not clear. Line 36: Line 37: glusterPath = volname + '/' + imgFileRelPath Line 38: Line 39: return {'volType': VmVolumeInfo.TYPE_NETWORK, 'path': glusterPath,
.................................................... File vdsm/storage/sdc.py Line 140: # if an nfs mount is unavailable we will get stuck Line 141: # until it times out, this should affect fetching Line 142: # of block\local domains. If for any case in the future Line 143: # this changes, please update the order. Line 144: for mod in (blockSD, localFsSD, glusterSD, nfsSD): Done Line 145: try: Line 146: return mod.findDomain(sdUUID) Line 147: except se.StorageDomainDoesNotExist: Line 148: pass
.................................................... File vdsm/storage/storageServer.py Line 248: class GlusterFSConnection(MountConnection): Line 249: Line 250: def _getLocalPath(self): Line 251: return os.path.join( Line 252: self.localPathBase, "glusterSD", So instead of overriding, I can jsut do localPathBase = os.path.join(MountConnection.localPathBase, "glusterSD") inside the GlusterConnection class, is my understanding rite ? Line 253: self._remotePath.replace("_", "__").replace("/", "_")) Line 254: Line 255: Line 256: class NFSConnection(object):
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14: (2 inline comments)
.................................................... File vdsm/storage/glusterVolume.py Line 31: volPort = "0" Line 32: Line 33: imgFilePath = self.getVolumePath() Line 34: imgFilePath_list = imgFilePath.rsplit("/") Line 35: imgFileRelPath = "/".join(imgFilePath_list[4:]) I meant that you could build /rhev/data-center/mnt/glusterSD/vm-vdsm-de-1:dpkvol3, find its len(), and drop it from the path.
However, in a second thought,
"/".join(imgFilePath_list[-4:])
with a proper comment would be even better. Line 36: Line 37: glusterPath = volname + '/' + imgFileRelPath Line 38: Line 39: return {'volType': VmVolumeInfo.TYPE_NETWORK, 'path': glusterPath,
.................................................... File vdsm/storage/storageServer.py Line 248: class GlusterFSConnection(MountConnection): Line 249: Line 250: def _getLocalPath(self): Line 251: return os.path.join( Line 252: self.localPathBase, "glusterSD", that was my suggestion. I did not look deeply into the possible side effects, but the code would be nicer. Line 253: self._remotePath.replace("_", "__").replace("/", "_")) Line 254: Line 255: Line 256: class NFSConnection(object):
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14: (2 inline comments)
@Dan, Added few comments to yours. Pls let me know your thoughts, post which I will send next patchset. Thanks.
.................................................... File vdsm/storage/glusterVolume.py Line 31: volPort = "0" Line 32: Line 33: imgFilePath = self.getVolumePath() Line 34: imgFilePath_list = imgFilePath.rsplit("/") Line 35: imgFileRelPath = "/".join(imgFilePath_list[4:]) Actually getVolumePath returns the path as below ( unlike i put before )...
/rhev/data-center/9d9790c3-3486-4a6e-b6ed-32cfdd1f7d27/6e5c80ef-523b-45c3-a366-28406c4c5daf/images/78abe539-2d20-469b-adf7-8fa753016102/dbf9b95f-22d3-4ccc-aa5e-dbdb988fdb20
So your suggestion to use [-4:] would still work, since we are going from the back of the list. Thanks, i will use ur suggestion and post next patchset. Line 36: Line 37: glusterPath = volname + '/' + imgFileRelPath Line 38: Line 39: return {'volType': VmVolumeInfo.TYPE_NETWORK, 'path': glusterPath,
.................................................... File vdsm/storage/storageServer.py Line 248: class GlusterFSConnection(MountConnection): Line 249: Line 250: def _getLocalPath(self): Line 251: return os.path.join( Line 252: self.localPathBase, "glusterSD", I tried this
class GlusterFSConnection(MountConnection): localPathBase = os.path.join(sd.mountBasePath, "glusterSD") This needs storageServer.py to do 'import sd'.
I am hoping this is fine & no need to override getLocalPath ? This works as I tested. Line 253: self._remotePath.replace("_", "__").replace("/", "_")) Line 254: Line 255: Line 256: class NFSConnection(object):
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14: (1 inline comment)
.................................................... File vdsm/storage/storageServer.py Line 248: class GlusterFSConnection(MountConnection): Line 249: Line 250: def _getLocalPath(self): Line 251: return os.path.join( Line 252: self.localPathBase, "glusterSD", I have a vague memory of Saggi hating this import. Let us not infuriate him.
But wouldn't
o.p.join(MountConnection.mountBasePath, "glusterSD")
be just as good? Line 253: self._remotePath.replace("_", "__").replace("/", "_")) Line 254: Line 255: Line 256: class NFSConnection(object):
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 14: (1 inline comment)
@Dan, pls see my comment
.................................................... File vdsm/storage/storageServer.py Line 248: class GlusterFSConnection(MountConnection): Line 249: Line 250: def _getLocalPath(self): Line 251: return os.path.join( Line 252: self.localPathBase, "glusterSD", Ok, i too won't like to infuriate him :)
Unfortunately MountConnection doesn't have mountBasePath, it only has localPathBase = "/tmp" and HSM's _init__ sets the localPathBase of MountConn to /rhev/dc/mnt/
So you suggestion above won't work for GlusterConn.
I can set GlusterConn's localPathBase in a similar way as done in HSM for MountConn, just that in storageServer.py there won't be anything inside the GlusterCOnn class.. it would just be a empty class.. will that be good ? Line 253: self._remotePath.replace("_", "__").replace("/", "_")) Line 254: Line 255: Line 256: class NFSConnection(object):
-- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 15:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/888/ (2/3)
-- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 15:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/923/ (3/3)
-- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 15:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/31/ (1/3)
-- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 15: Verified
@Dan, I hope to have addressed all your comments here. Thanks Deepak
-- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 15: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/888/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/923/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/31/ : FAILURE
-- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Patch Set 15: Looks good to me, approved
yay!
-- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Support for GLUSTERFS_DOMAIN ......................................................................
Support for GLUSTERFS_DOMAIN
This patch introduces a new storage domain of type GLUSTERFS_DOMAIN, which uses gluster as the storage backend.
In GLUSTERFS_DOMAIN, vdsm creates the storage domain by mounting the gluster volume (akin to nfs mounting export path). VMs created using this domain exploit the QEMU's gluster block backend. Instead of accessing the vmdisk as a file path, it accesses the vmdisk as a network disk device, served by gluster server/volume.
This patch attempts to re-use nfsSD core logic (to an extent) to support domain of type GLUSTERFS_DOMAIN.
Change-Id: I9ac37da88625f20d148beaf53bb6371c15b33ad7 Signed-off-by: Deepak C Shetty deepakcs@linux.vnet.ibm.com --- M vdsm.spec.in M vdsm/clientIF.py M vdsm/libvirtvm.py M vdsm/storage/Makefile.am A vdsm/storage/glusterSD.py A vdsm/storage/glusterVolume.py M vdsm/storage/hsm.py M vdsm/storage/sd.py M vdsm/storage/sdc.py M vdsm/storage/storageServer.py M vdsm/storage/volume.py 11 files changed, 158 insertions(+), 6 deletions(-)
Approvals: Dan Kenigsberg: Looks good to me, approved Deepak C Shetty: Verified
-- To view, visit http://gerrit.ovirt.org/6856 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I9ac37da88625f20d148beaf53bb6371c15b33ad7 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
vdsm-patches@lists.fedorahosted.org