Change in vdsm[master]: Prefetch domains when connecting a storage server.

ewarszaw at redhat.com ewarszaw at redhat.com
Sun Mar 31 08:48:21 UTC 2013


Eduardo has posted comments on this change.

Change subject: Prefetch domains when connecting a storage server.
......................................................................


Patch Set 6: (5 inline comments)

....................................................
File vdsm/storage/hsm.py
Line 2238: 
Line 2239:     def __prefetchDomains(self, domType, conObj):
Line 2240:         uuidPatern = "????????-????-????-????-????????????"
Line 2241: 
Line 2242:         if domType in (sd.FCP_DOMAIN, sd.ISCSI_DOMAIN):
The assumption that "direct" chunks of storage will be luns forever may be not true.
There is no real reason in the code for not using "direct files".

The too wide vgs can be improved issuing a vgs only in the added luns when connecting, narrowing the filter.
Line 2243:             uuids = tuple(blockSD.getStorageDomainsList())
Line 2244:         elif domType is sd.NFS_DOMAIN:
Line 2245:             lPath = conObj._mountCon._getLocalPath()
Line 2246:             self.log.debug("nfs local path: %s", lPath)


Line 2241: 
Line 2242:         if domType in (sd.FCP_DOMAIN, sd.ISCSI_DOMAIN):
Line 2243:             uuids = tuple(blockSD.getStorageDomainsList())
Line 2244:         elif domType is sd.NFS_DOMAIN:
Line 2245:             lPath = conObj._mountCon._getLocalPath()
Since a NFSConnection is composed by (opposed to inherit) a MountConnection, exposing such method will lead to two different methods, with the same name and producing the same result in the NFSMount class!

Here is confuse but storageServer is a mess! The classes should be redefined, as we already discussed.

This should not be done for the sake of simplify this unique function only.
Line 2246:             self.log.debug("nfs local path: %s", lPath)
Line 2247:             goop = oop.getGlobalProcPool()
Line 2248:             uuids = tuple(os.path.basename(d) for d in
Line 2249:                           goop.glob.glob(os.path.join(lPath, uuidPatern)))


Line 2256:         elif domType is sd.GLUSTERFS_DOMAIN:
Line 2257:             glusterDomPath = os.path.join(sd.GLUSTERSD_DIR, "*")
Line 2258:             self.log.debug("glusterDomPath: %s", glusterDomPath)
Line 2259:             uuids = tuple(sdUUID for sdUUID, domainPath in
Line 2260:                           nfsSD.fileSD.scanDomains(glusterDomPath))
Is preferable to use scanDomains() always, like here, since in this way we can unify the logic of what the domain definition is.
But using scanDomains() for POSIXFS and NFS implies a big refactoring out of the scope of a zstream patch.
Line 2261:         elif domType is sd.LOCALFS_DOMAIN:
Line 2262:             lPath = conObj._path
Line 2263:             self.log.debug("local _path: %s", lPath)
Line 2264:             uuids = tuple(os.path.basename(d) for d in


Line 2258:             self.log.debug("glusterDomPath: %s", glusterDomPath)
Line 2259:             uuids = tuple(sdUUID for sdUUID, domainPath in
Line 2260:                           nfsSD.fileSD.scanDomains(glusterDomPath))
Line 2261:         elif domType is sd.LOCALFS_DOMAIN:
Line 2262:             lPath = conObj._path
The names are confusing; getLocalPath() returns a different path.

We want to avoid using oop for local domains, if local has any sense at all.
Line 2263:             self.log.debug("local _path: %s", lPath)
Line 2264:             uuids = tuple(os.path.basename(d) for d in
Line 2265:                           glob.glob(os.path.join(lPath, uuidPatern)))
Line 2266:         else:


Line 2307:                     "Could not connect to storageServer", exc_info=True)
Line 2308:                 status, _ = self._translateConnectionError(err)
Line 2309:             else:
Line 2310:                 status = 0
Line 2311:                 doms = self.__prefetchDomains(domType, conObj)
__prefetchDomains() fail to find domains will return an empty doms iterable.

It will not raise if they are not programmatic (systematic) bugs.
Line 2312:                 sdCache.knownSDs.update(doms)
Line 2313: 
Line 2314:             self.log.debug("knownSDs: %s", sdCache.knownSDs)
Line 2315: 


--
To view, visit http://gerrit.ovirt.org/12869
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f2b19a566c659bac30a318d17cef929e0b2575b
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov at gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list