Change in vdsm[master]: Adding remove\disable verbs to vdsm-tool for admin usages

mtayer at redhat.com mtayer at redhat.com
Mon Mar 3 12:50:18 UTC 2014


mooli tayer has posted comments on this change.

Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
......................................................................


Patch Set 3:

(7 comments)

Also rebasing on top of new tests:
http://gerrit.ovirt.org/#/c/25263/

http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 139:         except RuntimeError:
Line 140:             return False
Line 141: 
Line 142:     def removeConf(self):
Line 143:         vdsm_ver = '-4.9.10'
This patch fails due to this line. Where from do I get the current version?

Yaniv?
Line 144:         conf_prefix = '## beginning of configuration section by vdsm' + \
Line 145:                       vdsm_ver
Line 146:         conf_suffix = '## end of configuration section by vdsm' + vdsm_ver
Line 147: 


Line 148:         for path in [
Line 149:             P_VDSM_LCONF,
Line 150:             P_VDSM_QCONF,
Line 151:             P_VDSM_LDCONF,
Line 152:         ]:
> tuple and not list will be better.
Done
Line 153:             if os.path.exists(path):
Line 154:                 removeSectionFromFile(path, conf_prefix, conf_suffix)
Line 155:         utils.rmFile(P_SYSTEMCTL_CONF)
Line 156: 


Line 151:             P_VDSM_LDCONF,
Line 152:         ]:
Line 153:             if os.path.exists(path):
Line 154:                 removeSectionFromFile(path, conf_prefix, conf_suffix)
Line 155:         utils.rmFile(P_SYSTEMCTL_CONF)
> yes, we need to split and arrange - sysctl conf to SystemctlModuleConfigure
So for now I leave this as is
Line 156: 
Line 157: 
Line 158: class SanlockModuleConfigure(_ModuleConfigure):
Line 159: 


Line 168:     def getServices(self):
Line 169:         return ['sanlock']
Line 170: 
Line 171:     def removeConf(self):
Line 172:         pass
> no need
Done
Line 173: 
Line 174:     def configure(self):
Line 175:         """
Line 176:         Configure sanlock process groups


Line 360:                 c.removeConf()
Line 361:             except:
Line 362:                 failed.append(c.getName)
Line 363:     if failed:
Line 364:         sys.stdout.write(
> stderr?
Done.

We have the same problem in other modules.
Fix in a different patch set?
Line 365:             "Could not remove configuration for modules %s\n" % ','.join(failed),
Line 366:         )
Line 367:         raise RuntimeError("Remove configuration failed")
Line 368: 


http://gerrit.ovirt.org/#/c/21772/3/lib/vdsm/tool/passwd.py
File lib/vdsm/tool/passwd.py:

Line 50: def remove_saslpasswd():
Line 51:     """
Line 52:     Remove vdsm password for libvirt connection
Line 53:     """
Line 54:     p = subprocess.Popen(('/usr/sbin/saslpasswd2', '-p', '-a', 'libvirt', '-d', constants.SASL_USERNAME))
> should be in P_EXEC_XXXX
Are you saying /usr/sbin/saslpasswd2 should be in constants.py?
Line 55:     output, err = p.communicate()
Line 56:     if p.returncode != 0:


Line 52:     Remove vdsm password for libvirt connection
Line 53:     """
Line 54:     p = subprocess.Popen(('/usr/sbin/saslpasswd2', '-p', '-a', 'libvirt', '-d', constants.SASL_USERNAME))
Line 55:     output, err = p.communicate()
Line 56:     if p.returncode != 0:
> please use vdsm utilities for process execution, I guess there are.
Yaniv?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list