Nir Soffer has posted comments on this change. Change subject: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow. ......................................................................
Patch Set 4: Code-Review-1 (4 comments) qemuimg module cannot have storage logic, move the logic to sd.py. We should have a new qcow_compat parameter and update the tests. 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 use both in the list above. 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. Injecting here a domain version is a layering violation. This is a wrapper around qemu-img command, and is not part of the storage layer. This layer does not know anything about version. The caller of this api should choose the correct qcow_compat value, based on the storage domain version. I think the best place for this in StorageDomain.qcow_compat() - it will check the domain version and return correct value. 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 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() This will be used if the user did not specify the compat 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') -- 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 <mlipc...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Yaniv Kaul <yk...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
