Change in vdsm[master]: replace configure_libvirt.py with python code.
mtayer at redhat.com
mtayer at redhat.com
Thu May 8 11:25:32 UTC 2014
mooli tayer has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
......................................................................
Patch Set 4:
(6 comments)
http://gerrit.ovirt.org/#/c/27298/4/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 149: 'require_lease_for_disks': 0,
Line 150: }
Line 151:
Line 152: # libvirt log rotate configuration
Line 153: LLOGR_CONF = (
> why don't you embed this as file at /usr/share instead of hardcode?
A hint on how to do this?
Line 154: '/var/log/libvirt/libvirtd.log {'
Line 155: ' rotate 100'
Line 156: ' missingok'
Line 157: ' copytruncate'
Line 158: ' size 15M'
Line 159: ' compress'
Line 160: ' compresscmd /usr/bin/xz'
Line 161: ' uncompresscmd /usr/bin/unxz'
Line 162: ' compressext .xz'
> I hope we have xz dependency at spec...
Yes we do.
Line 163: '}'
Line 164: )
Line 165:
Line 166: def __init__(self):
Line 218: if SANLOCK_ENABLED:
Line 219: qconf_map.update(self.QCONF_SANLOCK)
Line 220: qlconf_map.update(self.QLCONF_SANLOCK)
Line 221: if not LIBVIRT_SELINUX:
Line 222: qconf_map.update(self.QCONF_NO_SELINUX)
> I do not see any reason to put one time used constants...
Ok I'm giving this a try.
Line 223:
Line 224: # write configuration
Line 225: for file_name, configuration in [
Line 226: (self.envGet('LCONF'), lconf_map),
Line 240: self.envGet('LRCONF'), **VDSM_CONF_SECTION) as conf:
Line 241: conf.prefixLines('# VDSM backup')
Line 242: conf.prependSection(self.LLOGR_CONF)
Line 243:
Line 244: if utils.isOvirtNode() and ovirtfunctions:
> it cannot be that it is node and functions are missing.
Done
Line 245: for fname in (
Line 246: self.envGet('LCONF'),
Line 247: self.envGet('QCONF'),
Line 248: self.envGet('LDCONF'),
Line 260: crashed.
Line 261: """
Line 262: INITCTL = '/sbin/initctl'
Line 263: LIBVIRTD_UPSTART = 'libvirtd.upstart'
Line 264: TARGET = "/etc/init/libvirtd.conf"
> etc should be sysconfdir from autoconf
Done
Line 265: if os.path.isfile(INITCTL) and os.access(INITCTL, os.X_OK):
Line 266: ts = rpm.TransactionSet()
Line 267: mi = itertools.chain(*[ts.dbMatch('name', name)
Line 268: for name in ['libvirt', 'libvirt-daemon']])
http://gerrit.ovirt.org/#/c/27298/4/tests/toolTests.py
File tests/toolTests.py:
Line 56: "keepalive_interval=-1\n"
Line 57: "log_outputs=\"1:file:/var/log/libvirt/libvirtd.log\"\n"
Line 58: "ca_file=\"/etc/pki/vdsm/certs/cacert.pem\"\n"
Line 59: "cert_file=\"/etc/pki/vdsm/certs/vdsmcert.pem\"\n"
Line 60: "## end of configuration section by vdsm-4.13.0\n"
> I would have put all these in separate files out of the script
Done
Line 61: ),
Line 62:
Line 63: "qconf_ssl": (
Line 64: "## beginning of configuration section by vdsm-4.13.0\n"
--
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: 4
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: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list