Change in vdsm[master]: Nits on isLocalFsDomain()

smizrahi at redhat.com smizrahi at redhat.com
Thu Jan 3 15:34:50 UTC 2013


Saggi Mizrahi has uploaded a new change for review.

Change subject: Nits on isLocalFsDomain()
......................................................................

Nits on isLocalFsDomain()

- Add followSymlinks parameter to findMountOfPath() for cases where the user
  wants to avoid touching the file for the price of reduced accuracy.
- Uses the storage.mount module because there is no reason to reinvent
  the wheel every time.
- Changes the logic to check that the domain mount is different from the
  mount dir mount. This is a bit more robust and a lot more correct.

Change-Id: I415e3ed87d1c2f9e0f67d90c5bcd8734e70e53c3
Bug-url: https://bugzilla.redhat.com/show_bug.cgi?id=886533
Signed-off-by: Saggi Mizrahi <smizrahi at redhat.com>
---
M vdsm/storage/localFsSD.py
M vdsm/storage/mount.py
2 files changed, 32 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/10628/1

diff --git a/vdsm/storage/localFsSD.py b/vdsm/storage/localFsSD.py
index 74becea..dc9e727 100644
--- a/vdsm/storage/localFsSD.py
+++ b/vdsm/storage/localFsSD.py
@@ -26,6 +26,7 @@
 import fileUtils
 import storage_exception as se
 import misc
+import mount
 
 
 class LocalFsStorageDomain(fileSD.FileStorageDomain):
@@ -106,22 +107,23 @@
 def isLocalFsDomain(domainPath):
     """This ancillary function differentiates local and mounted SD's.
 
-    Assumed that a local SD can't be mounted.
-    mounted: nfs, posixFS
-    local: a link to a local dir in the /rhev/datacenter
+    It does this by checking if the mount path of "domainPath" is different
+    from the mount path of it's parent directory or the /ovirt/datacenter/mnt
+    directory. Because localFS domains a symlinks they will have the same mount
+    path.
+
+    This method has an inherent race that if the mounted domain has been
+    unmounted between getting the domainPath and actually checking for locality
+    it will be reported as local. Since the domain doesn't exist anyway I
+    choose to ignore this.
     """
-    mtab = open("/etc/mtab", "r").readlines()
-    for mEntry in mtab:
-        mPoint = mEntry.split()[1]
-        if mPoint == '/':
-            continue
-        elif mPoint in domainPath:
-            # This is a mounted domain therefore not exists as a local SD.
-            isLocal = False
-            break
-    else:
-        isLocal = True
-    return isLocal
+
+    mountDir = os.path.join(domainPath, "..")
+    mountDir = os.path.normpath(mountDir)
+
+    mountDirMnt = mount.findMountOfPath(mountDir, False)
+    domMnt = mount.findMountOfPath(domainPath, False)
+    return (domMnt.fs_file != mountDirMnt.fs_file)
 
 
 def findDomain(sdUUID):
diff --git a/vdsm/storage/mount.py b/vdsm/storage/mount.py
index 81ca17f..d643528 100644
--- a/vdsm/storage/mount.py
+++ b/vdsm/storage/mount.py
@@ -154,10 +154,22 @@
     raise OSError(errno.ENOENT, 'Mount target %s not found' % target)
 
 
-def findMountOfPath(path):
+def findMountOfPath(path, followSymlinks=True):
+    """ Finds the mount associated with a path.
+
+    :param path: path to a node on the filesystem
+    :param followSymlinks: The functions will not resolve symlinks. This means
+                           that the function can be used in process without
+                           fear of any IO getting stuck. Use this for cases
+                           where you know that the target is not a symlink or
+                           when it isn't  important if it is.
+
+    :returns: A mount object representing the mount associated with the file.
+    """
     # TBD: Bind mounts, should we ignore them?
-    # Follow symlinks (if file exists)
-    path = os.path.realpath(path)
+    if followSymlinks:
+        # Follow symlinks (if file exists)
+        path = os.path.realpath(path)
 
     # Make sure using canonical representation
     path = os.path.normpath(path)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I415e3ed87d1c2f9e0f67d90c5bcd8734e70e53c3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi at redhat.com>


More information about the vdsm-patches mailing list