Maor Lipchuk has posted comments on this change. Change subject: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow. ......................................................................
Patch Set 4: (6 comments) @Nir, I disagree regarding the approach of no upload new versions while the tests are broken since this patch is a WIP and posted as a draft. Once the tests will be fixed I will publish this draft patch https://gerrit.ovirt.org/#/c/64169/4//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2016-09-20 01:41:01 +0300 Line 4: Commit: Maor Lipchuk <[email protected]> Line 5: CommitDate: 2016-09-20 03:56:54 +0300 Line 6: Line 7: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow. > We don't use such titles ([FOO]). I'm not sure why to use qcow3? qcow_compat 1.1 should be supported for qcow2 as well... I would simply use qemuimg without QCOW1.1 The Version attribute in the qcow header indicates the qcow version (qcow1, qcow2, qcow3), it should not be related to the compat version AFAIK Line 8: 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: https://gerrit.ovirt.org/#/c/64169/4/lib/vdsm/qemuimg.py File lib/vdsm/qemuimg.py: Line 45: RAW = "raw" Line 46: VMDK = "vmdk" Line 47: Line 48: _QCOW2_COMPAT_SUPPORTED = ("0.10", "1.1") Line 49: _QCOW2_1_1 = "1.1" > We need a better name for this, and we need another constant for 0.10, and Do you have any suggestions for names? How about _QCOW_COMPAT_LEVEL_0_10 and _QCOW_COMPAT_LEVEL_1_1 ? Line 50: Line 51: Line 52: def supports_compat(compat): Line 53: return compat in _QCOW2_COMPAT_SUPPORTED Line 102: return info Line 103: Line 104: Line 105: def create(image, size=None, format=None, backing=None, Line 106: backingFormat=None, **kwargs): > Please replace this change with qcow2_compat=None. Why not simply call it sd_version? qcow2_compat should be derived from the sd_version Line 107: cmd = [_qemuimg.cmd, "create"] Line 108: cwdPath = None Line 109: Line 110: if format: Line 154: raise QImgError(rc, out, err, "unable to parse qemu-img check output") Line 155: Line 156: Line 157: def convert(srcImage, dstImage, srcFormat=None, dstFormat=None, Line 158: backing=None, backingFormat=None, **kwargs): > Same, replace with qcow_compat=None see previous comment Line 159: cmd = [_qemuimg.cmd, "convert", "-p", "-t", "none", "-T", "none"] Line 160: options = [] Line 161: cwdPath = None Line 162: Line 307: if rc != 0: Line 308: raise QImgError(rc, out, err) Line 309: Line 310: Line 311: def _qcow2_compat(**kwargs): > Please rename to _default_qcow2_compat() done, I assume we should also change this to pass only the sd version? Line 312: sd_version = kwargs.get('version') Line 313: if sd_version == '4': Line 314: return _QCOW2_1_1 Line 315: value = config.get('irs', 'qcow2_compat') Line 309: Line 310: Line 311: def _qcow2_compat(**kwargs): Line 312: sd_version = kwargs.get('version') Line 313: if sd_version == '4': > >= 4 perhaps? indeed, done Line 314: return _QCOW2_1_1 Line 315: value = config.get('irs', 'qcow2_compat') Line 316: if value not in _QCOW2_COMPAT_SUPPORTED: Line 317: raise exception.InvalidConfiguration( -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Yaniv Kaul <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- [email protected] To unsubscribe send an email to [email protected]
