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 <rnach...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Ramesh N <rnach...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
