Change in vdsm[master]: replace configure_libvirt.py with python code.
ybronhei at redhat.com
ybronhei at redhat.com
Mon May 5 13:05:07 UTC 2014
Yaniv Bronhaim has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
......................................................................
Patch Set 3:
(8 comments)
love it ! thanks!! just few comments. please have more reviews now. i think its almost fully ready
http://gerrit.ovirt.org/#/c/27298/3/lib/vdsm/tool/Makefile.am
File lib/vdsm/tool/Makefile.am:
Line 31
Line 32
Line 33
Line 34
Line 35
remove all section
http://gerrit.ovirt.org/#/c/27298/3/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 176: VMCONF
leave it VDSMCONF
Line 195: raise RuntimeError(
Line 196: "vdsm: Missing certificate, vdsm not registered")
Line 197: validate_ovirt_certs.validate_ovirt_certs()
Line 198: # Remove a previous configuration (if present)
Line 199: self.removeConf()
for backward compatibility .. but im not sure we want to remove the conf if exist... maybe on force
Line 200: lconf_maps = [self.LCONF_GENERAL]
Line 201: qconf_maps = [self.QCONF_GENERAL]
Line 202: ldconf_maps = [self.LDCONF_GENERAL]
Line 203: qlconf_maps = []
Line 209: [self.CA_FILE, self.CERT_FILE, self.KEY_FILE]):
Line 210: lconf_maps.append(self.LCONF_SSL)
Line 211: qconf_maps.append(self.QCONF_SSL_CERTS)
Line 212: else:
Line 213: lconf_maps.append(self.LCONF_NO_SSL)
this will introduce conflict ^, you must have the certs if ssl=True, no ?
Line 214: else:
Line 215: qconf_maps.append(self.QCONF_NO_SSL)
Line 216: lconf_maps.append(self.LCONF_NO_SSL)
Line 217: if SANLOCK_ENABLED:
Line 237:
Line 238: with ConfigFile(
Line 239: self.envGet('LRCONF'), **VDSM_CONF_SECTION) as conf:
Line 240: conf.prefixLines('# VDSM backup')
Line 241: conf.prependSection(self.LLOGR_CONF)
maybe pass boolean if backup is required or not. it can lead to big conf files and might cause a regression
Line 242:
Line 243: for fname in (
Line 244: self.envGet('LCONF'),
Line 245: self.envGet('QCONF'),
Line 247: self.envGet('QLCONF'),
Line 248: self.envGet('LRCONF')
Line 249: ):
Line 250: if utils.isOvirtNode() and ovirtfunctions:
Line 251: ovirtfunctions.ovirt_store_config(fname)
put the condition before the for
Line 252: sys.stdout.write("Reconfiguration of libvirt is done.")
Line 253:
Line 254: def sysvToUpstart(self):
Line 255: """
Line 258: crashed.
Line 259: """
Line 260: INITCTL = '/sbin/initctl'
Line 261: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 262: TARGET = "/etc/init/libvirtd.conf"
private. add underscore ^
Line 263: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
Line 264: ts = rpm.TransactionSet()
Line 265: mi = itertools.chain(*[ts.dbMatch('name', name)
Line 266: for name in ['libvirt', 'libvirt-daemon']])
Line 296: 'listen_tcp': '0',
Line 297: 'auth_tcp': 'none'
Line 298: })
Line 299: lconf_p.read(self.envGet('LCONF'))
Line 300: listen_tcp = lconf_p.getboolean('listen_tcp')
is getboolean really needed? you check if its 1 or 0
Line 301: auth_tcp = lconf_p.get('auth_tcp')
Line 302: qconf_p = ParserWrapper({'spice_tls': '0'})
Line 303: qconf_p.read(self.envGet('QCONF'))
Line 304: spice_tls = qconf_p.getboolean('spice_tls')
--
To view, visit http://gerrit.ovirt.org/27298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74bfe05bb4b5f5d09021f21b324f9b7d5d0fdaab
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list