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