Adam Litke has posted comments on this change. Change subject: storage: Add basic BlockVolumeArtifacts ......................................................................
Patch Set 14: (9 comments) https://gerrit.ovirt.org/#/c/55987/14/vdsm/storage/sdm/volume_artifacts.py File vdsm/storage/sdm/volume_artifacts.py: Line 300: - No logical volume exists Line 301: - Operations: Line 302: - is_garbage -> false Line 303: - is_image -> false Line 304: - create artifacts -> change state GARBAGE > The indentation is broken - we had the same problem in file volume. Done Line 305: Line 306: GARBAGE Line 307: Line 308: - States: Line 310: - Operations: Line 311: - is_garbage -> true Line 312: - is_image -> true or false Line 313: - clean -> change state to MISSING Line 314: - commit -> change state to VOLUME > Same Done Line 315: Line 316: VOLUME Line 317: Line 318: - States: Line 320: - Operations: Line 321: - is_garbage -> false Line 322: - is_image -> true Line 323: - create new volume -> change state to GARBAGE Line 324: - destroy this volume -> change state to GARBAGE > Same Done Line 325: """ Line 326: log = logging.getLogger('Storage.BlockVolumeArtifacts') Line 327: Line 328: def __init__(self, sd_manifest, img_id, vol_id): Line 338: return self.img_id in self.sd_manifest.getAllImages() Line 339: Line 340: def is_garbage(self): Line 341: lv = self._get_lv() Line 342: return lv and TEMP_VOL_LVTAG in lv.tags > The previous code was much better. Done Line 343: Line 344: @property Line 345: def volume_path(self): Line 346: return lvm.lvPath(self.sd_manifest.sdUUID, self.vol_id) Line 360: Line 361: def commit(self): Line 362: if self._get_lv() and not self.is_garbage(): Line 363: raise se.VolumeAlreadyExists("LV %r has already been committed" % Line 364: self.vol_id) > This should fail if: Done Line 365: lvm.changeLVTags(self.sd_manifest.sdUUID, self.vol_id, addTags=(), Line 366: delTags=(TEMP_VOL_LVTAG,)) Line 367: Line 368: def get_volume_preallocation(self, vol_format): Line 361: def commit(self): Line 362: if self._get_lv() and not self.is_garbage(): Line 363: raise se.VolumeAlreadyExists("LV %r has already been committed" % Line 364: self.vol_id) Line 365: lvm.changeLVTags(self.sd_manifest.sdUUID, self.vol_id, addTags=(), > Isn't addTags=() redundant? Done Line 366: delTags=(TEMP_VOL_LVTAG,)) Line 367: Line 368: def get_volume_preallocation(self, vol_format): Line 369: if vol_format == volume.RAW_FORMAT: Line 370: return volume.PREALLOCATED_VOL Line 371: else: Line 372: return volume.SPARSE_VOL Line 373: Line 374: def _get_lv(self): > This helper is harmful - returning None instead of raising lead to Attribut Done Line 375: try: Line 376: return lvm.getLV(self.sd_manifest.sdUUID, self.vol_id) Line 377: except se.LogicalVolumeDoesNotExistError: Line 378: return None Line 404: if vol_format == volume.RAW_FORMAT: Line 405: dev_size = int(self.sd_manifest.getVSize(self.img_id, self.vol_id)) Line 406: if dev_size > size: Line 407: return dev_size Line 408: return size > Since we don't check that dev_size < size, we can simplify this now to: Done Line 409: Line 410: def _create_metadata(self, size, vol_format, prealloc, disk_type, desc, Line 411: parent): Line 412: size = self._get_lv_actual_size(vol_format, size) Line 415: self.vol_id, blockVolume.VOLUME_MDNUMBLKS) as mdSlot: Line 416: sd_id = self.sd_manifest.sdUUID Line 417: lvm.changeLVTags(sd_id, self.vol_id, Line 418: addTags=[blockVolume.TAG_PREFIX_MD + str(mdSlot)]) Line 419: meta_id = (sd_id, mdSlot) > This function does two things: Done Line 420: Line 421: # We use the BlockVolumeManifest API here because we create the Line 422: # metadata in the standard way. We cannot do this for file volumes Line 423: # because the metadata needs to be written to a specially named file. -- 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: 14 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/mailman/listinfo/vdsm-patches
