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


qemuimg module cannot have storage logic, move the logic to

We should have a new qcow_compat parameter and update the tests.
File lib/vdsm/

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 
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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <>
Gerrit-Reviewer: Adam Litke <>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <>
Gerrit-Reviewer: Yaniv Kaul <>
Gerrit-Reviewer: gerrit-hooks <>
Gerrit-HasComments: Yes
vdsm-patches mailing list --
To unsubscribe send an email to

Reply via email to