Change in vdsm[master]: StorageDomainManifest: deleteImage and deps

nsoffer at redhat.com nsoffer at redhat.com
Fri Aug 14 21:09:29 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: StorageDomainManifest: deleteImage and deps
......................................................................


Patch Set 20: Code-Review-1

(3 comments)

https://gerrit.ovirt.org/#/c/42268/20/tests/sdm_indirection_tests.py
File tests/sdm_indirection_tests.py:

Line 302:         #    getattr (we must follow the name mangling rules).
Line 303:         # 2. __markForDelVols in the BlockStorageDomain redirects to the
Line 304:         #    _markForDelVols function in the BlockStorageDomainManifest.
Line 305:         self._check('_BlockStorageDomain__markForDelVols', (0, 1, 2, 3),
Line 306:                     [('_markForDelVols', (0, 1, 2, 3), {})])
You are working too hard for no reason.

First, there is no reason to use the __ prefix. lets name it to _markForDelVols to simplify things.

Second, I don't think we need to move private methods and leave a stub for calling them. We should move them without a stub and change the callers to use _manifest._method().
Line 307: 
Line 308:     @permutations([
Line 309:         ['extend', 2],
Line 310:         ['resizePV', 1],


Line 310:         ['resizePV', 1],
Line 311:         ['readMetadataMapping', 0],
Line 312:         ['extendVolume', 3],
Line 313:         ['rmDCImgDir', 2],
Line 314:         ['_getImgExclusiveVols', 2],
Don't leave a stub for this.
Line 315:     ])
Line 316:     def test_block_functions(self, fn, nargs=0):
Line 317:         self.check_call(fn, nargs)
Line 318: 


https://gerrit.ovirt.org/#/c/42268/20/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 1013
Line 1014
Line 1015
Line 1016
Line 1017
Just remove it to manifest and change the callers, this is not storage domain api.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e418262d70ed50b45fdddf2fc77eb80a03a3811
Gerrit-PatchSet: 20
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: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list