Change in vdsm[master]: mount: Perform mount and umount in supervdsm

Friday, 24 July 2015

Nir Soffer has uploaded a new change for review.

Change subject: mount: Perform mount and umount in supervdsm
......................................................................

mount: Perform mount and umount in supervdsm

We used to run the mount and umount commands using sudo, which requires
error prone sudoers configuration, and allows any process running as
vdsm (e.g. a vdsm hook) to perform commands, instead of only vdsmd
daemon itself.

This patch changes Mount class to perform mount and umount using
supervdsm, and remove the now unneeded sudoers configuration for mount,
umount, and systemd-run.

Change-Id: I38fb0eed0ba3e2c36aba8ca4ec262032cb012fc2
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
M tests/mountTests.py
M vdsm/storage/mount.py
M vdsm/sudoers.vdsm.in
M vdsm/supervdsmServer
4 files changed, 93 insertions(+), 46 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/43969/1

diff --git a/tests/mountTests.py b/tests/mountTests.py
index 16a98b9..b7b0c34 100644
--- a/tests/mountTests.py
+++ b/tests/mountTests.py
@@ -62,7 +62,21 @@
         os.unlink(path)
 
 
+class FakeSupervdsm(object):
+
+    def getProxy(self):
+        return self
+
+    def mount(self, *a, **kw):
+        return mount._mount(*a, **kw)
+
+    def umount(self, *a, **kw):
+        return mount._umount(*a, **kw)
+
+
 class MountTests(TestCaseBase):
+
+    @monkeypatch.MonkeyPatch(mount, "supervdsm", FakeSupervdsm())
     def testLoopMount(self):
         checkSudo(["mount", "-o", "loop",
"somefile", "target"])
         checkSudo(["umount", "target"])
@@ -76,6 +90,7 @@
                 finally:
                     m.umount()
 
+    @monkeypatch.MonkeyPatch(mount, "supervdsm", FakeSupervdsm())
     def testSymlinkMount(self):
         checkSudo(["mount", "-o", "loop",
"somefile", "target"])
         checkSudo(["umount", "target"])
diff --git a/vdsm/storage/mount.py b/vdsm/storage/mount.py
index 4786ccb..e579450 100644
--- a/vdsm/storage/mount.py
+++ b/vdsm/storage/mount.py
@@ -27,6 +27,8 @@
 
 from vdsm import cmdutils
 from vdsm import constants
+
+import supervdsm
 import misc
 
 # Common vfs types
@@ -214,51 +216,14 @@
         return hsh
 
     def mount(self, mntOpts=None, vfstype=None, timeout=None, cgroup=None):
-        cmd = [constants.EXT_MOUNT]
-
-        if vfstype is not None:
-            cmd.extend(("-t", vfstype))
-
-        if mntOpts:
-            cmd.extend(("-o", mntOpts))
-
-        cmd.extend((self.fs_spec, self.fs_file))
-
-        if cgroup:
-            cmd = cmdutils.systemd_run(cmd, scope=True, slice=cgroup)
-
-        return self._runcmd(cmd, timeout)
-
-    def _runcmd(self, cmd, timeout):
-        isRoot = os.geteuid() == 0
-        p = misc.execCmd(cmd, sudo=not isRoot, sync=False)
-        if not p.wait(timeout):
-            p.kill()
-            raise OSError(errno.ETIMEDOUT,
-                          "%s operation timed out" % os.path.basename(cmd[0]))
-
-        out, err = p.communicate()
-        rc = p.returncode
-
-        if rc == 0:
-            return
-
-        raise MountError(rc, ";".join((out, err)))
+        p = supervdsm.getProxy()
+        return p.mount(self.fs_spec, self.fs_file, mntOpts=mntOpts,
+                       vfstype=vfstype, timeout=timeout, cgroup=cgroup)
 
     def umount(self, force=False, lazy=False, freeloop=False, timeout=None):
-        cmd = [constants.EXT_UMOUNT]
-        if force:
-            cmd.append("-f")
-
-        if lazy:
-            cmd.append("-l")
-
-        if freeloop:
-            cmd.append("-d")
-
-        cmd.append(self.fs_file)
-
-        return self._runcmd(cmd, timeout)
+        p = supervdsm.getProxy()
+        return p.umount(self.fs_file, force=force, lazy=lazy,
+                        freeloop=freeloop, timeout=timeout)
 
     def isMounted(self):
         try:
@@ -287,3 +252,58 @@
     def __repr__(self):
         return ("<Mount fs_spec='%s' fs_file='%s'>" %
                 (self.fs_spec, self.fs_file))
+
+
+def _mount(fs_spec, fs_file, mntOpts=None, vfstype=None, timeout=None, cgroup=None):
+    """
+    Called from supervdsm for running the mount command as root.
+    """
+    cmd = [constants.EXT_MOUNT]
+
+    if vfstype is not None:
+        cmd.extend(("-t", vfstype))
+
+    if mntOpts:
+        cmd.extend(("-o", mntOpts))
+
+    cmd.extend((fs_spec, fs_file))
+
+    if cgroup:
+        cmd = cmdutils.systemd_run(cmd, scope=True, slice=cgroup)
+
+    return _runcmd(cmd, timeout)
+
+
+def _umount(fs_file, force=False, lazy=False, freeloop=False, timeout=None):
+    """
+    Called from supervdsm for running the umount command as root.
+    """
+    cmd = [constants.EXT_UMOUNT]
+    if force:
+        cmd.append("-f")
+
+    if lazy:
+        cmd.append("-l")
+
+    if freeloop:
+        cmd.append("-d")
+
+    cmd.append(fs_file)
+
+    return _runcmd(cmd, timeout)
+
+
+def _runcmd(cmd, timeout):
+    p = misc.execCmd(cmd, sync=False)
+    if not p.wait(timeout):
+        p.kill()
+        raise OSError(errno.ETIMEDOUT,
+                      "%s operation timed out" % os.path.basename(cmd[0]))
+
+    out, err = p.communicate()
+    rc = p.returncode
+
+    if rc == 0:
+        return
+
+    raise MountError(rc, ";".join((out, err)))
diff --git a/vdsm/sudoers.vdsm.in b/vdsm/sudoers.vdsm.in
index 924cc87..76b2386 100644
--- a/vdsm/sudoers.vdsm.in
+++ b/vdsm/sudoers.vdsm.in
@@ -1,7 +1,7 @@
 Cmnd_Alias VDSM_LIFECYCLE = \
     @DMIDECODE_PATH@, \
     @VDSMDIR@/mk_sysprep_floppy
-Cmnd_Alias VDSM_STORAGE = @MOUNT_PATH@, @UMOUNT_PATH@, \
+Cmnd_Alias VDSM_STORAGE = \
     @FSCK_PATH@ -p *, \
     @TUNE2FS_PATH@ -j *, \
     @MKFS_PATH@ -q -j *, \
@@ -27,8 +27,7 @@
     @MULTIPATH_PATH@, \
     @SETSID_PATH@ @IONICE_PATH@ -c ? -n ? @SU_PATH@ vdsm -s /bin/sh -c
/usr/libexec/vdsm/spmprotect.sh*, \
     @SERVICE_PATH@ vdsmd *, \
-    @REBOOT_PATH@ -f, \
-    @SYSTEMD_RUN_PATH@ --scope --slice=vdsm-glusterfs @MOUNT_PATH@ *
+    @REBOOT_PATH@ -f
 
 vdsm  ALL=(ALL) NOPASSWD: VDSM_LIFECYCLE, VDSM_STORAGE
 Defaults:vdsm !requiretty
diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer
index 119c2ba..4b728aa 100755
--- a/vdsm/supervdsmServer
+++ b/vdsm/supervdsmServer
@@ -70,6 +70,7 @@
 from storage.iscsi import readSessionInfo as _readSessionInfo
 from supervdsm import _SuperVdsmManager
 from storage import hba
+from storage import mount
 from storage import multipath
 from storage.fileUtils import chown, resolveGid, resolveUid
 from storage.fileUtils import validateAccess as _validateAccess
@@ -150,6 +151,18 @@
         return _getScsiSerial(*args, **kwargs)
 
     @logDecorator
+    def mount(self, fs_spec, fs_file, mntOpts=None, vfstype=None, timeout=None,
+              cgroup=None):
+        return mount._mount(fs_spec, fs_file, mntOpts=mntOpts, vfstype=vfstype,
+                            timeout=timeout, cgroup=cgroup)
+
+    @logDecorator
+    def umount(self, fs_file, force=False, lazy=False, freeloop=False,
+               timeout=None):
+        return mount._umount(fs_file, force=force, lazy=lazy,
+                             freeloop=freeloop, timeout=timeout)
+
+    @logDecorator
     def resizeMap(self, devName):
         return multipath._resize_map(devName)
 


-- 
To view, visit https://gerrit.ovirt.org/43969
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I38fb0eed0ba3e2c36aba8ca4ec262032cb012fc2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;

    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011