Change in vdsm[master]: Introduce VolumeArtifacts

nsoffer at redhat.com nsoffer at redhat.com
Fri Apr 1 18:58:33 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: Introduce VolumeArtifacts
......................................................................


Patch Set 27: Code-Review-1

(4 comments)

https://gerrit.ovirt.org/#/c/48097/26/tests/storage_volume_artifacts_test.py
File tests/storage_volume_artifacts_test.py:

Line 211:         # not allowed.
Line 212:         with self.fake_env() as env:
Line 213:             artifacts = env.sd_manifest.get_volume_artifacts(
Line 214:                 self.img_id, self.vol_id)
Line 215:             artifacts._create_metadata_artifact = self.failure
> Needed for block volume tests coming up and I figured I might as well estab
OK
Line 216:             self.assertRaises(ExpectedFailure, artifacts.create,
Line 217:                               *BASE_RAW_PARAMS)
Line 218:             artifacts = env.sd_manifest.get_volume_artifacts(
Line 219:                 self.img_id, self.vol_id)


https://gerrit.ovirt.org/#/c/48097/27/tests/storage_volume_artifacts_test.py
File tests/storage_volume_artifacts_test.py:

Line 147: 
Line 148:             # File volumes are always sparse regardless of format
Line 149:             for fmt in volume.RAW_FORMAT, volume.COW_FORMAT:
Line 150:                 prealloc = artifacts._get_volume_preallocation(fmt)
Line 151:                 self.assertEqual(volume.SPARSE_VOL, prealloc)
This tests the implementation instead of the behavior, so we will have to change the test if we change the internal private helper. I suggest to remove this test and keep only the next test. When we add support for cow format, we can add another test  like the next test for sparseness.
Line 152: 
Line 153:     def test_raw_volume_preallocation(self):
Line 154:         with self.fake_env() as env:
Line 155:             artifacts = env.sd_manifest.get_volume_artifacts(


https://gerrit.ovirt.org/#/c/48097/26/vdsm/storage/sdm/volume_artifacts.py
File vdsm/storage/sdm/volume_artifacts.py:

Line 227:         # If we created a new image directory, rename it to the correct name
Line 228:         if not self.is_image():
Line 229:             self._oop.os.rename(self.artifacts_dir, self._image_dir)
Line 230: 
Line 231:         self._image_exists = True
> We can remove this now.
Adam?
Line 232: 
Line 233:     def _get_volume_preallocation(self, vol_format):
Line 234:         # File volumes are always sparse regardless of format
Line 235:         return volume.SPARSE_VOL


https://gerrit.ovirt.org/#/c/48097/27/vdsm/storage/sdm/volume_artifacts.py
File vdsm/storage/sdm/volume_artifacts.py:

Line 227:         # If we created a new image directory, rename it to the correct name
Line 228:         if not self.is_image():
Line 229:             self._oop.os.rename(self.artifacts_dir, self._image_dir)
Line 230: 
Line 231:         self._image_exists = True
This is unused, any reason to keep it?
Line 232: 
Line 233:     def _get_volume_preallocation(self, vol_format):
Line 234:         # File volumes are always sparse regardless of format
Line 235:         return volume.SPARSE_VOL


-- 
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: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Amit Aviram <aaviram at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipchuk at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: Vered Volansky <vvolansk at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list