Change in vdsm[master]: qemuimg: Add qcow_compat optional parameter.
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 LipchukReviewed-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukLine 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.
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 LipchukGerrit-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.
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 LipchukGerrit-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.
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 LipchukGerrit-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