Change in vdsm[master]: mount: Reassign mount specification in case of backup option

smizrahi at redhat.com smizrahi at redhat.com
Thu Jul 11 09:11:15 UTC 2013


Saggi Mizrahi has posted comments on this change.

Change subject: mount: Reassign mount specification in case of backup option
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(7 inline comments)

....................................................
File vdsm/storage/mount.py
Line 186: 
Line 187:     raise OSError(errno.ENOENT, 'device %s not mounted' % device)
Line 188: 
Line 189: 
Line 190: def _getRecord(spec, file):
Why move it out of the instance. If you have the spec and file you can get a mount instance easily.
Line 191:     for record in _iterMountRecords():
Line 192:         if (record.fs_spec == spec and record.fs_file == file):
Line 193:             return record
Line 194: 


....................................................
File vdsm/storage/storageServer.py
Line 196:         return os.path.join(self.getLocalPathBase(),
Line 197:                             self._remotePath.replace("_",
Line 198:                                                      "__").replace("/", "_"))
Line 199: 
Line 200:     def createConnection(self):
Why is this public
Line 201:         if self._mount.isMounted():
Line 202:             return
Line 203: 
Line 204:         fileUtils.createdir(self._getLocalPath())


Line 213:                 self.log.warn("Failed to remove mount point directory: %s",
Line 214:                               self._getLocalPath(), exc_info=True)
Line 215:             raise e
Line 216: 
Line 217:     def validateConnection(self):
Shouldn't be public and you don't validate the connection you validate the mount point.
Line 218:         try:
Line 219:             fileSD.validateDirAccess(
Line 220:                 self.getMountObj().getRecord().fs_file)
Line 221:         except se.StorageServerAccessPermissionError as e:


Line 253:     def __hash__(self):
Line 254:         return hash(type(self)) ^ hash(self._mount)
Line 255: 
Line 256: 
Line 257: class GlusterFSConnection(MountConnection):
Who allowed this????
Line 258: 
Line 259:     def getLocalPathBase(cls):
Line 260:         return os.path.join(MountConnection.getLocalPathBase(), "glusterSD")
Line 261: 


Line 259:     def getLocalPathBase(cls):
Line 260:         return os.path.join(MountConnection.getLocalPathBase(), "glusterSD")
Line 261: 
Line 262:     def updateBackup(self):
Line 263:         if self.options and not self.isConnected():
if not self.options or self.isConnected():
    return

saves an indentation
Line 264:             for opt in self.options.split(','):
Line 265:                 if opt.strip().startswith("backupvolfile-server"):
Line 266:                     backupSpec = opt.split('=', 1)[1].strip()
Line 267:                     try:


Line 262:     def updateBackup(self):
Line 263:         if self.options and not self.isConnected():
Line 264:             for opt in self.options.split(','):
Line 265:                 if opt.strip().startswith("backupvolfile-server"):
Line 266:                     backupSpec = opt.split('=', 1)[1].strip()
move the iteration to it's own function
self.getBackupSpec() so the logic is not in the middle of the iteration.
Line 267:                     try:
Line 268:                         mount._getRecord(backupSpec, self._mount.fs_file)
Line 269:                     except OSError as e:
Line 270:                         self.log.warning("Mount of backup failed: %s", e)


Line 264:             for opt in self.options.split(','):
Line 265:                 if opt.strip().startswith("backupvolfile-server"):
Line 266:                     backupSpec = opt.split('=', 1)[1].strip()
Line 267:                     try:
Line 268:                         mount._getRecord(backupSpec, self._mount.fs_file)
if mount.Mount(backupSpec, self._mout.fs_file).isMounted()
Line 269:                     except OSError as e:
Line 270:                         self.log.warning("Mount of backup failed: %s", e)
Line 271:                     else:
Line 272:                         self._mount.fs_spec = backupSpec


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3166c6863dffa297bc0adcdeb4c22f810d18de8e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>


More information about the vdsm-patches mailing list