Adam Litke has posted comments on this change.

Change subject: storage: Use detect_format workaround in copy_data
......................................................................


Patch Set 6:

(5 comments)

https://gerrit.ovirt.org/#/c/64231/3/tests/storage_sdm_copy_data_test.py
File tests/storage_sdm_copy_data_test.py:

Line 218:     # Copy between 2 different domains
Line 219:     # Abort before copy
Line 220:     # Abort during copy
Line 221
Line 222
> After all this setup, we did not verify the copy, jus that it did not raise
Done


https://gerrit.ovirt.org/#/c/64231/6/tests/storage_sdm_copy_data_test.py
File tests/storage_sdm_copy_data_test.py:

Line 24: 
Line 25: from fakelib import FakeScheduler
Line 26: from monkeypatch import MonkeyPatchScope
Line 27: from storagefakelib import FakeResourceManager
Line 28: from storagetestlib import fake_env, fake_file_env
> pyflakes thinks this is unused:
Done
Line 29: from storagetestlib import make_qemu_chain, write_qemu_chain, 
verify_qemu_chain
Line 30: from storagetestlib import qemu_pattern_write, qemu_pattern_verify
Line 31: from storagetestlib import ChainVerificationError
Line 32: from testlib import VdsmTestCase, expandPermutations, permutations


Line 201:             src_vol = src_chain[0]
Line 202:             dst_vol = dst_chain[0]
Line 203:             src_path = src_vol.getVolumePath()
Line 204:             qemuimg.create(src_path, vm_conf_size, qemuimg.FORMAT.RAW)
Line 205:             qemu_pattern_write(src_path, qemuimg.FORMAT.RAW)
> We need to comment the part corrupting the qcow image, and separate it from
Done
Line 206:             source = dict(endpoint_type='div', sd_id=src_vol.sdUUID,
Line 207:                           img_id=src_vol.imgUUID, 
vol_id=src_vol.volUUID)
Line 208:             dest = dict(endpoint_type='div', sd_id=dst_vol.sdUUID,
Line 209:                         img_id=dst_vol.imgUUID, 
vol_id=dst_vol.volUUID)


Line 211:             job.run()
Line 212:             wait_for_job(job)
Line 213:             self.assertEqual(jobs.STATUS.DONE, job.status)
Line 214:             dst_path = dst_vol.getVolumePath()
Line 215:             qemu_pattern_verify(dst_path, qemuimg.FORMAT.RAW)
> Since we are dealing with raw format, I think we can test this in a simpler
Close, but qemu pads the destination file to a 1k boundary.  See if you like 
what I have done.
Line 216: 
Line 217:     # TODO: Missing tests:
Line 218:     # Copy between 2 different domains
Line 219:     # Abort before copy


https://gerrit.ovirt.org/#/c/64231/3/vdsm/storage/sdm/api/copy_data.py
File vdsm/storage/sdm/api/copy_data.py:

Line 66:                     return
Line 67: 
Line 68:                 # Workaround for volumes containing VM configuration 
info that
Line 69:                 # were created with invalid vdsm metadata.
Line 70:                 if self._source.is_invalid_vm_conf_disk():
> is_xxx is typically a method, not property.
Done
Line 71:                     src_format = dst_format = qemuimg.FORMAT.RAW
Line 72:                 else:
Line 73:                     src_format = self._source.qemu_format
Line 74:                     dst_format = self._dest.qemu_format


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iecfd3e0fd5923e9a333379ab21e01c9b12def78c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to