Change in vdsm[master]: storageConnection: Refactor MountConnection class

nsoffer at redhat.com nsoffer at redhat.com
Tue Jun 2 21:29:14 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: storageConnection: Refactor MountConnection class
......................................................................


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/40575/5/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 368
Line 369
Line 370
Line 371
Line 372
This code is also setting the mountBasePath - you cannot change MountConnection and leave LocalDirectoryConnection without change.

If you want to fix this code, self.storage_repository should be computed and stored in one module, and accessed by all other modules using a function.

If the value is not available in import time, this must not be module or class constant. People assume that class or module constants are static values that never change after a module was imported.


Line 367: 
Line 368:         mountBasePath = os.path.join(self.storage_repository,
Line 369:                                      sd.DOMAIN_MNT_POINT)
Line 370:         fileUtils.createdir(mountBasePath)
Line 371:         storageServer.MountConnection.ROOT = mountBasePath
You cannot do that. If ROOT is a constant, it cannot be modified in runtime, and it certainly should not be modified directly by other code outside of the MountConnection class.

The fact that Python will let you do this does not mean that this is valid.

If this value is set in runtime, it must be done using an accessor, in the same way it was set before this patch, or it must be in some other module.
Line 372:         storageServer.LocalDirectoryConnection.setLocalPathBase(mountBasePath)
Line 373:         self._connectionAliasRegistrar = \
Line 374:             storageServer.ConnectionAliasRegistrar(STORAGE_CONNECTION_DIR)
Line 375:         self._connectionMonitor = \


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2a55a7f219a60c7ffb0ce981175492e560bb757f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini 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