Change in vdsm[ovirt-3.5]: vdsm-tool: changing root checking in configurator.

mtayer at redhat.com mtayer at redhat.com
Wed Sep 3 14:33:58 UTC 2014


Hello Yaniv Bronhaim, Dima Kuznetsov, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/32408

to review the following change.

Change subject: vdsm-tool: changing root checking in configurator.
......................................................................

vdsm-tool: changing root checking in configurator.

Checking for root as we currently do in configure of libvirt and
sanlock is not enough. Currently this will fail during the
isConfigured() check since it does not have premissions to the
files it attempts to check:
"OSError: No such file or directory: /etc/libvirt/libvirtd.conf"

It is easy to see that this check is needed for all the exposed
methods of configurator. I'm suggesting to do it in the exposed
verbs in a uniform manner.

3.5 note: This commit broke master and it must be merged along
with change I8e86dd38e3a871d55ffc5cb67d5f4596ec4dc3a6.

Change-Id: I52967e30f677e4537b83c2db442963e3eadecb55
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1136897
Signed-off-by: Mooli Tayer <mtayer at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31293
Reviewed-by: Dima Kuznetsov <dkuznets at redhat.com>
Reviewed-by: Yaniv Bronhaim <ybronhei at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M lib/vdsm/tool/__init__.py
M lib/vdsm/tool/configurator.py
M lib/vdsm/tool/configurators/libvirt.py
M lib/vdsm/tool/configurators/sanlock.py
4 files changed, 20 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/32408/1

diff --git a/lib/vdsm/tool/__init__.py b/lib/vdsm/tool/__init__.py
index 008e41f..b9dd9ac 100644
--- a/lib/vdsm/tool/__init__.py
+++ b/lib/vdsm/tool/__init__.py
@@ -16,6 +16,8 @@
 #
 # Refer to the README and COPYING files for full details of the license
 #
+import functools
+import os
 
 
 class expose(object):
@@ -27,6 +29,15 @@
         return fun
 
 
+def requiresRoot(func):
+    @functools.wraps(func)
+    def wrapper(*args, **kwargs):
+        if os.getuid() != 0:
+            raise NotRootError()
+        func(*args, **kwargs)
+    return wrapper
+
+
 class UsageError(RuntimeError):
     """ Raise on runtime when usage is invalid """
 
diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index 7d3752e..9160d75 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -22,7 +22,11 @@
 import sys
 import traceback
 
-from . import service, expose, UsageError
+from . import \
+    service, \
+    expose, \
+    UsageError, \
+    requiresRoot
 from .configurators import \
     certificates, \
     CONFIGURED, \
@@ -41,6 +45,7 @@
 
 
 @expose("configure")
+ at requiresRoot
 def configure(*args):
     """
     configure [-h|...]
@@ -87,6 +92,7 @@
 
 
 @expose("is-configured")
+ at requiresRoot
 def isconfigured(*args):
     """
     is-configured [-h|...]
@@ -122,6 +128,7 @@
 
 
 @expose("validate-config")
+ at requiresRoot
 def validate_config(*args):
     """
     validate-config [-h|...]
@@ -144,6 +151,7 @@
 
 
 @expose("remove-config")
+ at requiresRoot
 def remove_config(*args):
     """
     Remove vdsm configuration from conf files
diff --git a/lib/vdsm/tool/configurators/libvirt.py b/lib/vdsm/tool/configurators/libvirt.py
index 86f24e4..0f88b32 100644
--- a/lib/vdsm/tool/configurators/libvirt.py
+++ b/lib/vdsm/tool/configurators/libvirt.py
@@ -61,9 +61,6 @@
         return ["vdsmd", "supervdsmd", "libvirtd"]
 
     def configure(self):
-        if os.getuid() != 0:
-            raise NotRootError()
-
         self._sysvToUpstart()
 
         if utils.isOvirtNode():
diff --git a/lib/vdsm/tool/configurators/sanlock.py b/lib/vdsm/tool/configurators/sanlock.py
index 9225252..0c847a3 100644
--- a/lib/vdsm/tool/configurators/sanlock.py
+++ b/lib/vdsm/tool/configurators/sanlock.py
@@ -46,9 +46,6 @@
         """
         Configure sanlock process groups
         """
-        if os.getuid() != 0:
-            raise NotRootError()
-
         rc, out, err = utils.execCmd(
             (
                 '/usr/sbin/usermod',


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I52967e30f677e4537b83c2db442963e3eadecb55
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>


More information about the vdsm-patches mailing list