Change in vdsm[master]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[nsoffer]

Nir Soffer has posted comments on this change.

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


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64169/4/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 310: 
Line 311: def _qcow2_compat(**kwargs):
Line 312: sd_version = kwargs.get('version')
Line 313: if sd_version == '4':
Line 314: return _QCOW2_1_1
> ok, I think I got it now, this method should not be changed (only its name)
Let make the storage domain changes in the next patch to keep things simple.
Line 315: value = config.get('irs', 'qcow2_compat')
Line 316: if value not in _QCOW2_COMPAT_SUPPORTED:
Line 317: raise exception.InvalidConfiguration(
Line 318: "Unsupported value for irs:qcow2_compat: %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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[mlipchuk]

Maor Lipchuk has posted comments on this change.

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


Patch Set 4:

(5 comments)

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 
Line 5: CommitDate: 2016-09-20 03:56:54 +0300
Line 6: 
Line 7: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.
> The version in qcow head is the compat, this is how we detect the image for
ok, I think I got confused with qcow2 version 3 vs version 2.

The version it self can only be 2 or 3, I assume that version 2 implies it 
supports compat level 0.10 and version 3 supports compat level 1.1
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"
> Way too long, using the strings "0.10" and "1.1" is the best way.
Done
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):
> Because this module does not or care about storage domains and version. It 
done
Line 107: cmd = [_qemuimg.cmd, "create"]
Line 108: cwdPath = None
Line 109: 
Line 110: if format:


PS4, Line 311: _qcow2_co
ok, I think I got it now, this method should not be changed (only its name) 
since this will be used only once the user did not specify the compat version.


Line 310: 
Line 311: def _qcow2_compat(**kwargs):
Line 312: sd_version = kwargs.get('version')
Line 313: if sd_version == '4':
Line 314: return _QCOW2_1_1
> The place for this is in StorageDomain class:
ok, I think I got it now, this method should not be changed (only its name) 
since this will be used only once the user did not specify the compat version.
Line 315: value = config.get('irs', 'qcow2_compat')
Line 316: if value not in _QCOW2_COMPAT_SUPPORTED:
Line 317: raise exception.InvalidConfiguration(
Line 318: "Unsupported value for irs:qcow2_compat: %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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[nsoffer]

Nir Soffer has posted comments on this change.

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


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64169/4/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 310: 
Line 311: def _qcow2_compat(**kwargs):
Line 312: sd_version = kwargs.get('version')
Line 313: if sd_version == '4':
Line 314: return _QCOW2_1_1
The place for this is in StorageDomain class:

def qcow2_compat(self):
if int(self.getVersion()) >= 4:
return "1.1"
return qcowimg.default_format()

Keeping this is storage domain will allow easy export to the correct format, 
using the destination storage domain qcow_format. This is another example why 
this module cannot decide on the format.

default_format() can be useful to set the default value in this module, if it 
is called not from storage layer.
Line 315: value = config.get('irs', 'qcow2_compat')
Line 316: if value not in _QCOW2_COMPAT_SUPPORTED:
Line 317: raise exception.InvalidConfiguration(
Line 318: "Unsupported value for irs:qcow2_compat: %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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[nsoffer]

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 
Gerrit-Reviewer: Adam Litke 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[nsoffer]

Nir Soffer has posted comments on this change.

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


Patch Set 4:

(1 comment)

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 
Line 5: CommitDate: 2016-09-20 03:56:54 +0300
Line 6: 
Line 7: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.
> I'm not sure why to use qcow3?
The version in qcow head is the compat, this is how we detect the image format 
during upload.
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: 


-- 
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 
Gerrit-Reviewer: Adam Litke 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[mlipchuk]

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 
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 
Gerrit-Reviewer: Adam Litke 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[nsoffer]

Nir Soffer has posted comments on this change.

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


Patch Set 4:

This change broke many tests - please make sure all tests pass before you 
upload the next version.

3:46:15 ==
23:46:15 ERROR: test_volume_chain_copy('block', 'raw', 'raw', (0, 1)) 
(storage_sdm_copy_data_test.TestCopyDataDIV)
23:46:15 --
23:46:15 Traceback (most recent call last):
23:46:15   File 
"/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/tests/testlib.py",
 line 85, in wrapper
23:46:15 return f(self, *args)
23:46:15   File 
"/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/tests/storage_sdm_copy_data_test.py",
 line 164, in test_volume_chain_copy
23:46:15 chain_length=nr_vols) as (src_chain,
23:46:15   File "/usr/lib64/python2.7/contextlib.py", line 17, in __enter__
23:46:15 return self.gen.next()
23:46:15   File 
"/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/tests/storage_sdm_copy_data_test.py",
 line 74, in get_vols
23:46:15 chain_length)
23:46:15   File 
"/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/tests/storagetestlib.py",
 line 356, in make_qemu_chain
23:46:15 format=qemuimg.FORMAT.QCOW2, backing=backing)
23:46:15   File 
"/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/lib/vdsm/qemuimg.py",
 line 113, in create
23:46:15 cmd.extend(('-o', 'compat=' + _qcow2_compat(kwargs)))
23:46:15 TypeError: _qcow2_compat() takes exactly 0 arguments (1 given)
23:46:15  >> begin captured logging << 
23:46:15 2016-09-19 23:45:16,559 DEBUG   [storage.PersistentDict] (MainThread) 
Created a persistent dict with VGTagMetadataRW backend
23:46:15 2016-09-19 23:45:16,560 DEBUG   [storage.PersistentDict] (MainThread) 
read lines (VGTagMetadataRW)=[]
23:46:15 2016-09-19 23:45:16,561 DEBUG   [storage.PersistentDict] (MainThread) 
Empty metadata
23:46:15 2016-09-19 23:45:16,561 DEBUG   [storage.PersistentDict] (MainThread) 
Starting transaction
23:46:15 2016-09-19 23:45:16,561 DEBUG   [storage.PersistentDict] (MainThread) 
Flushing changes
23:46:15 2016-09-19 23:45:16,562 DEBUG   [storage.PersistentDict] (MainThread) 
about to write lines (VGTagMetadataRW)=['CLASS=Data', 
'POOL_UUID=a65f91b4-d51c-49c7-92ec-d3767bd94e14', 
'SDUUID=3c02be87-d1da-4fa1-8fca-adfb7f4f63e5', 'VERSION=3', 
'_SHA_CKSUM=0c7bd0762a865207d4d00f5ae16b69a607125565']
23:46:15 2016-09-19 23:45:16,563 DEBUG   [storage.Metadata.VGTagMetadataRW] 
(MainThread) Updating metadata 
adding=MDT_POOL_UUID=a65f91b4-d51c-49c7-92ec-d3767bd94e14, MDT_VERSION=3, 
MDT_SDUUID=3c02be87-d1da-4fa1-8fca-adfb7f4f63e5, 
MDT__SHA_CKSUM=0c7bd0762a865207d4d00f5ae16b69a607125565, MDT_CLASS=Data 
removing=
23:46:15 2016-09-19 23:45:16,563 DEBUG   [storage.PersistentDict] (MainThread) 
Finished transaction
23:46:15 2016-09-19 23:45:16,568 WARNING [storage.StorageDomainManifest] 
(MainThread) Could not find mapping for lv 
3c02be87-d1da-4fa1-8fca-adfb7f4f63e5/b8f76961-14df-4679-86c6-c21fe522cb8b
23:46:15 2016-09-19 23:45:16,569 DEBUG   [storage.StorageDomainManifest] 
(MainThread) Found freeSlot 4 in VG 3c02be87-d1da-4fa1-8fca-adfb7f4f63e5
23:46:15 2016-09-19 23:45:16,571 DEBUG   [storage.VolumeManifest] (MainThread) 
Creating symlink from 
/var/tmp/tmpBX_Rfb/dev/3c02be87-d1da-4fa1-8fca-adfb7f4f63e5/b8f76961-14df-4679-86c6-c21fe522cb8b
 to 
/var/tmp/tmpBX_Rfb/mnt/blockSD/3c02be87-d1da-4fa1-8fca-adfb7f4f63e5/images/d6ae6eb6-fb86-40e5-929b-8e554df0d1a2/b8f76961-14df-4679-86c6-c21fe522cb8b
23:46:15 2016-09-19 23:45:16,575 WARNING [storage.StorageDomainManifest] 
(MainThread) Could not find mapping for lv 
3c02be87-d1da-4fa1-8fca-adfb7f4f63e5/8c4ecba5-0767-41a2-abb5-87afcc9b0949
23:46:15 2016-09-19 23:45:16,576 DEBUG   [storage.StorageDomainManifest] 
(MainThread) Found freeSlot 5 in VG 3c02be87-d1da-4fa1-8fca-adfb7f4f63e5
23:46:15 2016-09-19 23:45:16,583 DEBUG   [storage.VolumeManifest] (MainThread) 
Creating symlink from 
/var/tmp/tmpBX_Rfb/dev/3c02be87-d1da-4fa1-8fca-adfb7f4f63e5/8c4ecba5-0767-41a2-abb5-87afcc9b0949
 to 
/var/tmp/tmpBX_Rfb/mnt/blockSD/3c02be87-d1da-4fa1-8fca-adfb7f4f63e5/images/d6ae6eb6-fb86-40e5-929b-8e554df0d1a2/8c4ecba5-0767-41a2-abb5-87afcc9b0949
23:46:15 - >> end captured logging << -

-- 
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 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: 

Change in vdsm[master]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[nsoffer]

Nir Soffer has posted comments on this change.

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


Patch Set 4:

(1 comment)

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 
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]).

You can name this after the module or subsystem modified, or the feature, which 
is actually "qcow3".

- qcow2 == qcow_compat 0.10
- qcow3 == qcow_compat 1.1

I don't know why qemu are not calling this "version", this is how it is stored 
in qcow header.
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: 


-- 
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 
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 -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[nsoffer]

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 
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 -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[ykaul]

Yaniv Kaul has posted comments on this change.

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


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64169/4/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

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?
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 
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 -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[automation]

gerrit-hooks has posted comments on this change.

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


Patch Set 4:

* 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: 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: 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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[automation]

gerrit-hooks has posted comments on this change.

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


Patch Set 3:

* 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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[Jenkins CI]

Jenkins CI has posted comments on this change.

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


Patch Set 2: Continuous-Integration-1

Propagate review hook: Continuous Integration value inherited from patch 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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[automation]

gerrit-hooks has posted comments on this change.

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


Patch Set 2:

* 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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[automation]

gerrit-hooks has posted comments on this change.

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


Patch Set 1:

* 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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

[mlipchuk]

Maor Lipchuk has uploaded a new change for review.

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

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

Add kwargs of storage domain metadata to indicate the
appropriate qcow level to use for create and convert operations.

Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Signed-off-by: Maor Lipchuk 
---
M lib/vdsm/qemuimg.py
1 file changed, 9 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/64169/1

diff --git a/lib/vdsm/qemuimg.py b/lib/vdsm/qemuimg.py
index 53a02c7..213890b 100644
--- a/lib/vdsm/qemuimg.py
+++ b/lib/vdsm/qemuimg.py
@@ -46,6 +46,7 @@
 VMDK = "vmdk"
 
 _QCOW2_COMPAT_SUPPORTED = ("0.10", "1.1")
+_QCOW2_1_1 = "1.1"
 
 
 def supports_compat(compat):
@@ -101,14 +102,14 @@
 return info
 
 
-def create(image, size=None, format=None, backing=None, backingFormat=None):
+def create(image, size=None, format=None, backing=None, backingFormat=None, 
kwargs=None):
 cmd = [_qemuimg.cmd, "create"]
 cwdPath = None
 
 if format:
 cmd.extend(("-f", format))
 if format == FORMAT.QCOW2:
-cmd.extend(('-o', 'compat=' + _qcow2_compat()))
+cmd.extend(('-o', 'compat=' + _qcow2_compat(kwargs)))
 
 if backing:
 if not os.path.isabs(backing):
@@ -153,7 +154,7 @@
 
 
 def convert(srcImage, dstImage, srcFormat=None, dstFormat=None,
-backing=None, backingFormat=None):
+backing=None, backingFormat=None, kwargs=None):
 cmd = [_qemuimg.cmd, "convert", "-p", "-t", "none", "-T", "none"]
 options = []
 cwdPath = None
@@ -166,7 +167,7 @@
 if dstFormat:
 cmd.extend(("-O", dstFormat))
 if dstFormat == FORMAT.QCOW2:
-options.append('compat=' + _qcow2_compat())
+options.append('compat=' + _qcow2_compat(kwargs))
 
 if backing:
 if not os.path.isabs(backing):
@@ -306,7 +307,10 @@
 raise QImgError(rc, out, err)
 
 
-def _qcow2_compat():
+def _qcow2_compat(kwargs):
+sd_version = kwargs.get('version')
+if sd_version == '4':
+return _QCOW2_1_1
 value = config.get('irs', 'qcow2_compat')
 if value not in _QCOW2_COMPAT_SUPPORTED:
 raise exception.InvalidConfiguration(


-- 
To view, visit https://gerrit.ovirt.org/64169
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org