Change in vdsm[master]: vm: _normalizeVdsmImg refactoring
Federico Simoncelli has posted comments on this change. Change subject: vm: _normalizeVdsmImg refactoring .. Patch Set 3: (1 comment) http://gerrit.ovirt.org/#/c/19157/3//COMMIT_MSG Commit Message: Line 4: Commit: Federico Simoncelli Line 5: CommitDate: 2014-02-21 05:43:42 -0500 Line 6: Line 7: vm: _normalizeVdsmImg refactoring Line 8: Commit message to be improved once we decide to keep this. Line 9: Change-Id: Ie68292eee4b82fbe8527e3960739979cfe117dfa -- To view, visit http://gerrit.ovirt.org/19157 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie68292eee4b82fbe8527e3960739979cfe117dfa Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Miguel Angel Ajo Pelayo Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: _normalizeVdsmImg refactoring
Federico Simoncelli has posted comments on this change. Change subject: vm: _normalizeVdsmImg refactoring .. Patch Set 3: (1 comment) http://gerrit.ovirt.org/#/c/19157/3/vdsm/vm.py File vdsm/vm.py: Line 2040: drive['device'] = drive.get('device', 'disk') # Disk by default Line 2041: drive['reqsize'] = drive.get('reqsize', '0') # Backward compatible Line 2042: Line 2043: if drive['device'] == 'disk': Line 2044: volInfo = self.cif.irs.getVolumeInfo( I'll split this change out. Line 2045: drive['domainID'], drive['poolID'], drive['imageID'], Line 2046: drive['volumeID']) Line 2047: Line 2048: if volInfo['status']['code'] != 0: -- To view, visit http://gerrit.ovirt.org/19157 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie68292eee4b82fbe8527e3960739979cfe117dfa Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Miguel Angel Ajo Pelayo Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: _normalizeVdsmImg refactoring
Federico Simoncelli has posted comments on this change. Change subject: vm: _normalizeVdsmImg refactoring .. Patch Set 4: (2 comments) http://gerrit.ovirt.org/#/c/19157/4//COMMIT_MSG Commit Message: Line 4: Commit: Federico Simoncelli Line 5: CommitDate: 2014-02-21 05:56:20 -0500 Line 6: Line 7: vm: _normalizeVdsmImg refactoring Line 8: Commit message to be improved once we decide to keep this Line 9: Change-Id: Ie68292eee4b82fbe8527e3960739979cfe117dfa http://gerrit.ovirt.org/#/c/19157/4/vdsm/vm.py File vdsm/vm.py: Line 2040: drive['device'] = drive.get('device', 'disk') # Disk by default Line 2041: drive['reqsize'] = drive.get('reqsize', '0') # Backward compatible Line 2042: Line 2043: if drive['device'] == 'disk': Line 2044: volInfo = self.cif.irs.getVolumeInfo( I'll split this change out. Line 2045: drive['domainID'], drive['poolID'], drive['imageID'], Line 2046: drive['volumeID']) Line 2047: Line 2048: if volInfo.get('status', {}).get('code', -1): -- To view, visit http://gerrit.ovirt.org/19157 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie68292eee4b82fbe8527e3960739979cfe117dfa Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Miguel Angel Ajo Pelayo Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: _normalizeVdsmImg refactoring
Nir Soffer has posted comments on this change. Change subject: vm: _normalizeVdsmImg refactoring .. Patch Set 4: (3 comments) The refactoring is nice (name changes, use dict.get with defualt instead of using if-else. I don't see why getVolumeInfo is better, and the commit mesage should explain it. I'm not sure about the way the code is tring to check the status code, this will make it harder to fix bugs in irs code. The behavior changes (using getVolumeInfo instead of getVolumeSize and way of checking status code) should be split into separate patch. http://gerrit.ovirt.org/#/c/19157/4/vdsm/vm.py File vdsm/vm.py: Line 2037: return str(idx) Line 2038: Line 2039: def _normalizeVdsmImg(self, drive): Line 2040: drive['device'] = drive.get('device', 'disk') # Disk by default Line 2041: drive['reqsize'] = drive.get('reqsize', '0') # Backward compatible Nice Line 2042: Line 2043: if drive['device'] == 'disk': Line 2044: volInfo = self.cif.irs.getVolumeInfo( Line 2045: drive['domainID'], drive['poolID'], drive['imageID'], Line 2040: drive['device'] = drive.get('device', 'disk') # Disk by default Line 2041: drive['reqsize'] = drive.get('reqsize', '0') # Backward compatible Line 2042: Line 2043: if drive['device'] == 'disk': Line 2044: volInfo = self.cif.irs.getVolumeInfo( > I'll split this change out. Can you explain in the commit message why getVolumeInfo is better then getVolumeSize? Line 2045: drive['domainID'], drive['poolID'], drive['imageID'], Line 2046: drive['volumeID']) Line 2047: Line 2048: if volInfo.get('status', {}).get('code', -1): Line 2044: volInfo = self.cif.irs.getVolumeInfo( Line 2045: drive['domainID'], drive['poolID'], drive['imageID'], Line 2046: drive['volumeID']) Line 2047: Line 2048: if volInfo.get('status', {}).get('code', -1): I'm not sure about this - since it will hide a bad response from irs. We expect to get a dict with status dict containing a code key. If we don't get this, I prefer to get a KeyError so we can fix irs. Line 2049: self.log.error( Line 2050: 'unable to get volume info for %s (response: %s)', Line 2051: drive['volumeID'], volInfo) Line 2052: raise StorageUnavailableError( -- To view, visit http://gerrit.ovirt.org/19157 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie68292eee4b82fbe8527e3960739979cfe117dfa Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: _normalizeVdsmImg refactoring
Itamar Heim has posted comments on this change. Change subject: vm: _normalizeVdsmImg refactoring .. Patch Set 4: ping -- To view, visit http://gerrit.ovirt.org/19157 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie68292eee4b82fbe8527e3960739979cfe117dfa Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: _normalizeVdsmImg refactoring
Itamar Heim has abandoned this change. Change subject: vm: _normalizeVdsmImg refactoring .. Abandoned no activity. please restore if relevant. -- To view, visit http://gerrit.ovirt.org/19157 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ie68292eee4b82fbe8527e3960739979cfe117dfa Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches