Change in vdsm[master]: gluster: Refactor GlusterFSConnection class

nsoffer at redhat.com nsoffer at redhat.com
Wed May 6 12:40:39 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: gluster: Refactor GlusterFSConnection class
......................................................................


Patch Set 3:

(4 comments)

https://gerrit.ovirt.org/#/c/40575/3/vdsm/storage/storageServer.py
File vdsm/storage/storageServer.py:

Line 166
Line 167
Line 168
Line 169
Line 170
Previously, we use to create the local path by:

    os.path.join(self.getLocalpathBase(), remote_path)

And we customized the behavior by overriding self.getLocalpathBase(), accessing MountConnection.getLocalPathBase()

We also have code to customize MountConnection.localPathBase during runtime,
I don't see any need to support this.

We used the term "localPath" instead of the more common term "mountpoint".

This is clumsy and more complicated than needed. We can use this instead:

    class MountConnection(object):
    
        ROOT = "/tmp"
        DIR = ""

Subclass can customized by overriding DIR.


Line 199
Line 200
Line 201
Line 202
Line 203
Use:

    path = self._remotePath.replace("_", "__").replace("/", "_")
    return os.path.join(self.ROOT, self.DIR, path)


Line 200: 
Line 201:     def _getLocalPath(self):
Line 202:         localPath = self.getLocalPathBase()
Line 203:         if self._localPath:
Line 204:             localPath = self._localPath
Not needed
Line 205: 
Line 206:         return os.path.join(localPath,
Line 207:                             self._remotePath.replace("_",
Line 208:                                                      "__").replace("/", "_"))


Line 258:     def __hash__(self):
Line 259:         return hash(type(self)) ^ hash(self._mount)
Line 260: 
Line 261: 
Line 262: class GlusterFSConnection(object):
Generally I prefer composition over sub-classing, but in this case we don't have any behavior in the subclass, so creating an object using MountConnection is wasted effort.

The only differences between MountConnection and GlusterfsConnection are
- "glusterSD" used  in the mountpoint
- "glusterfs" used for vfsType

So a better change would be:

    GlusterFSConnection(MountConnection):
        VFSTYPE = "glusterfs"
        DIR = "gluserSD"

And change MountConnection to use this class constants - see the comments in MountConnection.

The other change planed is to modify options before connecting. We can
override the options property in the subclass, and return a modified self._options.
Line 263: 
Line 264:     def __init__(self, spec, vfsType="glusterfs", options=""):
Line 265:         self._vfsType = vfsType
Line 266:         self._remotePath = spec


-- 
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: 3
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