Change in vdsm[master]: gluster: fix size conversion issues in brick create

[automation]

gerrit-hooks has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 7:

* #1270792::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1270792::OK, public bug
* Check Product::#1270792::SKIPPED, Wrong product Red Hat Gluster Storage and 
classification Red Hat not oVirt
* Check Product::WARN, no bug url with correct product found, make sure you 
have at least one bug-url with a product in oVirt Red Hat Enterprise 
Virtualization Manager.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 7: Code-Review+1

Waiting for Sahina ack.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[rnachimu]

Ramesh N has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 7: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[sabose]

Sahina Bose has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 7: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[automation]

gerrit-hooks has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 8:

* #1270792::Update tracker: OK
* Set MODIFIED::bug 1270792#1270792IGNORE, not oVirt classification but 
Red Hat

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: gluster: fix size conversion issues in brick create
..


gluster: fix size conversion issues in brick create

 There is change in blivet API device.size.convertTo(spec).
spec was a string parameter in older versions of blivet but its changed
to constants in newer versions. This patch address this issue
in a generic way so that it can run on all releases.

Bug-Url: https://bugzilla.redhat.com/1270792
Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Signed-off-by: Ramesh Nachimuthu 
Reviewed-on: https://gerrit.ovirt.org/50705
Continuous-Integration: Jenkins CI
Reviewed-by: Sahina Bose 
Reviewed-by: Nir Soffer 
---
M vdsm/gluster/storagedev.py
1 file changed, 24 insertions(+), 9 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, approved
  Jenkins CI: Passed CI tests
  Sahina Bose: Looks good to me, but someone else must approve
  Ramesh N: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 6: Code-Review-1

(5 comments)

https://gerrit.ovirt.org/#/c/50705/6/vdsm/gluster/storagedev.py
File vdsm/gluster/storagedev.py:

Line 65: 
Line 66: # This method helps to convert the size to given Unittype.
Line 67: # This is required to since there a incompatible change in blivet API
Line 68: # size.convertTo where older versions required string param but newer 
versions
Line 69: # requires the constants in blivet.size.KiB.
blivet.size.KiB -> blivet.size.
Line 70: def _getDeviceSize(device, unitType='MiB'):
Line 71: if hasattr(blivet.size, 'MiB'):
Line 72: if unitType == 'KiB':
Line 73: return device.size.convertTo(blivet.size.KiB)


Line 66: # This method helps to convert the size to given Unittype.
Line 67: # This is required to since there a incompatible change in blivet API
Line 68: # size.convertTo where older versions required string param but newer 
versions
Line 69: # requires the constants in blivet.size.KiB.
Line 70: def _getDeviceSize(device, unitType='MiB'):
Add a note that this part is for the new blivet version.
Line 71: if hasattr(blivet.size, 'MiB'):
Line 72: if unitType == 'KiB':
Line 73: return device.size.convertTo(blivet.size.KiB)
Line 74: elif unitType == 'MiB':


Line 71: if hasattr(blivet.size, 'MiB'):
Line 72: if unitType == 'KiB':
Line 73: return device.size.convertTo(blivet.size.KiB)
Line 74: elif unitType == 'MiB':
Line 75: return device.size.convertTo(blivet.size.MiB)
This should raise, the caller provided unitType we don't know how to handle.
Line 76: else:
Line 77: return device.size.convertTo(spec=unitType)
Line 78: return 0
Line 79: 


Line 72: if unitType == 'KiB':
Line 73: return device.size.convertTo(blivet.size.KiB)
Line 74: elif unitType == 'MiB':
Line 75: return device.size.convertTo(blivet.size.MiB)
Line 76: else:
Add a note that this is for old blivet version supporting spec string.
Line 77: return device.size.convertTo(spec=unitType)
Line 78: return 0
Line 79: 
Line 80: 


Line 74: elif unitType == 'MiB':
Line 75: return device.size.convertTo(blivet.size.MiB)
Line 76: else:
Line 77: return device.size.convertTo(spec=unitType)
Line 78: return 0
Not needed, and a bad way to fail, now all the callers have to check for 0, or 
we will fail much later when someone try to use the invalid size, making 
debugging much harder.

I suggest to do this:

if hasattr(blivet.size, 'MiB'):
# New blivet requires size constants (e.g, size.MiB, size.KiB)
unit = getattr(blivet.size, unitType)
return device.size.convertTo(unit)
else:
   # Old blivet using spec string
   return device.size.convertTo(spec=unitType)

This will raise AttributeError if you call with invalid unitType, which should 
make this easy to debug.
Line 79: 
Line 80: 
Line 81: def _getDeviceDict(device, createBrick=False):
Line 82: info = {'name': device.name,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[rnachimu]

Ramesh N has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 6:

(5 comments)

Thanks for your suggestions Nir. I will fix them in the next patch set.

https://gerrit.ovirt.org/#/c/50705/6/vdsm/gluster/storagedev.py
File vdsm/gluster/storagedev.py:

Line 65: 
Line 66: # This method helps to convert the size to given Unittype.
Line 67: # This is required to since there a incompatible change in blivet API
Line 68: # size.convertTo where older versions required string param but newer 
versions
Line 69: # requires the constants in blivet.size.KiB.
> blivet.size.KiB -> blivet.size.
Done
Line 70: def _getDeviceSize(device, unitType='MiB'):
Line 71: if hasattr(blivet.size, 'MiB'):
Line 72: if unitType == 'KiB':
Line 73: return device.size.convertTo(blivet.size.KiB)


Line 66: # This method helps to convert the size to given Unittype.
Line 67: # This is required to since there a incompatible change in blivet API
Line 68: # size.convertTo where older versions required string param but newer 
versions
Line 69: # requires the constants in blivet.size.KiB.
Line 70: def _getDeviceSize(device, unitType='MiB'):
> Add a note that this part is for the new blivet version.
Done
Line 71: if hasattr(blivet.size, 'MiB'):
Line 72: if unitType == 'KiB':
Line 73: return device.size.convertTo(blivet.size.KiB)
Line 74: elif unitType == 'MiB':


Line 71: if hasattr(blivet.size, 'MiB'):
Line 72: if unitType == 'KiB':
Line 73: return device.size.convertTo(blivet.size.KiB)
Line 74: elif unitType == 'MiB':
Line 75: return device.size.convertTo(blivet.size.MiB)
> This should raise, the caller provided unitType we don't know how to handle
Done
Line 76: else:
Line 77: return device.size.convertTo(spec=unitType)
Line 78: return 0
Line 79: 


Line 72: if unitType == 'KiB':
Line 73: return device.size.convertTo(blivet.size.KiB)
Line 74: elif unitType == 'MiB':
Line 75: return device.size.convertTo(blivet.size.MiB)
Line 76: else:
> Add a note that this is for old blivet version supporting spec string.
Done
Line 77: return device.size.convertTo(spec=unitType)
Line 78: return 0
Line 79: 
Line 80: 


Line 74: elif unitType == 'MiB':
Line 75: return device.size.convertTo(blivet.size.MiB)
Line 76: else:
Line 77: return device.size.convertTo(spec=unitType)
Line 78: return 0
> Not needed, and a bad way to fail, now all the callers have to check for 0,
Done
Line 79: 
Line 80: 
Line 81: def _getDeviceDict(device, createBrick=False):
Line 82: info = {'name': device.name,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[rnachimu]

Ramesh N has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 5:

(7 comments)

https://gerrit.ovirt.org/#/c/50705/5/vdsm/gluster/storagedev.py
File vdsm/gluster/storagedev.py:

Line 73
Line 74
Line 75
Line 76
Line 77
> What is wrong with this?
device.size.convertTo(spec="MiB") doesn't work anymore. Spec should be passed 
using the constant from blivet.size like blivet.size.KiB. But older versions 
which is still being shipped in RHEL7.2 supports only the old way of passing 
spec as string.


Line 251
Line 252
Line 253
Line 254
Line 255
> metaDataSize is in Kib?
yes. Changed the name to metaDataSizeKib


Line 66: def _getDeviceSize(device, unitType=None):
Line 67: if hasattr(blivet.size, 'MiB'):
Line 68: if unitType == 'KiB':
Line 69: return device.size.convertTo(blivet.size.KiB)
Line 70: else:
> What if unitType is None? are you assuming that unitType is 'MiB' in this c
UnitType is assumed as MiB incase of None.
Line 71: return device.size.convertTo(blivet.size.MiB)
Line 72: else:
Line 73: if unitType == 'KiB':
Line 74: return device.size.convertTo(spec='KiB')


Line 68: if unitType == 'KiB':
Line 69: return device.size.convertTo(blivet.size.KiB)
Line 70: else:
Line 71: return device.size.convertTo(blivet.size.MiB)
Line 72: else:
> What does it mean when blivet.size does.MiB is not available?
This is due to a change in Blivet. Older versions of blivet requires unit spec 
to be string parameter. But newer version requires the constants defined in 
blivet.size.
Line 73: if unitType == 'KiB':
Line 74: return device.size.convertTo(spec='KiB')
Line 75: else:
Line 76: return device.size.convertTo(spec='MiB')


Line 71: return device.size.convertTo(blivet.size.MiB)
Line 72: else:
Line 73: if unitType == 'KiB':
Line 74: return device.size.convertTo(spec='KiB')
Line 75: else:
> What if unitType is None?
Its defaulted to MiB. May be I will change the default value to MiB instead of 
None.
Line 76: return device.size.convertTo(spec='MiB')
Line 77: return 0
Line 78: 
Line 79: 


Line 73: if unitType == 'KiB':
Line 74: return device.size.convertTo(spec='KiB')
Line 75: else:
Line 76: return device.size.convertTo(spec='MiB')
Line 77: return 0
> 0? Maybe you want to raise an error?
Done
Line 78: 
Line 79: 
Line 80: def _getDeviceDict(device, createBrick=False):
Line 81: info = {'name': device.name,


Line 87: 'mountPoint': '',
Line 88: 'uuid': '',
Line 89: 'createBrick': createBrick}
Line 90: if isinstance(device.size, blivet.size.Size):
Line 91: info['size'] = '%s' % _getDeviceSize(device)
> Using this without specifying the unit does not seem like a good idea, unle
Yes. MiB is the default here.
Line 92: else:
Line 93: info['size'] = '%s' % device.size
Line 94: if not info['bus'] and device.parents:
Line 95: info['bus'] = device.parents[0].bus


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[automation]

gerrit-hooks has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 6:

* #1270792::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1270792::OK, public bug
* Check Product::#1270792::SKIPPED, Wrong product Red Hat Gluster Storage and 
classification Red Hat not oVirt
* Check Product::WARN, no bug url with correct product found, make sure you 
have at least one bug-url with a product in oVirt Red Hat Enterprise 
Virtualization Manager.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[rnachimu]

Ramesh N has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 6: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[rnachimu]

Ramesh N has uploaded a new change for review.

Change subject: gluster: fix size conversion issues in brick create
..

gluster: fix size conversion issues in brick create

 There is change in blivet API device.size.convertTo(spec).
spec was a string parameter in older versions of blivet but its changed
to constants in newer versions. This patch address this issue
in a generic way so that it can run on all releases.

Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Signed-off-by: Ramesh Nachimuthu 
---
M vdsm/gluster/storagedev.py
1 file changed, 17 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/05/50705/1

diff --git a/vdsm/gluster/storagedev.py b/vdsm/gluster/storagedev.py
index ff39b80..9844cb0 100644
--- a/vdsm/gluster/storagedev.py
+++ b/vdsm/gluster/storagedev.py
@@ -65,6 +65,20 @@
 DEFAULT_MOUNT_OPTIONS = "inode64,noatime"
 
 
+def _getDeviceSize(device, unitType=None):
+if hasattr(blivet.size, 'MiB'):
+if unitType == 'KiB':
+return device.size.convertTo(blivet.size.KiB)
+else:
+return device.size.convertTo(blivet.size.MiB)
+else:
+if unitType == 'KiB':
+return device.size.convertTo(spec='KiB')
+else:
+return device.size.convertTo(spec='MiB')
+return 0
+
+
 def _getDeviceDict(device, createBrick=False):
 info = {'name': device.name,
 'devPath': device.path,
@@ -76,7 +90,7 @@
 'uuid': '',
 'createBrick': createBrick}
 if isinstance(device.size, blivet.size.Size):
-info['size'] = '%s' % device.size.convertTo(spec="MiB")
+info['size'] = '%s' % _getDeviceSize(device)
 else:
 info['size'] = '%s' % device.size
 if not info['bus'] and device.parents:
@@ -252,8 +266,8 @@
 # and create data LV (poolDataSize) that has a size which is
 # a multiple of stripe width
 # For JBOD, this adjustment is not necessary
-vgSizeKib = int(vg.size.convertTo(spec="KiB"))
-if vg.size.convertTo(spec='MiB') < MIN_VG_SIZE:
+vgSizeKib = int(_getDeviceSize(vg, 'KiB'))
+if  _getDeviceSize(vg) < MIN_VG_SIZE:
 metaDataSize = vgSizeKib * MIN_METADATA_PERCENT
 poolDataSize = vgSizeKib - metaDataSize
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[automation]

gerrit-hooks has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[automation]

gerrit-hooks has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[sabose]

Sahina Bose has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[automation]

gerrit-hooks has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[automation]

gerrit-hooks has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 4:

* #1270792::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1270792::OK, public bug
* Check Product::#1270792::SKIPPED, Wrong product Red Hat Gluster Storage and 
classification Red Hat not oVirt
* Check Product::WARN, no bug url with correct product found, make sure you 
have at least one bug-url with a product in oVirt Red Hat Enterprise 
Virtualization Manager.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[automation]

gerrit-hooks has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 5:

* #1270792::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1270792::OK, public bug
* Check Product::#1270792::SKIPPED, Wrong product Red Hat Gluster Storage and 
classification Red Hat not oVirt
* Check Product::WARN, no bug url with correct product found, make sure you 
have at least one bug-url with a product in oVirt Red Hat Enterprise 
Virtualization Manager.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[rnachimu]

Ramesh N has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 5: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: gluster: fix size conversion issues in brick create

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: gluster: fix size conversion issues in brick create
..


Patch Set 5:

(7 comments)

Thanks for separating this. Please check the questions in the code.

https://gerrit.ovirt.org/#/c/50705/5/vdsm/gluster/storagedev.py
File vdsm/gluster/storagedev.py:

Line 73
Line 74
Line 75
Line 76
Line 77
What is wrong with this?


Line 251
Line 252
Line 253
Line 254
Line 255
metaDataSize is in Kib?


Line 66: def _getDeviceSize(device, unitType=None):
Line 67: if hasattr(blivet.size, 'MiB'):
Line 68: if unitType == 'KiB':
Line 69: return device.size.convertTo(blivet.size.KiB)
Line 70: else:
What if unitType is None? are you assuming that unitType is 'MiB' in this case?
Line 71: return device.size.convertTo(blivet.size.MiB)
Line 72: else:
Line 73: if unitType == 'KiB':
Line 74: return device.size.convertTo(spec='KiB')


Line 68: if unitType == 'KiB':
Line 69: return device.size.convertTo(blivet.size.KiB)
Line 70: else:
Line 71: return device.size.convertTo(blivet.size.MiB)
Line 72: else:
What does it mean when blivet.size does.MiB is not available?
Line 73: if unitType == 'KiB':
Line 74: return device.size.convertTo(spec='KiB')
Line 75: else:
Line 76: return device.size.convertTo(spec='MiB')


Line 71: return device.size.convertTo(blivet.size.MiB)
Line 72: else:
Line 73: if unitType == 'KiB':
Line 74: return device.size.convertTo(spec='KiB')
Line 75: else:
What if unitType is None?
Line 76: return device.size.convertTo(spec='MiB')
Line 77: return 0
Line 78: 
Line 79: 


Line 73: if unitType == 'KiB':
Line 74: return device.size.convertTo(spec='KiB')
Line 75: else:
Line 76: return device.size.convertTo(spec='MiB')
Line 77: return 0
0? Maybe you want to raise an error?
Line 78: 
Line 79: 
Line 80: def _getDeviceDict(device, createBrick=False):
Line 81: info = {'name': device.name,


Line 87: 'mountPoint': '',
Line 88: 'uuid': '',
Line 89: 'createBrick': createBrick}
Line 90: if isinstance(device.size, blivet.size.Size):
Line 91: info['size'] = '%s' % _getDeviceSize(device)
Using this without specifying the unit does not seem like a good idea, unless 
MiB is the default, and all values are always converted to MiB. Is this the 
case?
Line 92: else:
Line 93: info['size'] = '%s' % device.size
Line 94: if not info['bus'] and device.parents:
Line 95: info['bus'] = device.parents[0].bus


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I576b1a8c880ef6f355157225c7b763378d8cf46d
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ramesh N 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches