Change in vdsm[master]: gluster: add createBrick verb
barumuga at redhat.com
barumuga at redhat.com
Tue Jan 27 09:10:40 UTC 2015
Bala.FA has posted comments on this change.
Change subject: gluster: add createBrick verb
......................................................................
Patch Set 8: Code-Review-1
(12 comments)
http://gerrit.ovirt.org/#/c/35498/8/vdsm.spec.in
File vdsm.spec.in:
Line 121: BuildRequires: python-ordereddict
Line 122: %endif
Line 123: %if 0%{?rhel} > 6 || 0%{?fedora}
Line 124: BuildRequires: python-blivet
Line 125: BuildRequires: xfsprogs
xfsprogs is available in rhel 6 too. This dependency needs to be placed in gluster sub-package.
Line 126: %endif
Line 127:
Line 128: # Autotools BuildRequires
Line 129: %if 0%{?enable_autotools}
Line 701: Requires: glusterfs-server
Line 702: Requires: python-magic
Line 703: %if 0%{?rhel} > 6 || 0%{?fedora}
Line 704: Requires: python-blivet
Line 705: Requires: xfsprogs
same as above
Line 706: %endif
Line 707:
Line 708: %description gluster
Line 709: Gluster plugin enables VDSM to serve Gluster functionalities.
http://gerrit.ovirt.org/#/c/35498/8/vdsm/gluster/exception.py
File vdsm/gluster/exception.py:
Line 403: code = 4409
Line 404:
Line 405: def __init__(self, deviceList):
Line 406: self.message = "Device(s) %s not found" % [
Line 407: name for name in deviceList]
Why is device list read and converted to list again?
Line 408:
Line 409:
Line 410: class GlusterHostStorageDeviceInUseException(GlusterHostException):
Line 411: code = 4410
Line 411: code = 4410
Line 412:
Line 413: def __init__(self, deviceList):
Line 414: self.message = "Device(s) %s already in use" % [
Line 415: name for name in deviceList]
same as above
Line 416:
Line 417:
Line 418: class GlusterHostStorageDeviceMountFailedException(GlusterHostException):
Line 419: code = 4411
Line 416:
Line 417:
Line 418: class GlusterHostStorageDeviceMountFailedException(GlusterHostException):
Line 419: code = 4411
Line 420: message = "Device mount failed"
Why is deviceName taken out?
Line 421:
Line 422:
Line 423: class GlusterHostStorageDeviceMkfsFailedException(GlusterHostException):
Line 424: code = 4412
Line 421:
Line 422:
Line 423: class GlusterHostStorageDeviceMkfsFailedException(GlusterHostException):
Line 424: code = 4412
Line 425: message = "Device format failed"
1. Why is deviceName taken out?
2. Why aren't file system type/options added in the exception?
Line 426:
Line 427:
Line 428: class GlusterHostStorageDeviceAlreadyMounted(GlusterHostException):
Line 429: code = 4413
http://gerrit.ovirt.org/#/c/35498/8/vdsm/gluster/storagedev.py
File vdsm/gluster/storagedev.py:
Line 44: _mountCommandPath = utils.CommandPath("mount",
Line 45: "/sbin",
Line 46: "/usr/sbin/mount",)
Line 47: _mkfsCommandPath = utils.CommandPath("mkfs",
Line 48: "/sbin",
Have you tested this?
Line 49: "/usr/sbin/mkfs",)
Line 50:
Line 51: # All size are in MiB unless otherwise specified
Line 52: DEFAULT_CHUNK_SIZE = 0.25
Line 106: for devName in devNameList]
Line 107:
Line 108: def _makePartition(deviceList):
Line 109: pvDeviceList = []
Line 110: requirePart = False
How about doPartitioning than requirePart?
Line 111: for dev in deviceList:
Line 112: if dev.type in ['disk', 'dm-multipath']:
Line 113: blivetEnv.initializeDisk(dev)
Line 114: part = blivetEnv.newPartition(fmt_type="lvmpv", grow=True,
Line 128: for dev in deviceList:
Line 129: rc, out, err = utils.execCmd([_pvcreateCommandPath.cmd,
Line 130: '--dataalignment',
Line 131: '%sK' % alignment,
Line 132: dev.path])
You haven't done
1. Please mention this workaround along with blivet bz# and why you are doing.
2. Handle if the command fails.
Line 133: blivetEnv.reset()
Line 134: return _getDeviceList([dev.name for dev in deviceList])
Line 135:
Line 136: if alignment:
Line 203: def _updateFstab(device, path):
Line 204: if mount.isMounted(path):
Line 205: raise ge.GlusterHostStorageDeviceAlreadyMounted(device)
Line 206: with open("/etc/fstab", "a") as fstab:
Line 207: fstab.write("%s %s xfs rw,inode64,noatime,nouuid 1 2\n" % (
below is not yet addressed
Write proper fstab management. I believe vdsm has this capabilities. Please check and write fstab management accordingly.
Line 208: device, path))
Line 209:
Line 210: vgName = "vg-" + brickName
Line 211: poolName = "pool-" + brickName
Line 217: alignment = raidParams['stripeSize'] * (raidParams['pdCount'] - 2)
Line 218: chunkSize = alignment
Line 219: elif raidParams.get('type') == '10':
Line 220: alignment = raidParams['stripeSize'] * (raidParams['pdCount'] / 2)
Line 221: chunkSize = DEFAULT_CHUNK_SIZE
Below is not yet addressed
Add pointer as comment to admin guide how/why you are doing this calculation.
Line 222:
Line 223: blivetEnv = blivet.Blivet()
Line 224: blivetEnv.reset()
Line 225:
Line 254: blivetEnv.doIt()
Line 255:
Line 256: if alignment:
Line 257: _formatDevice(thinlv.path, alignment, raidParams.get('stripeSize'))
Line 258: mpath = "/bricks/%s" % brickName
Below is not yet addressed
The mount prefix should be configurable. For eg, in RHS its '/rhs' and in upstream its /bricks
Line 259: m = mount.Mount(thinlv.path, mpath)
Line 260: m.mount(vfstype="xfs", mntOpts="inode64,noatime")
Line 261: if not m.isMounted():
Line 262: ge.GlusterHostStorageDeviceMountFailedException()
--
To view, visit http://gerrit.ovirt.org/35498
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic47c4c56834deb457ae9d038f77bcf69c7b39ba5
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Timothy Asir <tjeyasin at redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Sahina Bose <sabose at redhat.com>
Gerrit-Reviewer: Shubhendu Tripathi <shtripat at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list