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

tjeyasin at redhat.com tjeyasin at redhat.com
Tue Mar 17 09:46:43 UTC 2015


Timothy Asir has posted comments on this change.

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


Patch Set 24:

(6 comments)

https://gerrit.ovirt.org/#/c/35498/24/vdsm/gluster/fstab.py
File vdsm/gluster/fstab.py:

Line 22: import logging
Line 23: from collections import namedtuple
Line 24: import exception as ge
Line 25: from . import safeWrite
Line 26: 
> please sort the imports and separate system imports from project imports wi
Done
Line 27: 
Line 28: log = logging.getLogger("Gluster")
Line 29: FstabRecord = namedtuple("FstabRecord", "device, mountPoint, fsType, "
Line 30:                          "mntOpts, fsDump, fsPass")


https://gerrit.ovirt.org/#/c/35498/24/vdsm/gluster/storagedev.py
File vdsm/gluster/storagedev.py:

Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: 
Line 21: import os
Line 22: import logging
> please separate additional libraries import from system library import with
Done
Line 23: import blivet
Line 24: import blivet.formats
Line 25: import blivet.formats.fs
Line 26: import blivet.size


Line 111:     def _makePartition(deviceList):
Line 112:         pvDeviceList = []
Line 113:         doPartitioning = False
Line 114:         for dev in deviceList:
Line 115:             if dev.type in ['disk', 'dm-multipath']:
> please use shortest branch first
Done
Line 116:                 blivetEnv.initializeDisk(dev)
Line 117:                 part = blivetEnv.newPartition(fmt_type="lvmpv", grow=True,
Line 118:                                               parents=[dev])
Line 119:                 blivetEnv.createDevice(part)


Line 162:         return vg
Line 163: 
Line 164:     def _createThinPool(poolName, vg, alignment=0,
Line 165:                         poolMetaDataSize=0, poolDataSize=0):
Line 166:         if alignment:
> Please use shortest branch first.
Done
Line 167:             metaName = "meta-%s" % poolName
Line 168:             vgPoolName = "%s/%s" % (vg.name, poolName)
Line 169:             metaLv = LVMLogicalVolumeDevice(
Line 170:                 metaName, parents=[vg],


Line 249:     # /#chap-Configuring_Red_Hat_Storage_for_Enhancing_Performance
Line 250: 
Line 251:     if alignment:
Line 252:         vgSizeKib = int(vg.size.convertTo(spec="KiB"))
Line 253:         if vg.size.convertTo(spec='MiB') < MIN_VG_SIZE:
> What version of blivet is required for this?  I think you need to update de
Done
Line 254:             metaDataSize = vgSizeKib * MIN_METADATA_PERCENT
Line 255:         poolDataSize = vgSizeKib - metaDataSize
Line 256:         metaDataSize = (metaDataSize - (metaDataSize % alignment))
Line 257:         poolDataSize = (poolDataSize - (poolDataSize % alignment))


Line 276:     blivetEnv.formatDevice(thinlv, format)
Line 277:     blivetEnv.doIt()
Line 278: 
Line 279:     if not os.path.exists(mountPoint):
Line 280:         os.makedirs(mountPoint)
> Please use makedirs and handle the exception instead of testing the directo
Done
Line 281:     thinlv.format.setup(mountpoint=mountPoint)
Line 282:     blivetEnv.doIt()
Line 283:     fstab.FsTab().add(thinlv.path, mountPoint, DEFAULT_FS_TYPE)


-- 
To view, visit https://gerrit.ovirt.org/35498
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic47c4c56834deb457ae9d038f77bcf69c7b39ba5
Gerrit-PatchSet: 24
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: Federico Simoncelli <fsimonce 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