Change in vdsm[master]: vm: _normalizeVdsmImg refactoring

2014-02-21 Thread Federico Simoncelli 
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

2014-02-21 Thread Federico Simoncelli 
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

2014-02-21 Thread Federico Simoncelli 
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

2014-02-21 Thread nsoffer 
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

2014-05-19 Thread iheim 
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

2014-07-14 Thread Itamar Heim 
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