[NEW PATCH] BZ#726105 - Check that qemu can also access file mounts (via gerrit-bot)

Saggi Mizrahi smizrahi at redhat.com
Thu Aug 18 13:17:34 UTC 2011


New patch submitted by Saggi Mizrahi (smizrahi at redhat.com)

You can review this change at: http://gerrit.usersys.redhat.com/776

commit 76a6d82170521664ff5f3e2e9dc8a5357d50c1d6
Author: Saggi Mizrahi <smizrahi at redhat.com>
Date:   Mon Aug 1 14:56:03 2011 +0300

    BZ#726105 - Check that qemu can also access file mounts
    
    v3:
    - use both qemu and kvm groups when testing access to directory
    v4:
    - add OOP handling. I hate NFS.
    
    Change-Id: I9367410e0172569b4ccb83d5cea1064771f66bb7

diff --git a/vdsm/storage/fileUtils.py b/vdsm/storage/fileUtils.py
index 1c63078..5496949 100644
--- a/vdsm/storage/fileUtils.py
+++ b/vdsm/storage/fileUtils.py
@@ -153,11 +153,11 @@ def transformPath(remotePath):
     """
     return remotePath.replace('_','__').replace('/','_')
 
-def validateAccess(targetPath):
+def validateAccess(targetPath, perms=(os.R_OK | os.W_OK | os.X_OK)):
     """
     Validate the RWX access to a given path
     """
-    if not os.access(targetPath, os.R_OK | os.W_OK | os.X_OK):
+    if not os.access(targetPath, perms):
         raise se.StorageServerAccessPermissionError(targetPath)
 
 def validateQemuReadable(targetPath):
@@ -227,20 +227,27 @@ def createdir(dirPath, mode=None):
         else:
             os.makedirs(dirPath)
 
-def chown(path, user=-1, group=-1):
-    """
-    Change the owner and\or group of a file.
-    The user and group parameters can either be a name or an id.
-    """
+def resolveUid(user):
     if isinstance(user, types.StringTypes):
         uid = pwd.getpwnam(user).pw_uid
     else:
         uid = int(user)
+    return uid
 
+def resolveGid(group):
     if isinstance(group, types.StringTypes):
         gid = grp.getgrnam(group).gr_gid
     else:
         gid = int(group)
+    return gid
+
+def chown(path, user=-1, group=-1):
+    """
+    Change the owner and\or group of a file.
+    The user and group parameters can either be a name or an id.
+    """
+    uid = resolveUid(user)
+    gid = resolveGid(group)
 
     stat = os.stat(path)
     currentUid = stat.st_uid
diff --git a/vdsm/storage/localFsSD.py b/vdsm/storage/localFsSD.py
index 012f304..5ff9d5f 100644
--- a/vdsm/storage/localFsSD.py
+++ b/vdsm/storage/localFsSD.py
@@ -15,6 +15,7 @@ import sd
 import fileSD
 import fileUtils
 import storage_exception as se
+from storage_connection import validateDirAccess
 
 
 class LocalFsStorageDomain(fileSD.FileStorageDomain):
@@ -25,7 +26,7 @@ class LocalFsStorageDomain(fileSD.FileStorageDomain):
         if os.path.abspath(typeSpecificArg) != typeSpecificArg:
             raise se.StorageDomainIllegalRemotePath(typeSpecificArg)
 
-        fileUtils.validateAccess(domPath)
+        validateDirAccess(domPath)
 
         sd.validateDomainVersion(version)
 
diff --git a/vdsm/storage/nfsSD.py b/vdsm/storage/nfsSD.py
index abcf1ff..ffd4b6d 100644
--- a/vdsm/storage/nfsSD.py
+++ b/vdsm/storage/nfsSD.py
@@ -16,6 +16,7 @@ from sd import processPoolDict
 import fileSD
 import fileUtils
 import storage_exception as se
+from storage_connection import validateDirAccess
 
 class NfsStorageDomain(fileSD.FileStorageDomain):
 
@@ -31,7 +32,7 @@ class NfsStorageDomain(fileSD.FileStorageDomain):
         if not fileUtils.isMounted(mountPoint=domPath, mountType=fileUtils.FSTYPE_NFS):
             raise se.StorageDomainFSNotMounted(typeSpecificArg)
 
-        processPoolDict[sdUUID].fileUtils.validateAccess(domPath)
+        validateDirAccess(domPath)
 
         # Make sure there are no remnants of other domain
         mdpat = os.path.join(domPath, "*", sd.DOMAIN_META_DATA)
diff --git a/vdsm/storage/storage_connection.py b/vdsm/storage/storage_connection.py
index d5ebc21..6cf47b8 100644
--- a/vdsm/storage/storage_connection.py
+++ b/vdsm/storage/storage_connection.py
@@ -18,6 +18,16 @@ import sd
 import storage_exception as se
 import outOfProcess as oop
 from processPool import Timeout
+import supervdsm
+import constants
+
+def validateDirAccess(dirPath):
+    oop.fileUtils.validateAccess(dirPath)
+    supervdsm.getProxy().validateAccess(constants.QEMU_PROCESS_USER,
+            (constants.DISKIMAGE_GROUP, constants.METADATA_GROUP), dirPath,
+            (os.R_OK | os.X_OK))
+    oop.fileUtils.validatePermissions(dirPath)
+
 
 class StorageServerConnection:
     log = logging.getLogger('Storage.ServerConnection')
@@ -156,7 +166,7 @@ class StorageServerConnection:
                 rc = oop.fileUtils.mount(con['rp'], mntPath, fsType)
                 if rc == 0:
                     try:
-                        oop.fileUtils.validateAccess(mntPath)
+                        validateDirAccess(mntPath)
                     except se.StorageServerAccessPermissionError, ex:
                         self.log.debug("Unmounting file system %s "
                             "(not enough access permissions)" % con['rp'])
@@ -248,7 +258,7 @@ class StorageServerConnection:
                     rc = oop.fileUtils.mount(con['rp'], mountpoint, fsType)
                     if rc == 0:
                         try:
-                            oop.fileUtils.validateAccess(mountpoint)
+                            validateDirAccess(mountpoint)
                         except se.StorageServerAccessPermissionError, ex:
                             rc = ex.code
                     else:
@@ -280,8 +290,7 @@ class StorageServerConnection:
             rc = 0
             try:
                 if os.path.exists(con['rp']):
-                    fileUtils.validateAccess(con['rp'])
-                    fileUtils.validatePermissions(con['rp'])
+                    validateDirAccess(con['rp'])
                 else:
                     self.log.error("Path %s does not exists.", con['rp'])
                     rc = se.StorageServerValidationError.code
diff --git a/vdsm/supervdsmServer.py b/vdsm/supervdsmServer.py
index 1f5643e..0ea59be 100755
--- a/vdsm/supervdsmServer.py
+++ b/vdsm/supervdsmServer.py
@@ -7,15 +7,20 @@ import errno
 import threading
 from time import sleep
 import signal
+from multiprocessing import Pipe, Process
 
 from storage.multipath import getScsiSerial as _getScsiSerial
 from storage.iscsi import forceIScsiScan as _forceIScsiScan
 from supervdsm import _SuperVdsmManager, PIDFILE, ADDRESS
-from storage.fileUtils import chown, open_ex
+from storage.fileUtils import chown, open_ex, resolveGid, resolveUid
+from storage.fileUtils import validateAccess as _validateAccess
 from constants import METADATA_GROUP, METADATA_USER
 from storage.devicemapper import _removeMapping, _getPathsStatus
 import configNetwork
+from config import config
 
+RUN_AS_TIMEOUT= config.getint("irs", "process_pool_timeout")
+class Timeout(RuntimeError): pass
 
 def logDecorator(func):
     callbackLogger = logging.getLogger("SuperVdsm.ServerCallback")
@@ -71,6 +76,51 @@ class _SuperVdsm(object):
     def setupNetworks(self, networks={}, bondings={}, options={}):
         return configNetwork.setupNetworks(networks, bondings, **options)
 
+    def _runAs(self, user, groups, func, args=(), kwargs={}):
+        def child(pipe):
+            res = ex = None
+            try:
+                uid = resolveUid(user)
+                if groups:
+                     gids = map(resolveGid, groups)
+
+                     os.setgid(gids[0])
+                     os.setgroups(gids)
+                os.setuid(uid)
+
+                res = func(*args, **kwargs)
+            except BaseException, e:
+                ex = e
+
+            pipe.send((res, ex))
+            pipe.recv()
+
+        pipe, hisPipe = Pipe()
+        proc = Process(target=child, args=(hisPipe,))
+        proc.start()
+
+        if not pipe.poll(RUN_AS_TIMEOUT):
+            try:
+                os.kill(proc.pid, errno.SIGKILL)
+            except OSError, e:
+                # If it didn't fail because process is already dead
+                if e.errno != errno.ESRCH:
+                    raise
+
+            raise Timeout()
+
+        res, err = pipe.recv()
+        pipe.send("Bye")
+        proc.terminate()
+        if err is not None:
+            raise err
+
+        return res
+
+    @logDecorator
+    def validateAccess(self, user, groups, *args, **kwargs):
+        return self._runAs(user, groups, _validateAccess, args=args, kwargs=kwargs)
+
     @logDecorator
     def setSafeNetworkConfig(self):
         return configNetwork.setSafeNetworkConfig()




More information about the vdsm-patches mailing list