Adam Litke has posted comments on this change. Change subject: Introduce VolumeArtifacts ......................................................................
Patch Set 4: (2 comments) https://gerrit.ovirt.org/#/c/48097/4/vdsm/storage/sdm/volumeartifacts.py File vdsm/storage/sdm/volumeartifacts.py: Line 86: self._meta_file_commit_path()) Line 87: except OSError as e: Line 88: if e.errno == errno.ENOENT: Line 89: raise VolumeArtifactsNotFound() Line 90: raise > I think we search for image-uuid-pattern/*.meta, but lets keep this simple UUID_GLOB_PATTERN = '*-*-*-*-*' Which picks up garbage images too :) Line 91: Line 92: # If we created a new image directory, rename it to the correct name Line 93: if self._get_artifacts_path() == self._new_image_path(): Line 94: self._oop.os.rename(self._get_artifacts_path(), Line 109: meta = fileVolume.FileVolumeMetadata.makeMetadata( Line 110: self.domain_manifest.sdUUID, self.img_id, parent_vol_id, size, Line 111: volume.type2name(vol_format), volume.type2name(prealloc), Line 112: leaf_type, disk_type, desc, volume.LEGAL_VOL) Line 113: fileVolume.FileVolumeMetadata.putArtifactMetadata(meta_id, meta) > Why we cannot use an instance here? Because you should only create a VolumeMetadata instance where there is an actual volume. Here we only have artifacts and some assumptions the VolumeMetadata class will want to make will not hold true here (ie. validateVolumePath). Line 114: Line 115: def _create_lease_artifact(self, meta_id): Line 116: fileVolume.FileVolumeMetadata.newVolumeLease( Line 117: meta_id, self.domain_manifest.sdUUID, self.vol_id) -- To view, visit https://gerrit.ovirt.org/48097 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I352423e39a899b9b83ccf3b8f6c17ec433e9c353 Gerrit-PatchSet: 4 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
