Change in vdsm[ovirt-3.4]: Validate conf in configure only if not forcing overriding

ybronhei at redhat.com ybronhei at redhat.com
Sun Feb 16 08:14:20 UTC 2014


Hello Dan Kenigsberg,

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

    http://gerrit.ovirt.org/24501

to review the following change.

Change subject: Validate conf in configure only if not forcing overriding
......................................................................

Validate conf in configure only if not forcing overriding

When --force flag is sent to configure the conf might be overrided
(depends on reconfigureOnForce return value).
Therefore, checking validate conf before overriding can lead to a fail
while the current configuration is not relevant.

libvirt_configure.sh already manage to set the right configure in
libvirtd.conf and qemu.conf according to the ssl verb in vdsm.conf.
BUT, without this patch we validate the current config
state before overriding the files even though the --force flag was
stated, and this lead to fail the configure.

With that patch, setting in vdsm.conf ssl=True and then running
"vdsm-tool configure --force" will update libvirtd.conf and qemu.conf
according to vdsm.conf state.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1036883
Change-Id: Ibd9e95c06b1437074f37248e77bb086a3c3bad63
Signed-off-by: Yaniv Bronhaim <ybronhei at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/24132
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M lib/vdsm/tool/configurator.py
1 file changed, 3 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/01/24501/1

diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index 29172e4..b885d81 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -198,11 +198,12 @@
     sys.stdout.write("\nChecking configuration status...\n\n")
     for c in __configurers:
         if c.getName() in args.modules:
-            if not c.validate():
+            override = args.force and c.reconfigureOnForce()
+            if not override and not c.validate():
                 raise RuntimeError(
                     "Configuration of %s is invalid" % c.getName()
                 )
-            if (args.force and c.reconfigureOnForce()) or not c.isconfigured():
+            if override or not c.isconfigured():
                 configurer_to_trigger.append(c)
 
     services = []


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibd9e95c06b1437074f37248e77bb086a3c3bad63
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list