Change in vdsm[master]: Introduce VolumeArtifacts

alitke at redhat.com alitke at redhat.com
Mon Dec 7 22:23:58 UTC 2015


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 <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list