Change in vdsm[ovirt-3.5-gluster]: gluster: add _getDeviceDict function to reuse common code

dnarayan at redhat.com dnarayan at redhat.com
Thu Apr 16 10:14:08 UTC 2015


Hello Piotr Kliczewski, Timothy Asir, Bala.FA, Dan Kenigsberg,

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

    https://gerrit.ovirt.org/39920

to review the following change.

Change subject: gluster: add _getDeviceDict function to reuse common code
......................................................................

gluster: add _getDeviceDict function to reuse common code

Added internal function to return device dictonary which can be
used for storageDevicesList and any subsequent function.
Updated _canCreateBrick internal function to return False for
the device which type belongs to 'lvmthinlv'
Added device format uuid into storageDeviceList which was
missing in the existing function.

Change-Id: I25221e4266a4e14499c45916c5a4c9b723821bfb
Signed-off-by: Timothy Asir <tjeyasin at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/37643
Reviewed-by: Bala.FA <barumuga at redhat.com>
Reviewed-by: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/glusterTestData.py
M vdsm/gluster/storagedev.py
2 files changed, 37 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/20/39920/1

diff --git a/tests/glusterTestData.py b/tests/glusterTestData.py
index 8e588da..165c1c3 100644
--- a/tests/glusterTestData.py
+++ b/tests/glusterTestData.py
@@ -842,14 +842,14 @@
                             'uuid': ''},
                            {'createBrick': False,
                             'devPath': '/dev/vda5',
-                            'devUuid': '',
+                            'devUuid': 'ee18ca64-806a-4a84-b69e-1f939265fa68',
                             'bus': '',
                             'fsType': 'xfs',
                             'model': 'Virtio Block Device (partition)',
                             'mountPoint': '/',
                             'name': 'vda5',
                             'size': '1024.0',
-                            'uuid': ''},
+                            'uuid': '12345-12345-12345'},
                            {'createBrick': False,
                             'devPath': '/dev/vda4',
                             'devUuid': '',
@@ -859,7 +859,7 @@
                             'mountPoint': '',
                             'name': 'vda4',
                             'size': '1024.0',
-                            'uuid': ''},
+                            'uuid': '12344-12344-12344'},
                            {'createBrick': True,
                             'devPath': '/dev/vda3',
                             'devUuid': '',
@@ -920,6 +920,7 @@
     device.type = "disk"
     device.format = TestStorageDev()
     device.format.type = None
+    device.format.uuid = None
     devices.append(device)
 
     # Check model
@@ -953,13 +954,14 @@
     device.type = "disk"
     device.format = TestStorageDev()
     device.format.type = None
+    device.format.uuid = None
     devices.append(device)
 
     device = TestStorageDev()
     device.name = 'vda5'
     device.size = 1024.0
     device.path = '/dev/vda5'
-    device.uuid = None
+    device.uuid = 'ee18ca64-806a-4a84-b69e-1f939265fa68'
     device.parents = []
     device.bus = ''
     device.kids = 0
@@ -967,8 +969,8 @@
     device.type = "partition"
     device.format = TestStorageDev()
     device.format.type = 'xfs'
+    device.format.uuid = "12345-12345-12345"
     device.format.mountpoint = '/'
-    device.format.uuid = 'ee18ca64-806a-4a84-b69e-1f939265fa68'
     devices.append(device)
 
     # check fstype, no mountpoint attribute
@@ -984,7 +986,7 @@
     device.type = "partition"
     device.format = TestStorageDev()
     device.format.type = 'xfs'
-    device.format.uuid = None
+    device.format.uuid = "12344-12344-12344"
     devices.append(device)
 
     # check empty mountpoint
@@ -1016,6 +1018,7 @@
     device.type = "disk"
     device.format = TestStorageDev()
     device.format.type = None
+    device.format.uuid = None
     devices.append(device)
 
     # device type cd-rom
@@ -1048,7 +1051,6 @@
     device.format = TestStorageDev()
     device.format.type = None
     device.format.uuid = None
-    device.format.type = None
     devices.append(device)
     return devices
 
diff --git a/vdsm/gluster/storagedev.py b/vdsm/gluster/storagedev.py
index ff50a61..8d0f986 100644
--- a/vdsm/gluster/storagedev.py
+++ b/vdsm/gluster/storagedev.py
@@ -1,5 +1,5 @@
 #
-# Copyright 2014 Red Hat, Inc.
+# Copyright 2015 Red Hat, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -22,39 +22,42 @@
 from . import makePublic
 
 
+def _getDeviceDict(device, createBrick=False):
+    info = {'name': device.name,
+            'size': '%s' % device.size,
+            'devPath': device.path,
+            'devUuid': device.uuid or '',
+            'bus': device.bus or '',
+            'model': '',
+            'fsType': '',
+            'mountPoint': '',
+            'uuid': '',
+            'createBrick': createBrick}
+    if not info['bus'] and device.parents:
+        info['bus'] = device.parents[0].bus
+    if device.model:
+        info['model'] = "%s (%s)" % (device.model, device.type)
+    else:
+        info['model'] = device.type
+    if device.format:
+        info['uuid'] = device.format.uuid or ''
+        info['fsType'] = device.format.type or ''
+    if hasattr(device.format, 'mountpoint'):
+        info['mountPoint'] = device.format.mountpoint or ''
+    return info
+
+
 def _parseDevices(devices):
     deviceList = []
     for device in devices:
-        info = {'name': device.name,
-                'size': '%s' % device.size,
-                'devPath': device.path,
-                'devUuid': device.uuid or '',
-                'bus': device.bus or '',
-                'model': '',
-                'fsType': '',
-                'mountPoint': '',
-                'uuid': '',
-                'createBrick': True}
-        if not info['bus'] and device.parents:
-            info['bus'] = device.parents[0].bus
-        if device.model:
-            info['model'] = "%s (%s)" % (device.model, device.type)
-        else:
-            info['model'] = device.type
-        if device.format:
-            info['fsType'] = device.format.type or ''
-        if hasattr(device.format, 'mountpoint'):
-            info['mountPoint'] = device.format.mountpoint or ''
-        info['createBrick'] = _canCreateBrick(device)
-
-        deviceList.append(info)
+        deviceList.append(_getDeviceDict(device, _canCreateBrick(device)))
     return deviceList
 
 
 def _canCreateBrick(device):
     if not device or device.kids > 0 or device.format.type or \
        hasattr(device.format, 'mountpoint') or \
-       device.type in ['cdrom', 'lvmvg', 'lvmthinpool', 'lvmlv']:
+       device.type in ['cdrom', 'lvmvg', 'lvmthinpool', 'lvmlv', 'lvmthinlv']:
         return False
     return True
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I25221e4266a4e14499c45916c5a4c9b723821bfb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5-gluster
Gerrit-Owner: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin at redhat.com>


More information about the vdsm-patches mailing list