Change in vdsm[master]: StorageDomainManifest: introduce class heirarchy

alitke at redhat.com alitke at redhat.com
Tue Jun 16 21:39:31 UTC 2015


Adam Litke has posted comments on this change.

Change subject: StorageDomainManifest: introduce class heirarchy
......................................................................


Patch Set 9:

(1 comment)

https://gerrit.ovirt.org/#/c/41993/9/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 402:     def __init__(self, sdUUID):
Line 403:         domaindir = os.path.join(self.mountpoint, sdUUID)
Line 404:         sd.StorageDomainManifest.__init__(self, sdUUID, domaindir)
Line 405: 
Line 406:     def bind(self, metadata=None):
> What will happen if you forget to call bind? Will every/some operation fail
No, it will probably be an AttributeError.  I'd rather not pollute the code with runtime checks to guard against a programming error.  There are plenty of classes in vdsm that can be misused more easily than this one :)

Sure, but lazy initialization causes other problems.  It will become hard for class users to know which operations can be performed on the class without triggering the implicit bind.  I'd prefer an explicit call to bind.
Line 407:         if metadata is None:
Line 408:             metadata = selectMetadata(self.sdUUID)
Line 409:         self.replaceMetadata(metadata)
Line 410: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia275424a8d423722efb229a4d8545f035fe5fa51
Gerrit-PatchSet: 9
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: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
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