Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread nsoffer
Nir Soffer has submitted this change and it was merged.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


qemuimg: Add qcow_compat optional parameter.

Add qcow2_compat parameter that indicates the apropriate qemu
compatibility level to be used for create and convert operations.

Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Signed-off-by: Maor Lipchuk 
Reviewed-on: https://gerrit.ovirt.org/64169
Reviewed-by: Nir Soffer 
Continuous-Integration: Jenkins CI
---
M lib/vdsm/qemuimg.py
M tests/qemuimg_test.py
2 files changed, 49 insertions(+), 5 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, approved
  Jenkins CI: Passed CI tests
  Maor Lipchuk: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 28
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 28:

* update_tracker: OK
* Set MODIFIED::IGNORE, no Bug-Url found.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 28
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread mlipchuk
Maor Lipchuk has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 27: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 27:

* update_tracker: OK
* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 27: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 26: Verified+1

Tests pass, looks verified to me.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 26
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 26:

* update_tracker: OK
* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 26
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 25:

(1 comment)

https://gerrit.ovirt.org/#/c/64169/25/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 326: 
Line 327: def _validate_qcow2_compat(value):
Line 328: if value is None:
Line 329: value = default_qcow2_compat()
Line 330: return value
Better:

if value is None:
return default_qcow2_compat()
Line 331: if value not in _QCOW2_COMPAT_SUPPORTED:
Line 332: raise ValueError("Invalid compat version '%r'" % value)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 25
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 25:

* update_tracker: OK
* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 25
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread mlipchuk
Maor Lipchuk has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 25: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 25
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 24:

* update_tracker: OK
* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 23:

(1 comment)

https://gerrit.ovirt.org/#/c/64169/23/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 323: raise ValueError("not a JSON object")
Line 324: return obj
Line 325: 
Line 326: 
Line 327: def _validate_qcow2_compat(qcow2_value):
Can you rename qcow2_value to value?

This a helper to validate specific value, the name make it very clear. In the 
context of this 6 line helper, there is only one value, we don't have to give 
it a specific name.
Line 328: if qcow2_value is None:
Line 329: qcow2_value = default_qcow2_compat()
Line 330: return qcow2_value
Line 331: if qcow2_value not in _QCOW2_COMPAT_SUPPORTED:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread mlipchuk
Maor Lipchuk has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 23: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 23:

* update_tracker: OK
* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 22:

* update_tracker: OK
* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-27 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 21:

* update_tracker: OK
* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-26 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 20:

* update_tracker: OK
* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-26 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 19:

* update_tracker: OK
* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-26 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 18:

(2 comments)

https://gerrit.ovirt.org/#/c/64169/18/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 101: return info
Line 102: 
Line 103: 
Line 104: def create(image, size=None, format=None, backing=None,
Line 105:backingFormat=None, qcow2_compat=None):
This option is very clear here - format and qcow2_compat refer to the image 
being created. I also like the qcow2_compat style, but the old code is already 
using mixedCase, and we should match this style in the arguments.

Also the new argument refer to the format argument, not to the backing, so the 
argument should come after format=None.

Adding a parameter in the middle may break code calling this function 
incorrectly with positional arguments - this is good, we should fix such code.

Another option is to switch this all arguments to lower_case style before 
adding this option - this will be a very nice cleanup if you can spend the time 
on this rename. If not, please match the arguments to the existing ugly and 
error prone style.
Line 106: cmd = [_qemuimg.cmd, "create"]
Line 107: cwdPath = None
Line 108: 
Line 109: if format:


Line 168: if dstFormat:
Line 169: cmd.extend(("-O", dstFormat))
Line 170: if dstFormat == FORMAT.QCOW2:
Line 171: qcow2_compat = validate_qcow2_compat(qcow2_compat)
Line 172: options.append('compat=' + qcow2_compat)
But here he compat refer to the destination format (dstFormat), so it should be 
named dst_qcow2_compat, and the parameter should appear after dstFormat, and 
not after backingFormat.
Line 173: 
Line 174: if backing:
Line 175: if not os.path.isabs(backing):
Line 176: cwdPath = os.path.dirname(srcImage)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-26 Thread mlipchuk
Maor Lipchuk has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 18:

(3 comments)

https://gerrit.ovirt.org/#/c/64169/18/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 187: 
Line 188: return QemuImgOperation(cmd, cwd=cwdPath)
Line 189: 
Line 190: 
Line 191: def validate_qcow2_compat(qcow2_compat):
> I think this should be private helper (_validate_qcow2_compat), and it shou
done
Line 192: if qcow2_compat is None:
Line 193: qcow2_compat = default_qcow2_compat()
Line 194: if qcow2_compat in _QCOW2_COMPAT_SUPPORTED:
Line 195: return qcow2_compat


Line 189: 
Line 190: 
Line 191: def validate_qcow2_compat(qcow2_compat):
Line 192: if qcow2_compat is None:
Line 193: qcow2_compat = default_qcow2_compat()
> We can and should return here, we don't need to validate the return value o
Done
Line 194: if qcow2_compat in _QCOW2_COMPAT_SUPPORTED:
Line 195: return qcow2_compat
Line 196: else:
Line 197: raise ValueError("Invalid compat version '%r'" % qcow2_compat)


Line 193: qcow2_compat = default_qcow2_compat()
Line 194: if qcow2_compat in _QCOW2_COMPAT_SUPPORTED:
Line 195: return qcow2_compat
Line 196: else:
Line 197: raise ValueError("Invalid compat version '%r'" % qcow2_compat)
> This code works but it does not follow the vdsm style, using the happy path
Done
Line 198: 
Line 199: 
Line 200: class QemuImgOperation(object):
Line 201: REGEXPR = re.compile(r'\s*\(([\d.]+)/100%\)\s*')


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-26 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 18:

(3 comments)

https://gerrit.ovirt.org/#/c/64169/18/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 187: 
Line 188: return QemuImgOperation(cmd, cwd=cwdPath)
Line 189: 
Line 190: 
Line 191: def validate_qcow2_compat(qcow2_compat):
I think this should be private helper (_validate_qcow2_compat), and it should 
be located at the bottom of the file, near _parse_qemu_json.

"qcow2_compat" is too wordy here, why not use "value"?
Line 192: if qcow2_compat is None:
Line 193: qcow2_compat = default_qcow2_compat()
Line 194: if qcow2_compat in _QCOW2_COMPAT_SUPPORTED:
Line 195: return qcow2_compat


Line 189: 
Line 190: 
Line 191: def validate_qcow2_compat(qcow2_compat):
Line 192: if qcow2_compat is None:
Line 193: qcow2_compat = default_qcow2_compat()
We can and should return here, we don't need to validate the return value of 
default_qcow2_compat.
Line 194: if qcow2_compat in _QCOW2_COMPAT_SUPPORTED:
Line 195: return qcow2_compat
Line 196: else:
Line 197: raise ValueError("Invalid compat version '%r'" % qcow2_compat)


Line 193: qcow2_compat = default_qcow2_compat()
Line 194: if qcow2_compat in _QCOW2_COMPAT_SUPPORTED:
Line 195: return qcow2_compat
Line 196: else:
Line 197: raise ValueError("Invalid compat version '%r'" % qcow2_compat)
This code works but it does not follow the vdsm style, using the happy path 
idiom - first check for errors and raise, then handle the normal flow.

if qcow2_compat not in _QCOW2_COMPAT_SUPPORTED:
raise ...
return qcow2_compat
Line 198: 
Line 199: 
Line 200: class QemuImgOperation(object):
Line 201: REGEXPR = re.compile(r'\s*\(([\d.]+)/100%\)\s*')


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-26 Thread mlipchuk
Maor Lipchuk has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 18: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-26 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 18:

* update_tracker: OK
* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-26 Thread mlipchuk
Maor Lipchuk has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 17:

Found the issue, it is not related to a whitespace but a failing space, next 
upload should be with fixed tests

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-26 Thread mlipchuk
Maor Lipchuk has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 16:

I already fixed the trailing whitespaces issue in patchset 17.
The make check for this patch ends with success.

but the CI fails on something unexplained:
At the el7 it fails for the following reason:
Exception AttributeError: "'NoneType' object has no attribute 'udev_unref'" in 
> 
ignored

And the fedora CI fails on the following:
/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/vdsm/virt/vmtune.py
  97  793%
14:18:43 
/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/vdsm/virt/vmxml.py
  244 1793%
14:18:43 
--
14:18:43 TOTAL  
42807  2043652%
14:18:43 --
14:18:43 Ran 2686 tests in 203.690s
14:18:43 
14:18:43 FAILED (SKIP=141, errors=6, failures=8)
14:18:43 Makefile:1291: recipe for target 'check' failed
14:18:43 make[1]: *** [check] Error 1
14:18:43 make[1]: Leaving directory 
'/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/tests'
14:18:43 Makefile:1011: recipe for target 'tests' failed
14:18:43 make: *** [tests] Error 2
14:18:43 Took 678 seconds
14:18:43 ===
14:18:43 ##!
14:18:43 ##! ERROR ^^^


None of them seem related to this change...

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-25 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 17:

Please fix pep8 errors, without fixing them the CI won't even try the real 
tests.

./lib/vdsm/qemuimg.py:113:57: W291 trailing whitespace

Did you try to run "make check" locally before uploading the patch?

You can also run "make pep8" and "make pyflakes" to run a quick specific
check.

Or even faster, pep8 lib/vdsm/qemuimg.py and pyflakes lib/vdsm/qemuimg.py to 
get immediate feedback.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-25 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 17:

* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-25 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 16:

* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-25 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 15:

* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-25 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 14:

* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-25 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 13:

* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-25 Thread mlipchuk
Maor Lipchuk has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 12:

(4 comments)

https://gerrit.ovirt.org/#/c/64169/12/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 110: cmd.extend(("-f", format))
Line 111: if format == FORMAT.QCOW2:
Line 112: if qcow2_compat is None:
Line 113: qcow2_compat = default_qcow2_compat()
Line 114: if qcow2_compat in ("0.10", "1.1"):
> We have a constant for this.
Done
Line 115: cmd.extend(('-o', 'compat=' + qcow2_compat))
Line 116: else:
Line 117: raise ValueError("Invalid compat version '%s'" % 
qcow2_compat)
Line 118: 


Line 113: qcow2_compat = default_qcow2_compat()
Line 114: if qcow2_compat in ("0.10", "1.1"):
Line 115: cmd.extend(('-o', 'compat=' + qcow2_compat))
Line 116: else:
Line 117: raise ValueError("Invalid compat version '%s'" % 
qcow2_compat)
> Use %r instead of '%s'.
Done
Line 118: 
Line 119: if backing:
Line 120: if not os.path.isabs(backing):
Line 121: cwdPath = os.path.dirname(image)


Line 176: qcow2_compat = default_qcow2_compat()
Line 177: if qcow2_compat in ("0.10", "1.1"):
Line 178: cmd.extend(('-o', 'compat=' + qcow2_compat))
Line 179: else:
Line 180: raise ValueError("Invalid compat version '%s'" % 
qcow2_compat)
> We do this logic twice - maybe extract a little helper for this, so we can 
Done
Line 181: 
Line 182: if backing:
Line 183: if not os.path.isabs(backing):
Line 184: cwdPath = os.path.dirname(srcImage)


https://gerrit.ovirt.org/#/c/64169/12/tests/qemuimg_test.py
File tests/qemuimg_test.py:

Line 170: qemuimg.create('image', format='qcow2')
Line 171: 
Line 172: def test_qcow2_compat_invalid(self):
Line 173: with self.assertRaises(ValueError):
Line 174: qemuimg.create('image', format='qcow2', 
qcow2_compat='1.11')
> Nice!
"None" is already being tested (for example test_qcow2_compat tests it since it 
does not send any value)

Regarding the other tests - done
Line 175: 
Line 176: def test_invalid_config(self):
Line 177: config = make_config([('irs', 'qcow2_compat', '1.2')])
Line 178: with MonkeyPatchScope([(qemuimg, 'config', config)]):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-22 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 12:

(4 comments)

https://gerrit.ovirt.org/#/c/64169/12/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 110: cmd.extend(("-f", format))
Line 111: if format == FORMAT.QCOW2:
Line 112: if qcow2_compat is None:
Line 113: qcow2_compat = default_qcow2_compat()
Line 114: if qcow2_compat in ("0.10", "1.1"):
We have a constant for this.
Line 115: cmd.extend(('-o', 'compat=' + qcow2_compat))
Line 116: else:
Line 117: raise ValueError("Invalid compat version '%s'" % 
qcow2_compat)
Line 118: 


Line 113: qcow2_compat = default_qcow2_compat()
Line 114: if qcow2_compat in ("0.10", "1.1"):
Line 115: cmd.extend(('-o', 'compat=' + qcow2_compat))
Line 116: else:
Line 117: raise ValueError("Invalid compat version '%s'" % 
qcow2_compat)
Use %r instead of '%s'.

Please always use the happy path idiom:

if qcow2_compat not in _QCOW2_COMPAT_SUPPORTED:
raise ...
normal flow
Line 118: 
Line 119: if backing:
Line 120: if not os.path.isabs(backing):
Line 121: cwdPath = os.path.dirname(image)


Line 176: qcow2_compat = default_qcow2_compat()
Line 177: if qcow2_compat in ("0.10", "1.1"):
Line 178: cmd.extend(('-o', 'compat=' + qcow2_compat))
Line 179: else:
Line 180: raise ValueError("Invalid compat version '%s'" % 
qcow2_compat)
We do this logic twice - maybe extract a little helper for this, so we can 
write:

qcow2_compat = _validate_qcow2_compat(qcow2_compat)

None will return the default format, and invalid value will raise.
Line 181: 
Line 182: if backing:
Line 183: if not os.path.isabs(backing):
Line 184: cwdPath = os.path.dirname(srcImage)


https://gerrit.ovirt.org/#/c/64169/12/tests/qemuimg_test.py
File tests/qemuimg_test.py:

Line 170: qemuimg.create('image', format='qcow2')
Line 171: 
Line 172: def test_qcow2_compat_invalid(self):
Line 173: with self.assertRaises(ValueError):
Line 174: qemuimg.create('image', format='qcow2', 
qcow2_compat='1.11')
Nice!

But we should test also convert.

Then we should test also both valid value and None, in both create and convert.
Line 175: 
Line 176: def test_invalid_config(self):
Line 177: config = make_config([('irs', 'qcow2_compat', '1.2')])
Line 178: with MonkeyPatchScope([(qemuimg, 'config', config)]):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-22 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 12:

* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.

2016-09-22 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter.
..


Patch Set 11:

* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter for.

2016-09-22 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter for.
..


Patch Set 10:

* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter for.

2016-09-22 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter for.
..


Patch Set 9:

(2 comments)

https://gerrit.ovirt.org/#/c/64169/9//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2016-09-20 01:41:01 +0300
Line 4: Commit: Maor Lipchuk 
Line 5: CommitDate: 2016-09-22 15:03:52 +0300
Line 6: 
Line 7: qemuimg: Add qcow_compat optional parameter for.
qcow_compat -> qcow2_compat

for. ?
Line 8: 
Line 9: Add qcow_compat parameter that indicates the apropriate qemu
Line 10: compatibility level to be used for create and convert operations.
Line 11: 


Line 5: CommitDate: 2016-09-22 15:03:52 +0300
Line 6: 
Line 7: qemuimg: Add qcow_compat optional parameter for.
Line 8: 
Line 9: Add qcow_compat parameter that indicates the apropriate qemu
qemu compatibility level -> qcow2 compat value
Line 10: compatibility level to be used for create and convert operations.
Line 11: 
Line 12: Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter for.

2016-09-22 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter for.
..


Patch Set 9:

* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter for.

2016-09-22 Thread mlipchuk
Maor Lipchuk has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter for.
..


Patch Set 7:

(1 comment)

https://gerrit.ovirt.org/#/c/64169/7//COMMIT_MSG
Commit Message:

Line 9: Add kwargs of storage domain metadata to indicate the
Line 10: appropriate qcow level to use for create and convert operations.
Line 11: 
Line 12: WIP: This is only the first patch of the CQOW upgrade series of patches
Line 13: This specific patch introduces changes at qemuimg.
> Please update the text to reflect the solution we discussed.
Done
Line 14: 
Line 15: Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org


Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter for.

2016-09-22 Thread automation
gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Add qcow_compat optional parameter for.
..


Patch Set 8:

* 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-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org