Change in vdsm[master]: gluster: add createBrick verb
barumuga at redhat.com
barumuga at redhat.com
Mon Feb 2 08:26:27 UTC 2015
Bala.FA has posted comments on this change.
Change subject: gluster: add createBrick verb
......................................................................
Patch Set 11:
(12 comments)
http://gerrit.ovirt.org/#/c/35498/11/vdsm/gluster/storagedev.py
File vdsm/gluster/storagedev.py:
Line 74: int(tokens[5]),
Line 75: int(tokens[6])))
Line 76: return devList
Line 77:
Line 78: def _getDevUuidDict(self, device):
This should return fsUuid like
def _getFsUuid(self, device):
...
...
return uuid
Line 79: devUuidDict = {}
Line 80: for fpath in glob.iglob("/dev/disk/by-uuid" + '/*'):
Line 81: devUuidDict[os.path.realpath(fpath)] = fpath.split('/')[-1]
Line 82: return devUuidDict
Line 76: return devList
Line 77:
Line 78: def _getDevUuidDict(self, device):
Line 79: devUuidDict = {}
Line 80: for fpath in glob.iglob("/dev/disk/by-uuid" + '/*'):
you could use os.listdir() which simplifies things
Line 81: devUuidDict[os.path.realpath(fpath)] = fpath.split('/')[-1]
Line 82: return devUuidDict
Line 83:
Line 84: def _exists(self, device):
Line 91: if dev.device.startswith("UUID="):
Line 92: uuid = dev.device.split("UUID=")[-1]
Line 93: if device in devUuidDict and uuid == devUuidDict[device]:
Line 94: return True
Line 95: return False
The whole thing is simple if
uuid = "UUID=%s" % (self._getFsUuid(device))
for dev in self._list():
if device == dev.device or uuid = dev.device:
return True
return False
Line 96:
Line 97: def add(self, device, mountPoint, fsType,
Line 98: mntOpts='defaults', fsDump=0, fsPass=0):
Line 99: if self._exists(device):
Line 99: if self._exists(device):
Line 100: raise ge.GlusterHostStorageDeviceFsTabFoundException(device)
Line 101: devUuidDict = self._getDevUuidDict(device)
Line 102: if device not in devUuidDict:
Line 103: raise ge.GlusterHostStorageDeviceUuidNotFoundException(device)
I think this exception is not needed. If fsUuid is not found, you log it and continue by using device itself
Line 104: content = open(self.fileName).read()
Line 105: content += "UUID=%s\t%s\t%s\t%s\t%s\t%s\n" % (devUuidDict[device],
Line 106: mountPoint, fsType,
Line 107: ",".join(mntOpts),
Line 119: 'bus': device.bus or '',
Line 120: 'model': '',
Line 121: 'fsType': '',
Line 122: 'mountPoint': '',
Line 123: 'uuid': device.format.uuid or '',
Why are you missing this fix here? It should be a separate patch
Line 124: 'createBrick': True}
Line 125: if not info['bus'] and device.parents:
Line 126: info['bus'] = device.parents[0].bus
Line 127: if device.model:
Line 140:
Line 141: def _canCreateBrick(device):
Line 142: if not device or device.kids > 0 or device.format.type or \
Line 143: hasattr(device.format, 'mountpoint') or \
Line 144: device.type in ['cdrom', 'lvmvg', 'lvmthinpool', 'lvmlv', 'lvmthinlv']:
Have the above and this as one separate fix
Line 145: return False
Line 146: return True
Line 147:
Line 148:
Line 188: '%sK' % alignment,
Line 189: dev.path])
Line 190: if rc:
Line 191: raise ge.GlusterHostStorageDevicePVCreateFailedException(
Line 192: dev.path, "%sK" % alignment)
why can't we have alignment as is than string?
Line 193:
Line 194: blivetEnv.reset()
Line 195: return _getDeviceList([dev.name for dev in deviceList])
Line 196:
Line 260: devPath])
Line 261: if rc:
Line 262: cmd = "%s -t xfs -f -K -i size=512 -d -sw=%sk -su=%sk -n " \
Line 263: "size=8192 %d" % (_mkfsCommandPath.cmd, alignment,
Line 264: stripeSize, devPath)
This pattern is already logged by execCmd() itself.
Line 265: raise ge.GlusterHostStorageDeviceMkfsFailedException(cmd)
Line 266:
Line 267: vgName = "vg-" + brickName
Line 268: poolName = "pool-" + brickName
Line 261: if rc:
Line 262: cmd = "%s -t xfs -f -K -i size=512 -d -sw=%sk -su=%sk -n " \
Line 263: "size=8192 %d" % (_mkfsCommandPath.cmd, alignment,
Line 264: stripeSize, devPath)
Line 265: raise ge.GlusterHostStorageDeviceMkfsFailedException(cmd)
You would need to pass respective values to this exception and construct message there
Line 266:
Line 267: vgName = "vg-" + brickName
Line 268: poolName = "pool-" + brickName
Line 269: alignment = 0
Line 286: set([dev.name for dev in deviceList]))
Line 287: if notFoundList:
Line 288: raise ge.GlusterHostStorageDeviceNotFoundException(notFoundList)
Line 289:
Line 290: unavailList = set(devNameList).difference(set([not _canCreateBrick(
How about inUseList?
Line 291: dev) or dev.name for dev in deviceList]))
Line 292: if unavailList:
Line 293: raise ge.GlusterHostStorageDeviceInUseException(unavailList)
Line 294:
Line 320: m.mount(vfstype="xfs", mntOpts="inode64,noatime")
Line 321: if not m.isMounted():
Line 322: ge.GlusterHostStorageDeviceMountFailedException()
Line 323: else:
Line 324: newFmt = blivet.formats.getFormat("xfs", device=thinlv.path)
newFmt can be as format
Line 325: blivetEnv.formatDevice(thinlv, newFmt)
Line 326: blivetEnv.doIt()
Line 327:
Line 328: thinlv.format.setup(mountpoint=mountPoint)
Line 326: blivetEnv.doIt()
Line 327:
Line 328: thinlv.format.setup(mountpoint=mountPoint)
Line 329: blivetEnv.doIt()
Line 330: FsTab("/etc/fstab").add(thinlv.path, mountPoint, "xfs")
Define DEFAULT_FS_TYPE = "xfs" and use it everywhere. This also applicable to mountOptions.
Line 331:
Line 332: return {'name': thinlv.name,
Line 333: 'size': '%s' % thinlv.size,
Line 334: 'devPath': thinlv.path,
--
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: 11
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: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Sahina Bose <sabose at redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo at redhat.com>
Gerrit-Reviewer: Shubhendu Tripathi <shtripat at redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin 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