Saggi Mizrahi has uploaded a new change for review.
Change subject: Have direct file not use the O_DIRECT flag on tmpfs and ramfs ......................................................................
Have direct file not use the O_DIRECT flag on tmpfs and ramfs
tmpfs and ramfs don't support the O_DIRECT flags. This is intentional as O_DIRECT tells the kernel to bypass the page-cache and those file systems live solely on the page cache. Since the effect desired by direct IO is accomplished on these file systems without the flag there is no reason for the use to test the FS every time before opening a file for direct access.
The reason we keep the same class instead of falling back to the regular file object is so we keep the semantics of the DirectFile() class and the user doesn't have to care that the underlying FS doesn't really support direct IO.
Change-Id: I7db4136c1a34d960b17312c2c785fc3234b24b92 Signed-off-by: Saggi Mizrahi smizrahi@redhat.com --- M vdsm/storage/fileUtils.py M vdsm/storage/mount.py 2 files changed, 51 insertions(+), 16 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/95/9595/1
diff --git a/vdsm/storage/fileUtils.py b/vdsm/storage/fileUtils.py index 13a8cfa..e00a004 100644 --- a/vdsm/storage/fileUtils.py +++ b/vdsm/storage/fileUtils.py @@ -31,11 +31,12 @@ import ctypes from contextlib import contextmanager import subprocess - import shutil -from vdsm import constants import logging import errno + +from vdsm import constants +import mount libc = ctypes.CDLL("libc.so.6", use_errno=True)
log = logging.getLogger('fileUtils') @@ -184,10 +185,13 @@ log.warning("Dir %s already exists", dirPath) if mode is not None: statinfo = os.stat(dirPath) - if statinfo[stat.ST_MODE] != mode: + curMode = statinfo[stat.ST_MODE] + if curMode != mode: raise OSError(errno.EPERM, - "Existing %s permissions %s are not as requested %s" % - (dirPath, oct(statinfo[stat.ST_MODE]), oct(mode))) + ("Existing %s permissions %s are not as " + "requested %s") % (dirPath, + oct(curMode), + oct(mode)))
def resolveUid(user): @@ -219,7 +223,7 @@ currentGid = stat.st_gid
if ((uid == currentUid or user == -1) and - (gid == currentGid or group == -1)): + (gid == currentGid or group == -1)): return True
os.chown(path, uid, gid) @@ -243,7 +247,14 @@ raise ValueError("Invalid mode parameter")
self._writable = True - flags = os.O_DIRECT + # Memory only file systems don't support direct IO because direct IO + # means "skip the page cache" and they are 100% page cahce. + vfstype = mount.findMountOfPath(path).getRecord().fs_vfstype + if vfstype in ["tmpfs", "ramfs"]: + flags = 0 + else: + flags = os.O_DIRECT + if "r" in mode: if "+" in mode: flags |= os.O_RDWR diff --git a/vdsm/storage/mount.py b/vdsm/storage/mount.py index 94cfae0..aaa9eb0 100644 --- a/vdsm/storage/mount.py +++ b/vdsm/storage/mount.py @@ -34,14 +34,14 @@ VFS_EXT3 = "ext3"
MountRecord = namedtuple("MountRecord", "fs_spec fs_file fs_vfstype " - "fs_mntops fs_freq fs_passno") + "fs_mntops fs_freq fs_passno")
_RE_ESCAPE = re.compile(r"\0\d\d")
def _parseFstabLine(line): (fs_spec, fs_file, fs_vfstype, fs_mntops, - fs_freq, fs_passno) = line.split()[:6] + fs_freq, fs_passno) = line.split()[:6] fs_mntops = fs_mntops.split(",") fs_freq = int(fs_freq) fs_passno = int(fs_passno) @@ -50,7 +50,7 @@ fs_mntops = [_parseFstabPath(item) for item in fs_mntops]
return MountRecord(fs_spec, fs_file, fs_vfstype, fs_mntops, - fs_freq, fs_passno) + fs_freq, fs_passno)
def _iterateMtab(): @@ -118,7 +118,7 @@ continue
yield MountRecord(realSpec, rec.fs_file, rec.fs_vfstype, - rec.fs_mntops, rec.fs_freq, rec.fs_passno) + rec.fs_mntops, rec.fs_freq, rec.fs_passno)
def iterMounts(): @@ -144,6 +144,30 @@ return Mount(rec.fs_spec, rec.fs_file)
raise OSError(errno.ENOENT, 'Mount target %s not found' % target) + + +def findMountOfPath(path): + # TBD: Bind mounts, should we ignore them? + # Follow symlinks (if file exists) + path = os.path.realpath(path) + + # Make sure using canonical representation + path = os.path.normpath(path) + + # Find longest match + maxLen = 0 + mountRec = None + for rec in _iterMountRecords(): + mntPath = os.path.normpath(rec.fs_file) + if len(mntPath) > maxLen: + if path.startswith(mntPath): + maxLen = len(mntPath) + mountRec = rec + + if mountRec is None: + raise OSError(errno.ENOENT, os.strerror(errno.ENOENT)) + + return Mount(mountRec.fs_spec, mountRec.fs_file)
def getMountFromDevice(device): @@ -195,7 +219,7 @@ if not p.wait(timeout): p.kill() raise OSError(errno.ETIMEDOUT, - "%s operation timed out" % os.path.basename(cmd[0])) + "%s operation timed out" % os.path.basename(cmd[0]))
out, err = p.communicate() rc = p.returncode @@ -232,9 +256,9 @@ return record
raise OSError(errno.ENOENT, - "Mount of `%s` at `%s` does not exist" % - (self.fs_spec, self.fs_file)) + "Mount of `%s` at `%s` does not exist" % + (self.fs_spec, self.fs_file))
def __repr__(self): - return "<Mount fs_spec='%s' fs_file='%s'>" % \ - (self.fs_spec, self.fs_file) + return ("<Mount fs_spec='%s' fs_file='%s'>" % + (self.fs_spec, self.fs_file))
-- To view, visit http://gerrit.ovirt.org/9595 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I7db4136c1a34d960b17312c2c785fc3234b24b92 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com