Change in vdsm[master]: storage: Refactor workarounds.detect_format

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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]: storage: Refactor workarounds.detect_format

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: storage: Refactor workarounds.detect_format
..


storage: Refactor workarounds.detect_format

The function workarounds.detect_format accepts two volumes, checks for a
special case, then returns the qemuimg formats that should be used for
each volume.  To make this logic more sharable, change detect_format to
just detect if the problem exists and let the calling code decide which
format to use.  This will allow the sdm.copy_data code to use this
function cleanly.

Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Signed-off-by: Adam Litke 
Reviewed-on: https://gerrit.ovirt.org/64230
Reviewed-by: Nir Soffer 
Continuous-Integration: Nir Soffer 
---
M lib/vdsm/storage/workarounds.py
M tests/storage_workarounds_test.py
M vdsm/storage/image.py
3 files changed, 18 insertions(+), 23 deletions(-)

Approvals:
  Adam Litke: Verified
  Nir Soffer: Looks good to me, approved; Passed CI tests

Objections:
  Jenkins CI: Failed CI tests



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
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]: storage: Refactor workarounds.detect_format

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 4: Verified+1

Verified by the following steps:
- Create 1G thin disk on block sd
- Move disk to file sd
- Manually edit vdsm volume metadata to set SIZE=20
- Overwrite the volume file with "VM configuration"
- Move the disk back to the block sd

The operation succeeded and the following appeared in the log:
77fcb026-4c51-4984-adfd-fc54a9641c87::WARNING::2016-09-23 
17:39:57,139::workarounds::59::storage.workarounds::(invalid_vm_conf_disk) 
Incorrect volume format 'qcow2' has been detected for volume 
u'33bb30de-0f7e-4f49-864f-f1add3298790', using the actual format 'raw'.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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]: storage: Refactor workarounds.detect_format

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 4: Continuous-Integration+1

Networks tests continue to fail

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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]: storage: Refactor workarounds.detect_format

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 4:

I rebase this on master, the tests should pass now.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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]: storage: Refactor workarounds.detect_format

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


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/64230
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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]: storage: Refactor workarounds.detect_format

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 3: Code-Review+2

This needs verification running the old code in image.py.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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]: storage: Refactor workarounds.detect_format

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


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/64230
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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]: storage: Refactor workarounds.detect_format

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 2: Code-Review+2

Seems that this is the best way.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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]: storage: Refactor workarounds.detect_format

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


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/64230
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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]: storage: Refactor workarounds.detect_format

[alitke]

Adam Litke has restored this change.

Change subject: storage: Refactor workarounds.detect_format
..


Restored

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

Gerrit-MessageType: restore
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
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]: storage: Refactor workarounds.detect_format

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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]: storage: Refactor workarounds.detect_format

[alitke]

Adam Litke has abandoned this change.

Change subject: storage: Refactor workarounds.detect_format
..


Abandoned

Not needed.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
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]: storage: Refactor workarounds.detect_format

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 1:

(7 comments)

https://gerrit.ovirt.org/#/c/64230/1/tests/storage_workarounds_test.py
File tests/storage_workarounds_test.py:

Line 28
Line 29
Line 30
Line 31
Line 32
> Lets add a constants in the module for this, repeating this line in all the
Done


Line 31: 
Line 32: 
Line 33: class DetectFormatTest(VdsmTestCase):
Line 34: 
Line 35: def make_volume(self, env, size, src_md_fmt, src_qemu_fmt):
> There is no src anymore, so we can use now:
Done
Line 36: img_id = str(uuid.uuid4())
Line 37: vol_id = str(uuid.uuid4())
Line 38: env.make_volume(size, img_id, vol_id, vol_format=src_md_fmt)
Line 39: vol = env.sd_manifest.produceVolume(img_id, vol_id)


Line 48: """
Line 49: size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
Line 50: with fake_file_env() as env:
Line 51: vol = self.make_volume(env, size, sc.COW_FORMAT,
Line 52:qemuimg.FORMAT.RAW)
> To make it more clear:
Done
Line 53: self.assertTrue(workarounds.mismatched_vm_conf_disk(vol))
Line 54: 
Line 55: def test_bad_format_other_size(self):
Line 56: """


https://gerrit.ovirt.org/#/c/64230/1/vdsm/storage/image.py
File vdsm/storage/image.py:

Line 455: try:
Line 456: dstVol = destDom.produceVolume(imgUUID=imgUUID,
Line 457:
volUUID=srcVol.volUUID)
Line 458: 
Line 459: if workarounds.mismatched_vm_conf_disk(srcVol):
> Only this line is tested now.
I've gone back to returning the modified format like the original code did.
Line 460: srcFormat = dstFormat = qemuimg.FORMAT.RAW
Line 461: else:
Line 462: srcFormat = sc.fmt2str(srcVol.getFormat())
Line 463: dstFormat = sc.fmt2str(dstVol.getFormat())


Line 459: if workarounds.mismatched_vm_conf_disk(srcVol):
Line 460: srcFormat = dstFormat = qemuimg.FORMAT.RAW
Line 461: else:
Line 462: srcFormat = sc.fmt2str(srcVol.getFormat())
Line 463: dstFormat = sc.fmt2str(dstVol.getFormat())
> One issue with this approach, this code is not tested now, but there is no 
see above.
Line 464: 
Line 465: parentVol = dstVol.getParentVolume()
Line 466: 
Line 467: if parentVol is not None:


https://gerrit.ovirt.org/#/c/64230/1/vdsm/storage/workarounds.py
File vdsm/storage/workarounds.py:

Line 30
Line 31
Line 32
Line 33
Line 34
> This is not correct now, this patch code only detect invalid vm conf disk.
Not relevant anymore.


Line 28: # Size in blocks of the conf file generated during RAM snapshot 
operation.
Line 29: VM_CONF_SIZE_BLK = 20
Line 30: 
Line 31: 
Line 32: def mismatched_vm_conf_disk(vol):
> is_invalid_vm_conf_disk?
Back to original behavior so keeping original name.
Line 33: """
Line 34: set VM metadata images format to RAW
Line 35: 
Line 36: Since commit 0b61c4851a528fd6354d9ab77a68085c41f35dc9 copy of 
internal raw


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
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]: storage: Refactor workarounds.detect_format

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 1:

(4 comments)

https://gerrit.ovirt.org/#/c/64230/1/vdsm/storage/image.py
File vdsm/storage/image.py:

Line 455: try:
Line 456: dstVol = destDom.produceVolume(imgUUID=imgUUID,
Line 457:
volUUID=srcVol.volUUID)
Line 458: 
Line 459: if workarounds.mismatched_vm_conf_disk(srcVol):
Only this line is tested now.
Line 460: srcFormat = dstFormat = qemuimg.FORMAT.RAW
Line 461: else:
Line 462: srcFormat = sc.fmt2str(srcVol.getFormat())
Line 463: dstFormat = sc.fmt2str(dstVol.getFormat())


Line 459: if workarounds.mismatched_vm_conf_disk(srcVol):
Line 460: srcFormat = dstFormat = qemuimg.FORMAT.RAW
Line 461: else:
Line 462: srcFormat = sc.fmt2str(srcVol.getFormat())
Line 463: dstFormat = sc.fmt2str(dstVol.getFormat())
One issue with this approach, this code is not tested now, but there is no 
interesting logic here.
Line 464: 
Line 465: parentVol = dstVol.getParentVolume()
Line 466: 
Line 467: if parentVol is not None:


https://gerrit.ovirt.org/#/c/64230/1/vdsm/storage/workarounds.py
File vdsm/storage/workarounds.py:

Line 30
Line 31
Line 32
Line 33
Line 34
This is not correct now, this patch code only detect invalid vm conf disk.

This change splits the detection of the problem and the fix, and we have 2 
places that need this, so we have to repeat the code, and we don't have a place 
for the comment.


Line 28: # Size in blocks of the conf file generated during RAM snapshot 
operation.
Line 29: VM_CONF_SIZE_BLK = 20
Line 30: 
Line 31: 
Line 32: def mismatched_vm_conf_disk(vol):
is_invalid_vm_conf_disk?
Line 33: """
Line 34: set VM metadata images format to RAW
Line 35: 
Line 36: Since commit 0b61c4851a528fd6354d9ab77a68085c41f35dc9 copy of 
internal raw


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
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]: storage: Refactor workarounds.detect_format

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 1: Code-Review-1

(3 comments)

https://gerrit.ovirt.org/#/c/64230/1/tests/storage_workarounds_test.py
File tests/storage_workarounds_test.py:

Line 28
Line 29
Line 30
Line 31
Line 32
Lets add a constants in the module for this, repeating this line in all the 
tests is painful.

VM_CONF_SIZE = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE


Line 31: 
Line 32: 
Line 33: class DetectFormatTest(VdsmTestCase):
Line 34: 
Line 35: def make_volume(self, env, size, src_md_fmt, src_qemu_fmt):
There is no src anymore, so we can use now:

make_volume(env, size, md_fmt, real_fmt)
Line 36: img_id = str(uuid.uuid4())
Line 37: vol_id = str(uuid.uuid4())
Line 38: env.make_volume(size, img_id, vol_id, vol_format=src_md_fmt)
Line 39: vol = env.sd_manifest.produceVolume(img_id, vol_id)


Line 48: """
Line 49: size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
Line 50: with fake_file_env() as env:
Line 51: vol = self.make_volume(env, size, sc.COW_FORMAT,
Line 52:qemuimg.FORMAT.RAW)
To make it more clear:

   vol = make_volume(env, size, md_fmt="qcow2", real_fmt="raw")
Line 53: self.assertTrue(workarounds.mismatched_vm_conf_disk(vol))
Line 54: 
Line 55: def test_bad_format_other_size(self):
Line 56: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
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]: storage: Refactor workarounds.detect_format

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


Patch Set 1:

Nice, look much better then the original code, and the complicated tests are 
simple now.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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]: storage: Refactor workarounds.detect_format

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: storage: Refactor workarounds.detect_format
..

storage: Refactor workarounds.detect_format

The function workarounds.detect_format accepts two volumes, checks for a
special case, then returns the qemuimg formats that should be used for
each volume.  To make this logic more sharable, change detect_format to
just detect if the problem exists and let the calling code decide which
format to use.  This will allow the sdm.copy_data code to use this
function with a single volume and still apply the proper logic.

Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Signed-off-by: Adam Litke 
---
M tests/storage_workarounds_test.py
M vdsm/storage/image.py
M vdsm/storage/workarounds.py
3 files changed, 28 insertions(+), 34 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/30/64230/1

diff --git a/tests/storage_workarounds_test.py 
b/tests/storage_workarounds_test.py
index 900b512..f0d01b8 100644
--- a/tests/storage_workarounds_test.py
+++ b/tests/storage_workarounds_test.py
@@ -32,17 +32,13 @@
 
 class DetectFormatTest(VdsmTestCase):
 
-def make_volumes(self, env, size, src_md_fmt, src_qemu_fmt, dst_md_fmt):
-src_img_id = str(uuid.uuid4())
-src_vol_id = str(uuid.uuid4())
-dst_img_id = str(uuid.uuid4())
-dst_vol_id = str(uuid.uuid4())
-env.make_volume(size, src_img_id, src_vol_id, vol_format=src_md_fmt)
-env.make_volume(size, dst_img_id, dst_vol_id, vol_format=dst_md_fmt)
-src_vol = env.sd_manifest.produceVolume(src_img_id, src_vol_id)
-dst_vol = env.sd_manifest.produceVolume(dst_img_id, dst_vol_id)
-qemuimg.create(src_vol.getVolumePath(), size, src_qemu_fmt)
-return src_vol, dst_vol
+def make_volume(self, env, size, src_md_fmt, src_qemu_fmt):
+img_id = str(uuid.uuid4())
+vol_id = str(uuid.uuid4())
+env.make_volume(size, img_id, vol_id, vol_format=src_md_fmt)
+vol = env.sd_manifest.produceVolume(img_id, vol_id)
+qemuimg.create(vol.getVolumePath(), size, src_qemu_fmt)
+return vol
 
 def test_bad_format_vm_conf_disk(self):
 """
@@ -52,10 +48,9 @@
 """
 size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
 with fake_file_env() as env:
-src, dst = self.make_volumes(env, size, sc.COW_FORMAT,
- qemuimg.FORMAT.RAW, sc.RAW_FORMAT)
-self.assertEqual((qemuimg.FORMAT.RAW, qemuimg.FORMAT.RAW),
- workarounds.detect_format(src, dst))
+vol = self.make_volume(env, size, sc.COW_FORMAT,
+   qemuimg.FORMAT.RAW)
+self.assertTrue(workarounds.mismatched_vm_conf_disk(vol))
 
 def test_bad_format_other_size(self):
 """
@@ -64,10 +59,9 @@
 """
 size = 2 * workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
 with fake_file_env() as env:
-src, dst = self.make_volumes(env, size, sc.COW_FORMAT,
- qemuimg.FORMAT.RAW, sc.RAW_FORMAT)
-self.assertEqual((qemuimg.FORMAT.QCOW2, qemuimg.FORMAT.RAW),
- workarounds.detect_format(src, dst))
+vol = self.make_volume(env, size, sc.COW_FORMAT,
+   qemuimg.FORMAT.RAW)
+self.assertFalse(workarounds.mismatched_vm_conf_disk(vol))
 
 def test_cow_vm_conf_disk(self):
 """
@@ -75,7 +69,6 @@
 """
 size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
 with fake_file_env() as env:
-src, dst = self.make_volumes(env, size, sc.COW_FORMAT,
- qemuimg.FORMAT.QCOW2, sc.COW_FORMAT)
-self.assertEqual((qemuimg.FORMAT.QCOW2, qemuimg.FORMAT.QCOW2),
- workarounds.detect_format(src, dst))
+vol = self.make_volume(env, size, sc.COW_FORMAT,
+   qemuimg.FORMAT.QCOW2)
+self.assertFalse(workarounds.mismatched_vm_conf_disk(vol))
diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index 37619d9..7adc199 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -455,8 +455,12 @@
 try:
 dstVol = destDom.produceVolume(imgUUID=imgUUID,
volUUID=srcVol.volUUID)
-srcFormat, dstFormat = workarounds.detect_format(srcVol,
- dstVol)
+
+if workarounds.mismatched_vm_conf_disk(srcVol):
+srcFormat = dstFormat = qemuimg.FORMAT.RAW
+else:
+srcFormat = sc.fmt2str(srcVol.getFormat())
+dstFormat = 

Change in vdsm[master]: storage: Refactor workarounds.detect_format

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Refactor workarounds.detect_format
..


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/64230
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7b2be176c474f9049d067f0a0c169644ac39899
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
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