Adam Litke has posted comments on this change. Change subject: storage: Add basic BlockVolumeArtifacts ......................................................................
Patch Set 13: (15 comments) https://gerrit.ovirt.org/#/c/55987/13/tests/storage_volume_artifacts_test.py File tests/storage_volume_artifacts_test.py: Line 313: self.assertEqual(volume.PREALLOCATED_VOL, vol.getType()) Line 314: Line 315: def test_size_rounded_up(self): Line 316: # If the underlying device is larger the size will be updated Line 317: initial_size = (9 * MB) + 1024 change this to requested_size and only add 1. Line 318: expected_size = 10 * MB Line 319: with fake_block_env() as env: Line 320: artifacts = env.sd_manifest.get_volume_artifacts( Line 321: self.img_id, self.vol_id) Line 314: Line 315: def test_size_rounded_up(self): Line 316: # If the underlying device is larger the size will be updated Line 317: initial_size = (9 * MB) + 1024 Line 318: expected_size = 10 * MB get extent size from the vg and use that. Change fakelvm to round using vg extent size and write a test in storagefakelibtests for it. Line 319: with fake_block_env() as env: Line 320: artifacts = env.sd_manifest.get_volume_artifacts( Line 321: self.img_id, self.vol_id) Line 322: artifacts.create(initial_size, volume.RAW_FORMAT, Line 322: artifacts.create(initial_size, volume.RAW_FORMAT, Line 323: image.SYSTEM_DISK_TYPE, 'raw_volume') Line 324: artifacts.commit() Line 325: vol = env.sd_manifest.produceVolume(self.img_id, self.vol_id) Line 326: self.assertEqual(expected_size / volume.BLOCK_SIZE, vol.getSize()) also check lv size? Line 327: Line 328: # Invalid use of artifacts Line 329: Line 330: def test_commit_without_create(self): Line 338: with self.fake_env() as env: Line 339: artifacts = env.sd_manifest.get_volume_artifacts( Line 340: self.img_id, self.vol_id) Line 341: artifacts.create(*BASE_RAW_PARAMS) Line 342: artifacts.commit() check for the tag in commit so we can raise an exception for this. Line 343: artifacts.commit() # removing nonexistent tags is allowed Line 344: Line 345: def validate_artifacts(self, artifacts, env): Line 346: try: Line 354: md_slot = None Line 355: for tag in lv.tags: Line 356: if tag.startswith(blockVolume.TAG_PREFIX_MD): Line 357: md_slot = int(tag[len(blockVolume.TAG_PREFIX_MD):]) Line 358: break Use blockVolume._getVolumeTag Line 359: Line 360: self.validate_metadata(env, md_slot, artifacts) Line 361: Line 362: def validate_metadata(self, env, md_slot, artifacts): Line 357: md_slot = int(tag[len(blockVolume.TAG_PREFIX_MD):]) Line 358: break Line 359: Line 360: self.validate_metadata(env, md_slot, artifacts) Line 361: Add a TODO for validating the lease once sanlock is mocked. Line 362: def validate_metadata(self, env, md_slot, artifacts): Line 363: md_lines = misc.readblock( Line 364: env.lvm.lvPath(artifacts.sd_manifest.sdUUID, sd.METADATA), Line 365: md_slot * volume.METADATA_SIZE, volume.METADATA_SIZE) Line 359: Line 360: self.validate_metadata(env, md_slot, artifacts) Line 361: Line 362: def validate_metadata(self, env, md_slot, artifacts): Line 363: md_lines = misc.readblock( calc the lvpath in one line. calculate the offset explicitly in its own line. Line 364: env.lvm.lvPath(artifacts.sd_manifest.sdUUID, sd.METADATA), Line 365: md_slot * volume.METADATA_SIZE, volume.METADATA_SIZE) Line 366: md = volume.VolumeMetadata.from_lines(md_lines) Line 367: https://gerrit.ovirt.org/#/c/55987/13/vdsm/storage/sdm/volume_artifacts.py File vdsm/storage/sdm/volume_artifacts.py: Line 330: super(BlockVolumeArtifacts, self).__init__(sd_manifest, img_id, Line 331: vol_id) Line 332: Line 333: def is_image(self): Line 334: # TODO: Cache this value to avoid repeated expensive queries Change the comment to check if this call becomes too expensive. Line 335: return self.img_id in self.sd_manifest.getAllImages() Line 336: Line 337: def is_garbage(self): Line 338: try: Line 350: prealloc = self.get_volume_preallocation(vol_format) Line 351: self._validate_create_params(vol_format, parent, prealloc) Line 352: Line 353: lv_size = self.vol_class.calculate_volume_alloc_size( Line 354: prealloc, size / volume.BLOCK_SIZE, None) use a size_blk variable here. Line 355: parent_vol_id = parent.vol_id if parent else volume.BLANK_UUID Line 356: self._create_lv_artifact(parent_vol_id, lv_size) Line 357: meta_id = self._create_metadata(size, vol_format, prealloc, disk_type, Line 358: desc, parent_vol_id) Line 351: self._validate_create_params(vol_format, parent, prealloc) Line 352: Line 353: lv_size = self.vol_class.calculate_volume_alloc_size( Line 354: prealloc, size / volume.BLOCK_SIZE, None) Line 355: parent_vol_id = parent.vol_id if parent else volume.BLANK_UUID move this line into each function below. Line 356: self._create_lv_artifact(parent_vol_id, lv_size) Line 357: meta_id = self._create_metadata(size, vol_format, prealloc, disk_type, Line 358: desc, parent_vol_id) Line 359: self._create_lease(meta_id) Line 357: meta_id = self._create_metadata(size, vol_format, prealloc, disk_type, Line 358: desc, parent_vol_id) Line 359: self._create_lease(meta_id) Line 360: Line 361: def commit(self): if is_gabage: ... else raise Line 362: lvm.changeLVTags(self.sd_manifest.sdUUID, self.vol_id, addTags=(), Line 363: delTags=(TEMP_VOL_LVTAG,)) Line 364: Line 365: def get_volume_preallocation(self, vol_format): Line 384: lvm.createLV(self.sd_manifest.sdUUID, self.vol_id, lv_size, Line 385: activate=True, initialTags=(TEMP_VOL_LVTAG,)) Line 386: tags = [blockVolume.TAG_PREFIX_PARENT + parent_vol_id, Line 387: blockVolume.TAG_PREFIX_IMAGE + self.img_id] Line 388: lvm.changeLVTags(self.sd_manifest.sdUUID, self.vol_id, addTags=tags) add these during create call. Line 389: Line 390: def _get_lv_actual_size(self, vol_format, size): Line 391: # When the volume format is RAW the real volume capacity is the device Line 392: # size. The device size may have been rounded up if 'size' is not Line 392: # size. The device size may have been rounded up if 'size' is not Line 393: # divisible by the domain's extent size. Line 394: if vol_format == volume.RAW_FORMAT: Line 395: dev_size = int(self.sd_manifest.getVSize(self.img_id, self.vol_id)) Line 396: if dev_size < size: Let's get rid of this check. Not needed. Line 397: raise se.VolumeCreationError( Line 398: "The LV %s/%s is smaller than expected" % Line 399: (self.sd_manifest.sdUUID, self.vol_id)) Line 400: if dev_size > size: Line 411: lvm.changeLVTags(sd_id, self.vol_id, Line 412: addTags=[blockVolume.TAG_PREFIX_MD + str(mdSlot)]) Line 413: meta_id = (sd_id, mdSlot) Line 414: Line 415: self.vol_class.newMetadata( comment that we can use the regular api here because it's different than file. Line 416: meta_id, Line 417: sd_id, Line 418: self.img_id, Line 419: parent_vol_id, Line 416: meta_id, Line 417: sd_id, Line 418: self.img_id, Line 419: parent_vol_id, Line 420: size / volume.BLOCK_SIZE, # Size is stored as number of blocks use a size_blk helper. Line 421: volume.type2name(vol_format), Line 422: volume.type2name(prealloc), Line 423: volume.type2name(volume.LEAF_VOL), Line 424: disk_type, -- To view, visit https://gerrit.ovirt.org/55987 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4120c557fc89d82cc6bc854a9fdc8935e53bc93d Gerrit-PatchSet: 13 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] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
