Change in vdsm[master]: gluster: add createBrick verb

tjeyasin at redhat.com tjeyasin at redhat.com
Mon Feb 16 05:46:42 UTC 2015


Timothy Asir has posted comments on this change.

Change subject: gluster: add createBrick verb
......................................................................


Patch Set 18:

(8 comments)

http://gerrit.ovirt.org/#/c/35498/18/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, mountPoint, fsType, moutOpts):
Line 420:         self.message = "Failed to mount device %s on mount point %s using" +\
> + is not required
Done
Line 421:                        "fs-type %s with mount options %s" % (
Line 422:                            device, mountPoint, fsType, moutOpts)
Line 423: 
Line 424: 


Line 418: 
Line 419:     def __init__(self, device, mountPoint, fsType, moutOpts):
Line 420:         self.message = "Failed to mount device %s on mount point %s using" +\
Line 421:                        "fs-type %s with mount options %s" % (
Line 422:                            device, mountPoint, fsType, moutOpts)
> Fix the typo
Done
Line 423: 
Line 424: 
Line 425: class GlusterHostStorageDeviceMkfsFailedException(GlusterHostException):
Line 426:     code = 4412


Line 428:     def __init__(self, device, alignment, stripeSize, rc=0, out=(), err=()):
Line 429:         self.rc = rc
Line 430:         self.out = out
Line 431:         self.err = err
Line 432:         self.message = "Failed to format device %s with alignment %s " +\
> same as above
Done
Line 433:                        "stripe-size %s" % (device, alignment, stripeSize)
Line 434: 
Line 435: 
Line 436: class GlusterHostStorageDeviceFsTabFoundException(GlusterHostException):


Line 446:     def __init__(self, device, alignment, rc=0, out=(), err=()):
Line 447:         self.rc = rc
Line 448:         self.out = out
Line 449:         self.err = err
Line 450:         self.message = "Failed to create LVM PV for device %s with " +\
> same as above
Done
Line 451:                        "data alignment %s" % (device, alignment)
Line 452: 
Line 453: 
Line 454: class GlusterHostStorageDeviceLVConvertFailedException(GlusterHostException):


Line 464: 
Line 465: class GlusterHostStorageDeviceLVChangeFailedException(GlusterHostException):
Line 466:     code = 4416
Line 467: 
Line 468:     def __init__(self, poolName):
> Why are rc, out and err missing?
Added
Line 469:         self.message = "Failed to run lvchange for the thin pool: %s" % (
Line 470:             % poolName)
Line 471: 
Line 472: 


http://gerrit.ovirt.org/#/c/35498/18/vdsm/gluster/storagedev.py
File vdsm/gluster/storagedev.py:

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(
> Why vgPoolName is missing?
I will add it!
Line 204:                     rc, out, err)
Line 205: 
Line 206:             blivetEnv.reset()
Line 207:             return blivetEnv.devicetree.getDeviceByName(poolLv.name)


Line 284:                                         size=pool.size, grow=True)
Line 285:     blivetEnv.createDevice(thinlv)
Line 286:     blivetEnv.doIt()
Line 287: 
Line 288:     mountOpts = DEFAULT_MOUNT_OPTIONS
> Why this mountOpts? Why can't you use DEFAULT_MOUNT_OPTIONS?
I used to avoid max 80 char limit in GlusterHostStorageDeviceMountFailedException exception. However i can handle it differently, so i will remove it.
Line 289:     if alignment:
Line 290:         _formatDevice(thinlv.path, alignment,
Line 291:                       raidParams.get('stripeSize'), fsType)
Line 292:         m = mount.Mount(thinlv.path, mountPoint)


Line 304:         thinlv.format.setup(mountpoint=mountPoint)
Line 305:         blivetEnv.doIt()
Line 306:     fstab.FsTab().add(thinlv.path, mountPoint, DEFAULT_FS_TYPE)
Line 307: 
Line 308:     return _getDeviceDict(thinlv, False)
> Why do you need to pass False as its default already?
Done


-- 
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: 18
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