Change in vdsm[ovirt-3.5-gluster]: gluster: fix alignment issue for brick creation on JBOD

rnachimu at redhat.com rnachimu at redhat.com
Mon Dec 21 14:03:30 UTC 2015


Hello Nir Soffer, Sahina Bose,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/50806

to review the following change.

Change subject: gluster: fix alignment issue for brick creation on JBOD
......................................................................

gluster: fix alignment issue for brick creation on JBOD

  Chunk size and alignment value should be 256KB for
 JBOD use case. So assigining these values incase of
JBOD.

 Note: blivet doesn't support alignment values in PV,VG,
Thinpool creation. So we were using blivet API only when i
we don't have any alignment and LVM commands directly when
we have alignment values. Now, with this patch we will
use lvm commands always as we have alignment values in all
the cases and Blivet API usage can be removed completely
in PV, Thin pool, VG creation.

Bug-Url: https://bugzilla.redhat.com/1270792
Signed-off-by: Ramesh Nachimuthu <rnachimu at redhat.com>
Change-Id: I58cc322cb5140de2d2006d59b4c1dceaba2e5968
Signed-off-by: Timothy Asir Jeyasingh <asirtim at yahoo.com>
Reviewed-on: https://gerrit.ovirt.org/47959
Continuous-Integration: Jenkins CI
Reviewed-by: Sahina Bose <sabose at redhat.com>
Reviewed-by: Nir Soffer <nsoffer at redhat.com>
---
M vdsm/gluster/storagedev.py
1 file changed, 72 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/06/50806/1

diff --git a/vdsm/gluster/storagedev.py b/vdsm/gluster/storagedev.py
index 93219c2..f19d959 100644
--- a/vdsm/gluster/storagedev.py
+++ b/vdsm/gluster/storagedev.py
@@ -26,8 +26,6 @@
 import blivet.formats
 import blivet.formats.fs
 import blivet.size
-from blivet.devices import LVMVolumeGroupDevice
-from blivet.devices import LVMThinPoolDevice
 from blivet.devices import LVMLogicalVolumeDevice
 from blivet.devices import LVMThinLogicalVolumeDevice
 from blivet import udev
@@ -137,114 +135,89 @@
         return [blivetEnv.devicetree.getDeviceByName(devName.split("/")[-1])
                 for devName in devNameList]
 
-    def _createPV(deviceList, alignment=0):
-        def _createAlignedPV(deviceList, alignment):
-            for dev in deviceList:
-                # bz#1178705: Blivet always creates pv with 1MB dataalignment
-                # Workaround: Till blivet fixes the issue, we use lvm pvcreate
-                rc, out, err = utils.execCmd([_pvCreateCommandPath.cmd,
-                                              '--dataalignment',
-                                              '%sk' % alignment,
-                                              dev.path])
-                if rc:
-                    raise ge.GlusterHostStorageDevicePVCreateFailedException(
-                        dev.path, alignment, rc, out, err)
-            _reset_blivet(blivetEnv)
-            return _getDeviceList([dev.name for dev in deviceList])
-
-        if alignment:
-            blivetEnv.doIt()
-            return _createAlignedPV(deviceList, alignment)
-
+    def _createPV(deviceList, alignment):
         for dev in deviceList:
-            lvmpv = blivet.formats.getFormat("lvmpv", device=dev.path)
-            blivetEnv.formatDevice(dev, lvmpv)
-
-        blivet.partitioning.doPartitioning(blivetEnv)
-        return deviceList
-
-    def _createVG(vgName, deviceList, stripeSize=0):
-        if stripeSize:
-            # bz#1198568: Blivet always creates vg with 1MB stripe size
-            # Workaround: Till blivet fixes the issue, use vgcreate command
-            devices = ','.join([device.path for device in deviceList])
-            rc, out, err = utils.execCmd([_vgCreateCommandPath.cmd,
-                                          '-s', '%sk' % stripeSize,
-                                          vgName, devices])
+            # bz#1178705: Blivet always creates pv with 1MB dataalignment
+            # Workaround: Till blivet fixes the issue, we use lvm pvcreate
+            rc, out, err = utils.execCmd([_pvCreateCommandPath.cmd,
+                                          '--dataalignment',
+                                          '%sk' % alignment,
+                                          dev.path])
             if rc:
-                raise ge.GlusterHostStorageDeviceVGCreateFailedException(
-                    vgName, devices, stripeSize, rc, out, err)
-            blivetEnv.reset()
-            vg = blivetEnv.devicetree.getDeviceByName(vgName)
-        else:
-            vg = LVMVolumeGroupDevice(vgName, parents=deviceList)
-            blivetEnv.createDevice(vg)
-        return vg
+                raise ge.GlusterHostStorageDevicePVCreateFailedException(
+                    dev.path, alignment, rc, out, err)
+        _reset_blivet(blivetEnv)
+        return _getDeviceList([dev.name for dev in deviceList])
 
-    def _createThinPool(poolName, vg, alignment=0,
-                        poolMetaDataSize=0, poolDataSize=0):
-        if not alignment:
-            # bz#1180228: blivet doesn't handle percentage-based sizes properly
-            # Workaround: Till the bz gets fixed, we take only 99% size from vg
-            pool = LVMThinPoolDevice(poolName, parents=[vg],
-                                     size=(vg.size * 99 / 100),
-                                     grow=True)
-            blivetEnv.createDevice(pool)
-            return pool
-        else:
-            metaName = "meta-%s" % poolName
-            vgPoolName = "%s/%s" % (vg.name, poolName)
-            metaLv = LVMLogicalVolumeDevice(
-                metaName, parents=[vg],
-                size=blivet.size.Size('%d KiB' % poolMetaDataSize))
-            poolLv = LVMLogicalVolumeDevice(
-                poolName, parents=[vg],
-                size=blivet.size.Size('%d KiB' % poolDataSize))
-            blivetEnv.createDevice(metaLv)
-            blivetEnv.createDevice(poolLv)
-            blivetEnv.doIt()
+    def _createVG(vgName, deviceList, stripeSize):
+        # bz#1198568: Blivet always creates vg with 1MB stripe size
+        # Workaround: Till blivet fixes the issue, use vgcreate command
+        devices = ','.join([device.path for device in deviceList])
+        rc, out, err = utils.execCmd([_vgCreateCommandPath.cmd,
+                                      '-s', '%sk' % stripeSize,
+                                      vgName, devices])
+        if rc:
+            raise ge.GlusterHostStorageDeviceVGCreateFailedException(
+                vgName, devices, stripeSize, rc, out, err)
+        blivetEnv.reset()
+        return blivetEnv.devicetree.getDeviceByName(vgName)
 
-            # bz#1100514: LVM2 currently only supports physical extent sizes
-            # that are a power of 2. Till that support is available we need
-            # to use lvconvert to achive that.
-            # bz#1179826: blivet doesn't support lvconvert functionality.
-            # Workaround: Till the bz gets fixed, lvconvert command is used
-            rc, out, err = utils.execCmd([_lvconvertCommandPath.cmd,
-                                          '--chunksize', '%sK' % alignment,
-                                          '--thinpool', vgPoolName,
-                                          '--poolmetadata',
-                                          "%s/%s" % (vg.name, metaName),
-                                          '--poolmetadataspar', 'n', '-y'])
+    def _createThinPool(poolName, vg, alignment,
+                        poolMetaDataSize, poolDataSize):
+        metaName = "meta-%s" % poolName
+        vgPoolName = "%s/%s" % (vg.name, poolName)
+        metaLv = LVMLogicalVolumeDevice(
+            metaName, parents=[vg],
+            size=blivet.size.Size('%d KiB' % poolMetaDataSize))
+        poolLv = LVMLogicalVolumeDevice(
+            poolName, parents=[vg],
+            size=blivet.size.Size('%d KiB' % poolDataSize))
+        blivetEnv.createDevice(metaLv)
+        blivetEnv.createDevice(poolLv)
+        blivetEnv.doIt()
 
-            if rc:
-                raise ge.GlusterHostStorageDeviceLVConvertFailedException(
-                    vg.path, alignment, rc, out, err)
+        # bz#1100514: LVM2 currently only supports physical extent sizes
+        # that are a power of 2. Till that support is available we need
+        # to use lvconvert to achive that.
+        # bz#1179826: blivet doesn't support lvconvert functionality.
+        # Workaround: Till the bz gets fixed, lvconvert command is used
+        rc, out, err = utils.execCmd([_lvconvertCommandPath.cmd,
+                                      '--chunksize', '%sK' % alignment,
+                                      '--thinpool', vgPoolName,
+                                      '--poolmetadata',
+                                      "%s/%s" % (vg.name, metaName),
+                                      '--poolmetadataspar', 'n', '-y'])
 
-            rc, out, err = utils.execCmd([_lvchangeCommandPath.cmd,
-                                          '--zero', 'n', vgPoolName])
-            if rc:
-                raise ge.GlusterHostStorageDeviceLVChangeFailedException(
-                    vgPoolName, rc, out, err)
-            _reset_blivet(blivetEnv)
-            return blivetEnv.devicetree.getDeviceByName(poolLv.name)
+        if rc:
+            raise ge.GlusterHostStorageDeviceLVConvertFailedException(
+                vg.path, alignment, rc, out, err)
+        rc, out, err = utils.execCmd([_lvchangeCommandPath.cmd,
+                                      '--zero', 'n', vgPoolName])
+        if rc:
+            raise ge.GlusterHostStorageDeviceLVChangeFailedException(
+                vgPoolName, rc, out, err)
+        _reset_blivet(blivetEnv)
+        return blivetEnv.devicetree.getDeviceByName(poolLv.name)
 
     if os.path.ismount(mountPoint):
         raise ge.GlusterHostStorageMountPointInUseException(mountPoint)
 
     vgName = "vg-" + brickName
     poolName = "pool-" + brickName
-    alignment = 0
-    chunkSize = 0
     poolDataSize = 0
     count = 0
+    raidType = raidParams.get('type')
     metaDataSize = DEFAULT_METADATA_SIZE_KB
-    if raidParams.get('type') == '6':
+    if raidType == '6':
         count = raidParams['pdCount'] - 2
         alignment = raidParams['stripeSize'] * count
         chunkSize = alignment
-    elif raidParams.get('type') == '10':
+    elif raidType == '10':
         count = raidParams['pdCount'] / 2
         alignment = raidParams['stripeSize'] * count
+        chunkSize = DEFAULT_CHUNK_SIZE_KB
+    else:  # Device type is JBOD
+        alignment = DEFAULT_CHUNK_SIZE_KB
         chunkSize = DEFAULT_CHUNK_SIZE_KB
 
     blivetEnv = blivet.Blivet()
@@ -266,8 +239,7 @@
         raise ge.GlusterHostStorageDeviceInUseException(inUseList)
 
     pvDeviceList = _createPV(deviceList, alignment)
-    vg = _createVG(vgName, pvDeviceList, raidParams.get('stripeSize', 0))
-
+    vg = _createVG(vgName, pvDeviceList, alignment)
     # The following calculation is based on the redhat storage performance doc
     # http://docbuilder.usersys.redhat.com/22522
     # /#chap-Configuring_Red_Hat_Storage_for_Enhancing_Performance
@@ -277,11 +249,13 @@
     # otherwise allocate a minimum of 0.5% of the data device size
     # and create data LV (poolDataSize) that has a size which is
     # a multiple of stripe width
-    if alignment:
-        vgSizeKib = int(vg.size.convertTo(spec="KiB"))
-        if vg.size.convertTo(spec='MiB') < MIN_VG_SIZE:
-            metaDataSize = vgSizeKib * MIN_METADATA_PERCENT
-        poolDataSize = vgSizeKib - metaDataSize
+    # For JBOD, this adjustment is not necessary
+    vgSizeKib = int(vg.size.convertTo(spec="KiB"))
+    if vg.size.convertTo(spec='MiB') < MIN_VG_SIZE:
+        metaDataSize = vgSizeKib * MIN_METADATA_PERCENT
+    poolDataSize = vgSizeKib - metaDataSize
+
+    if raidType:
         metaDataSize = (metaDataSize - (metaDataSize % alignment))
         poolDataSize = (poolDataSize - (poolDataSize % alignment))
 
@@ -290,7 +264,7 @@
     #     --poolmetadata VOLGROUP/metadata_device_name
     pool = _createThinPool(poolName, vg, chunkSize, metaDataSize, poolDataSize)
     thinlv = LVMThinLogicalVolumeDevice(brickName, parents=[pool],
-                                        size=pool.size, grow=True)
+                                        size=vg.size, grow=True)
     blivetEnv.createDevice(thinlv)
     blivetEnv.doIt()
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I58cc322cb5140de2d2006d59b4c1dceaba2e5968
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5-gluster
Gerrit-Owner: Ramesh N <rnachimu at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sahina Bose <sabose at redhat.com>


More information about the vdsm-patches mailing list