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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Ramesh N <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
