Nir Soffer has posted comments on this change.

Change subject: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.
......................................................................


Patch Set 4:

(3 comments)

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"
> Do you have any suggestions for names?
Way too long, using the strings "0.10" and "1.1" is the best way.
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):
> Why not simply call it sd_version? qcow2_compat should be  derived from the
Because this module does not or care about storage domains and version. It 
should no logic related to storage.
Line 107:     cmd = [_qemuimg.cmd, "create"]
Line 108:     cwdPath = None
Line 109: 
Line 110:     if format:


Line 307:     if rc != 0:
Line 308:         raise QImgError(rc, out, err)
Line 309: 
Line 310: 
Line 311: def _qcow2_compat(**kwargs):
> done, I assume we should also change this to pass only the sd version?
No, no sd or version in this module.
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: Maor Lipchuk <mlipc...@redhat.com>
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

Reply via email to