Adam Litke has posted comments on this change.

Change subject: storage: Move detect_format to new workarounds module
......................................................................


Patch Set 1:

(5 comments)

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

Line 31: 
Line 32: 
Line 33: class DetectFormatTest(VdsmTestCase):
Line 34: 
Line 35:     def make_volumes(self, env, size, src_md_fmt, src_qemu_fmt, 
dst_md_fmt):
> Are you sure we need both src and dst format? We use this workaround when w
Done
Line 36:         src_img_id = str(uuid.uuid4())
Line 37:         src_vol_id = str(uuid.uuid4())
Line 38:         dst_img_id = str(uuid.uuid4())
Line 39:         dst_vol_id = str(uuid.uuid4())


Line 31: 
Line 32: 
Line 33: class DetectFormatTest(VdsmTestCase):
Line 34: 
Line 35:     def make_volumes(self, env, size, src_md_fmt, src_qemu_fmt, 
dst_md_fmt):
> Looking at the next version, we should just have make_volume() here, and cr
Done
Line 36:         src_img_id = str(uuid.uuid4())
Line 37:         src_vol_id = str(uuid.uuid4())
Line 38:         dst_img_id = str(uuid.uuid4())
Line 39:         dst_vol_id = str(uuid.uuid4())


Line 49:         When the volume size matches the VM configuration disk size 
and the
Line 50:         source volume reports COW even though qemuimg reports RAW then 
we
Line 51:         expect the workaround to report both volumes as RAW.
Line 52:         """
Line 53:         size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
> Having VM_CONF_SIZE (in bytes) would be much nicer, at leat for the tests.
Solved by just using a constant at the top of the file.
Line 54:         with fake_file_env() as env:
Line 55:             src, dst = self.make_volumes(env, size, sc.COW_FORMAT,
Line 56:                                          qemuimg.FORMAT.RAW, 
sc.RAW_FORMAT)
Line 57:             self.assertEqual((qemuimg.FORMAT.RAW, qemuimg.FORMAT.RAW),


Line 52:         """
Line 53:         size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
Line 54:         with fake_file_env() as env:
Line 55:             src, dst = self.make_volumes(env, size, sc.COW_FORMAT,
Line 56:                                          qemuimg.FORMAT.RAW, 
sc.RAW_FORMAT)
> This is not clear.
Applied the suggestions about using friendly format strings.
Line 57:             self.assertEqual((qemuimg.FORMAT.RAW, qemuimg.FORMAT.RAW),
Line 58:                              workarounds.detect_format(src, dst))
Line 59: 
Line 60:     def test_bad_format_other_size(self):


Line 52:         """
Line 53:         size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
Line 54:         with fake_file_env() as env:
Line 55:             src, dst = self.make_volumes(env, size, sc.COW_FORMAT,
Line 56:                                          qemuimg.FORMAT.RAW, 
sc.RAW_FORMAT)
> Looking in the next version, I don't know if it worth to add permutations h
ok.
Line 57:             self.assertEqual((qemuimg.FORMAT.RAW, qemuimg.FORMAT.RAW),
Line 58:                              workarounds.detect_format(src, dst))
Line 59: 
Line 60:     def test_bad_format_other_size(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If4da9d2c16679f99b55438d7336d0cfb27429c12
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@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