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 <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
