Nir Soffer has posted comments on this change.

Change subject: StorageDomainManifest: move BlockSD.getVSize
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/41994/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 420:                 self.log.warn("Could not get size for vol %s/%s",
Line 421:                               self.sdUUID, volUUID, exc_info=True)
Line 422:                 raise
Line 423: 
Line 424:         return int(size)
Lets add a test for this method. Now that the manifest is small and has minimal 
dependencies, it should be easy to add a test.

We need to mock lvm.lvPath and lvm.getLV in for this test.
Line 425: 
Line 426: 
Line 427: class BlockStorageDomain(sd.StorageDomain):
Line 428:     ManifestClass = BlockStorageDomainManifest


https://gerrit.ovirt.org/#/c/41994/2/vdsm/storage/sd.py
File vdsm/storage/sd.py:

Line 355:     def getReadDelay(self):
Line 356:         return self._manifest.getReadDelay()
Line 357: 
Line 358:     def getVSize(self, imgUUID, volUUID):
Line 359:         return self._manifest.getVSize(imgUUID, volUUID)
This breaks again the tests, since they do not know anything about the manifest:

 22:14:21 ======================================================================
 22:14:21 ERROR: test (volumeTests.FileVolumeGetVSizeTest)
 22:14:21 ----------------------------------------------------------------------
 22:14:21 Traceback (most recent call last):
 22:14:21   File "/tmp/run/vdsm/tests/volumeTests.py", line 77, in test
 22:14:21     self.sdobj.getVSize(self.imgUUID, self.volUUID) / SDBLKSZ)
 22:14:21   File "/tmp/run/vdsm/vdsm/storage/sd.py", line 359, in getVSize
 22:14:21     return self._manifest.getVSize(imgUUID, volUUID)
 22:14:21 AttributeError: 'FileDomainMockObject' object has no attribute 
'_manifest'

This test is about volume so we can just mock out getVSize() in 
FileDomainMockObject. But we also want to test the actual getVSize, which 
should be a FileStorageDomainManifest test.
Line 360: 
Line 361:     def prepareMailbox(self):
Line 362:         """
Line 363:         This method has been introduced in order to prepare the 
mailbox


-- 
To view, visit https://gerrit.ovirt.org/41994
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib00eba218cfb4af201aebcdc5071f95164c31687
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <[email protected]>
Gerrit-Reviewer: Ala Hino <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Freddy Rolland <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to