Change in vdsm[master]: gluster: add createBrick verb
barumuga at redhat.com
barumuga at redhat.com
Fri Feb 13 07:54:10 UTC 2015
Bala.FA has posted comments on this change.
Change subject: gluster: add createBrick verb
......................................................................
Patch Set 17: Code-Review-1
(14 comments)
http://gerrit.ovirt.org/#/c/35498/17/vdsm/gluster/Makefile.am
File vdsm/gluster/Makefile.am:
Line 31: api.py \
Line 32: apiwrapper.py \
Line 33: cli.py \
Line 34: exception.py \
Line 35: fstab.py \
You have to add fstab.py to vdsm.spec.in
Line 36: gfapi.py \
Line 37: hooks.py \
Line 38: services.py \
Line 39: storagedev.py \
http://gerrit.ovirt.org/#/c/35498/17/vdsm/gluster/api.py
File vdsm/gluster/api.py:
Line 391: mountPoint,
Line 392: devList,
Line 393: fsType,
Line 394: raidParams)
Line 395: return {'deviceInfo': status}
how about 'device' instead of 'deviceInfo'?
Line 396:
Line 397:
Line 398: def getGlusterMethods(gluster):
Line 399: l = []
http://gerrit.ovirt.org/#/c/35498/17/vdsm/gluster/exception.py
File vdsm/gluster/exception.py:
Line 416: class GlusterHostStorageDeviceMountFailedException(GlusterHostException):
Line 417: code = 4411
Line 418:
Line 419: def __init__(self, device):
Line 420: self.message = "Device %s mount failed" % device
You should have message like
"Failed to mount device %s on mount point %s using fs-type %s with mount options %s"
Line 421:
Line 422:
Line 423: class GlusterHostStorageDeviceMkfsFailedException(GlusterHostException):
Line 424: code = 4412
Line 428: self.out = out
Line 429: self.err = err
Line 430: self.message = "Device format failed for device %s with params " + \
Line 431: "alignment: %s, stripe size: %s" % (device, alignment,
Line 432: stripeSize)
You can have clear message like
"Failed to format device %s with alignment %s stripe-size %s"
Line 433:
Line 434:
Line 435: class GlusterHostStorageDeviceFsTabFoundException(GlusterHostException):
Line 436: code = 4413
Line 435: class GlusterHostStorageDeviceFsTabFoundException(GlusterHostException):
Line 436: code = 4413
Line 437:
Line 438: def __init__(self, device):
Line 439: self.message = "Device entry %s already found in fstab" % device
"fstab entry for device %s already exists"
Line 440:
Line 441:
Line 442: class GlusterHostStorageDevicePVCreateFailedException(GlusterHostException):
Line 443: code = 4414
Line 446: self.rc = rc
Line 447: self.out = out
Line 448: self.err = err
Line 449: self.message = "PV Create failed for the device %s " + \
Line 450: "with dataalignment %s" % (device, alignment)
"Failed to create LVM PV for device %s with data alignment %s"
Line 451:
Line 452:
Line 453: class GlusterHostStorageDeviceUuidNotFoundException(GlusterHostException):
Line 454: code = 4415
Line 449: self.message = "PV Create failed for the device %s " + \
Line 450: "with dataalignment %s" % (device, alignment)
Line 451:
Line 452:
Line 453: class GlusterHostStorageDeviceUuidNotFoundException(GlusterHostException):
This exception is not consumed anywhere. Please remove it
Line 454: code = 4415
Line 455:
Line 456: def __init__(self, device):
Line 457: self.message = "Device %s UUID not found" % device
Line 456: def __init__(self, device):
Line 457: self.message = "Device %s UUID not found" % device
Line 458:
Line 459:
Line 460: class GlusterHostStorageDeviceLvConvertFailedException(GlusterHostException):
rename to GlusterHostStorageDeviceLVConvertFailedException
Line 461: code = 4416
Line 462:
Line 463: def __init__(self, devicePath, alignment, rc=0, out=(), err=()):
Line 464: self.rc = rc
Line 464: self.rc = rc
Line 465: self.out = out
Line 466: self.err = err
Line 467: self.message = "LV Convert failed for the device: " + \
Line 468: "%s with alignment: %s" % (devicePath, alignment)
1. "Failed to run lvconvert for device %s with data alignment %s"
2. have consistent name as 'device' than 'devicePath'
Line 469:
Line 470:
Line 471: # Hook
Line 472: class GlusterHookException(GlusterException):
http://gerrit.ovirt.org/#/c/35498/17/vdsm/gluster/storagedev.py
File vdsm/gluster/storagedev.py:
Line 78: else:
Line 79: info['model'] = device.type
Line 80: if device.format:
Line 81: info['uuid'] = device.format.uuid or ''
Line 82: info['fsType'] = device.format.type or fsType or ''
1. I think 'device.format.type or fsType' itself is ok. Please check it out
2. This code has to be a separate patch though
Line 83: if hasattr(device.format, 'mountpoint'):
Line 84: info['mountPoint'] = device.format.mountpoint or ''
Line 85: return info
Line 86:
Line 199:
Line 200: rc, out, err = utils.execCmd([_lvchangeCommandPath.cmd,
Line 201: '--zero', 'n', vgPoolName])
Line 202: if rc:
Line 203: raise ge.GlusterHostStorageDeviceLvChangeFailedException(
1. This exception is missing in exception.py
2. You need to pass vgPoolName to the exception
Line 204: rc, out, err)
Line 205:
Line 206: blivetEnv.reset()
Line 207: return blivetEnv.devicetree.getDeviceByName(poolLv.name)
Line 230:
Line 231: if fsType == DEFAULT_FS_TYPE:
Line 232: return _formatDeviceXfs()
Line 233: else:
Line 234: log.error("%s is currently unsupported" % fsType)
How about 'fstype %s is currently unsupported' ?
Line 235: raise ge.GlusterHostStorageDeviceMkfsFailedException(
Line 236: devPath, alignment, stripeSize, fsType)
Line 237:
Line 238: vgName = "vg-" + brickName
Line 290: raidParams.get('stripeSize'), fsType)
Line 291: m = mount.Mount(thinlv.path, mountPoint)
Line 292: m.mount(vfstype=DEFAULT_FS_TYPE, mntOpts=DEFAULT_MOUNT_OPTIONS)
Line 293: if not m.isMounted():
Line 294: ge.GlusterHostStorageDeviceMountFailedException()
You should pass device, mount-point, fstype, mount options to this exception
Line 295: else:
Line 296: format = blivet.formats.getFormat(DEFAULT_FS_TYPE, device=thinlv.path)
Line 297: blivetEnv.formatDevice(thinlv, format)
Line 298: blivetEnv.doIt()
Line 298: blivetEnv.doIt()
Line 299:
Line 300: thinlv.format.setup(mountpoint=mountPoint)
Line 301: blivetEnv.doIt()
Line 302: fstab.FsTab("/etc/fstab").add(thinlv.path, mountPoint, DEFAULT_FS_TYPE)
Why pass '/etc/fstab' as its default in FsTab class
Line 303:
--
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: 17
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