Adam Litke has posted comments on this change. Change subject: storagetestlib: qemu chain verification ......................................................................
Patch Set 1: (11 comments) https://gerrit.ovirt.org/#/c/60999/1/tests/storagetestlib.py File tests/storagetestlib.py: Line 281: def _gen_pattern_bytes(): Line 282: # Produce a predictable sequence of pattern bytes (eg. '1', '2', '3') Line 283: byte_list = ['1', '2', '3', '4', '5', '6', '7', '8', '9'] Line 284: while byte_list: Line 285: yield byte_list.pop(0) > We can simplify this to: Removed. Line 286: Line 287: Line 288: def write_qemu_chain(vol_list): Line 289: # Starting with the base volume in vol_list, write to the chain in a Line 292: # vol chain index: 0K 1K 2K 3K Line 293: # 0: 1111 Line 294: # 1: 2222 Line 295: # 2: 3333 Line 296: # 3: 4444 > We should use zero-based patterns (000, 111...) I specifically avoided zeroes because a zero pattern is what we would have before writing anyway. So how would we detect the difference between not writing anything and writing the correct pattern? Line 297: # This allows us to verify the integrity of the whole chain. Line 298: byte_generator = _gen_pattern_bytes() Line 299: for i, vol in list(enumerate(vol_list)): Line 300: vol_fmt = sc.fmt2str(vol.getFormat()) Line 295: # 2: 3333 Line 296: # 3: 4444 Line 297: # This allows us to verify the integrity of the whole chain. Line 298: byte_generator = _gen_pattern_bytes() Line 299: for i, vol in list(enumerate(vol_list)): > We can simplify this to: Ok, but I want vol so I'll keep it as: for i, vol in enumerate(vol_list): Line 300: vol_fmt = sc.fmt2str(vol.getFormat()) Line 301: offset = "{}K".format(i) Line 302: pattern = byte_generator.next() Line 303: qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset, Line 298: byte_generator = _gen_pattern_bytes() Line 299: for i, vol in list(enumerate(vol_list)): Line 300: vol_fmt = sc.fmt2str(vol.getFormat()) Line 301: offset = "{}K".format(i) Line 302: pattern = byte_generator.next() > pattern is str(i) I'll use str(i + 1) in order to start with 1. Line 303: qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset, Line 304: len='1k', pattern=pattern) Line 305: Line 306: Line 313: # and verifying the pattern written by write_chain. Line 314: byte_generator = _gen_pattern_bytes() Line 315: vol = vol_list[-1] Line 316: vol_fmt = sc.fmt2str(vol.getFormat()) Line 317: for i in list(range(len(vol_list))): > We can simplify this to: same as above. Line 318: offset = "{}K".format(i) Line 319: pattern = byte_generator.next() Line 320: if not qemu_pattern_verify(vol.volumePath, vol_fmt, offset=offset, Line 321: len='1k', pattern=pattern): Line 315: vol = vol_list[-1] Line 316: vol_fmt = sc.fmt2str(vol.getFormat()) Line 317: for i in list(range(len(vol_list))): Line 318: offset = "{}K".format(i) Line 319: pattern = byte_generator.next() > The pattern is str(i) we don't need the byte_generator. str(i + 1) Line 320: if not qemu_pattern_verify(vol.volumePath, vol_fmt, offset=offset, Line 321: len='1k', pattern=pattern): Line 322: raise ChainVerificationError("Verification failed at offset %s" % Line 323: offset) Line 319: pattern = byte_generator.next() Line 320: if not qemu_pattern_verify(vol.volumePath, vol_fmt, offset=offset, Line 321: len='1k', pattern=pattern): Line 322: raise ChainVerificationError("Verification failed at offset %s" % Line 323: offset) > Since we check only the top volume, we will see all patterns written to any Done Line 324: Line 325: Line 326: def make_qemu_chain(env, size, base_vol_fmt, chain_len): Line 327: vol_list = [] Line 327: vol_list = [] Line 328: img_id = str(uuid.uuid4()) Line 329: parent_vol_id = sc.BLANK_UUID Line 330: vol_fmt = base_vol_fmt Line 331: for i in range(chain_len): > The pattern we should write here is str(i) str(i+1) Line 332: vol_id = str(uuid.uuid4()) Line 333: if parent_vol_id != sc.BLANK_UUID: Line 334: vol_fmt = sc.COW_FORMAT Line 335: env.make_volume(size, img_id, vol_id, Line 337: vol = env.sd_manifest.produceVolume(img_id, vol_id) Line 338: if vol_fmt == sc.COW_FORMAT: Line 339: backing = parent_vol_id if parent_vol_id != sc.BLANK_UUID else None Line 340: qemuimg.create(vol.volumePath, size=size, Line 341: format=qemuimg.FORMAT.QCOW2, backing=backing) > Writing the pattern here would be better. No, because I might want to use this helper for other tests without doing the chain poisoning. Line 342: vol_list.append(vol) Line 343: parent_vol_id = vol_id https://gerrit.ovirt.org/#/c/60999/1/tests/storagetestlibTests.py File tests/storagetestlibTests.py: Line 237: qemu_pattern_write(path, img_format, pattern='2') Line 238: self.assertFalse(qemu_pattern_verify(path, img_format, Line 239: pattern='4')) Line 240: Line 241: @permutations((('file',), ('block',))) > This tests both file_env and block_env, but the underlying writes are alway ok Line 242: def test_verify_chain(self, storage_type): Line 243: with fake_env(storage_type) as env: Line 244: vol_list = make_qemu_chain(env, MB, sc.RAW_FORMAT, 2) Line 245: write_qemu_chain(vol_list) Line 249: def test_bad_chain_raises(self, storage_type): Line 250: with fake_env(storage_type) as env: Line 251: vol_list = make_qemu_chain(env, MB, sc.RAW_FORMAT, 2) Line 252: write_qemu_chain(vol_list[:1]) # Write the same pattern byte Line 253: write_qemu_chain(vol_list[1:]) # to both volumes in the chain > We can do one write like this: Done Line 254: self.assertRaises(ChainVerificationError, Line 255: verify_qemu_chain, vol_list) Line 256: Line 257: -- To view, visit https://gerrit.ovirt.org/60999 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390 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 https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
