Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/28473
to review the following change.
Change subject: Setting enum for isconfigured return value and change override
semantics
......................................................................
Setting enum for isconfigured return value and change override semantics
isconfigured now returns CONFIGURED, NOT_CONFIGURED and NOT_SURE. This to
ease and clarify the override semantics.
If module returns CONFIGURED it means that also on force vdsm won't need to
reconfigure it, as the module configured properly. Although if configured
returns NOT_SURE, it means, vdsm already configured the module, but on
force we desire to override those configuration to defaults.
Change-Id: I58b2eef787a90073a06caf88b6847f34fbd042ed
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1088805
Signed-off-by: Yaniv Bronhaim <[email protected]>
Reviewed-on: http://gerrit.ovirt.org/28007
Reviewed-by: Dan Kenigsberg <[email protected]>
---
M lib/vdsm/tool/configurator.py
1 file changed, 57 insertions(+), 47 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/73/28473/1
diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index 37fd416..bb1999b 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -21,13 +21,26 @@
import sys
import grp
import argparse
+import pwd
from .. import utils
from . import service, expose
from ..constants import P_VDSM_EXEC, QEMU_PROCESS_GROUP, VDSM_GROUP
+# Declare state of configuration
+#
+# CONFIGURED = Module is set properly without any required changes on
+# force.
+# NOT_CONFIGURED = Module is not set properly for VDSM and need to be
+# configured.
+# NOT_SURE = VDSM configured module already but on force configure vdsm
+# will set configurations to defaults parameters.
+#
+CONFIGURED, NOT_CONFIGURED, NOT_SURE = range(3)
+
class _ModuleConfigure(object):
+
def __init__(self):
pass
@@ -44,10 +57,7 @@
pass
def isconfigured(self):
- return True
-
- def reconfigureOnForce(self):
- return True
+ return NOT_CONFIGURED
class LibvirtModuleConfigure(_ModuleConfigure):
@@ -101,9 +111,9 @@
"""
try:
self._exec_libvirt_configure("check_if_configured")
- return True
+ return NOT_SURE
except RuntimeError:
- return False
+ return NOT_CONFIGURED
class SanlockModuleConfigure(_ModuleConfigure):
@@ -146,47 +156,47 @@
True if sanlock service is configured, False if sanlock service
requires a restart to reload the relevant supplementary groups.
"""
- configured = False
- try:
- with open("/var/run/sanlock/sanlock.pid", "r") as f:
- sanlock_pid = f.readline().strip()
- with open(os.path.join('/proc', sanlock_pid, 'status'),
- "r") as sanlock_status:
- proc_status_group_prefix = "Groups:\t"
- for status_line in sanlock_status:
- if status_line.startswith(proc_status_group_prefix):
- groups = [int(x) for x in
- status_line[len(proc_status_group_prefix):].
- strip().split(" ")]
- break
+ configured = NOT_CONFIGURED
+ groups = [g.gr_name for g in grp.getgrall()
+ if 'sanlock' in g.gr_mem]
+ gid = pwd.getpwnam('sanlock').pw_gid
+ groups.append(grp.getgrgid(gid).gr_name)
+ if all(group in self.SANLOCK_GROUPS for group in groups):
+ configured = NOT_SURE
+
+ if configured == NOT_SURE:
+ try:
+ with open("/var/run/sanlock/sanlock.pid", "r") as f:
+ sanlock_pid = f.readline().strip()
+ with open(os.path.join('/proc', sanlock_pid, 'status'),
+ "r") as sanlock_status:
+ proc_status_group_prefix = "Groups:\t"
+ for status_line in sanlock_status:
+ if status_line.startswith(proc_status_group_prefix):
+ groups = [int(x) for x in
+ status_line[
+ len(proc_status_group_prefix):].
+ strip().split(" ")]
+ break
+ else:
+ raise RuntimeError(
+ "Unable to find sanlock service groups"
+ )
+
+ is_sanlock_groups_set = True
+ for g in self.SANLOCK_GROUPS:
+ if grp.getgrnam(g)[2] not in groups:
+ is_sanlock_groups_set = False
+ if is_sanlock_groups_set:
+ configured = CONFIGURED
+
+ except IOError as e:
+ if e.errno == os.errno.ENOENT:
+ configured = CONFIGURED
else:
- raise RuntimeError("Unable to find sanlock service groups")
-
- is_sanlock_groups_set = True
- for g in self.SANLOCK_GROUPS:
- if grp.getgrnam(g)[2] not in groups:
- is_sanlock_groups_set = False
- configured = is_sanlock_groups_set
-
- except IOError as e:
- if e.errno == os.errno.ENOENT:
- sys.stdout.write("sanlock service is not running\n")
- configured = True
- else:
- raise
-
- if not configured:
- sys.stdout.write("sanlock service requires restart\n")
- else:
- sys.stdout.write("sanlock service is already configured\n")
+ raise
return configured
-
- def reconfigureOnForce(self):
- # If sanlock is down isconfigure returns True and configure will skip
- # sanlock configure. on force users expected to run configure even if
- # isconfigure returned True.
- return True
__configurers = (
@@ -206,12 +216,12 @@
sys.stdout.write("\nChecking configuration status...\n\n")
for c in __configurers:
if c.getName() in args.modules:
- override = args.force and c.reconfigureOnForce()
+ override = args.force and (c.isconfigured != CONFIGURED)
if not override and not c.validate():
raise RuntimeError(
"Configuration of %s is invalid" % c.getName()
)
- if override or not c.isconfigured():
+ if override:
configurer_to_trigger.append(c)
services = []
@@ -248,7 +258,7 @@
m = [
c.getName() for c in __configurers
- if c.getName() in args.modules and not c.isconfigured()
+ if c.getName() in args.modules and c.isconfigured() == NOT_CONFIGURED
]
if m:
--
To view, visit http://gerrit.ovirt.org/28473
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I58b2eef787a90073a06caf88b6847f34fbd042ed
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
