Change in vdsm[master]: replace configure_libvirt.py with python code.
smizrahi at redhat.com
smizrahi at redhat.com
Sun Jun 8 15:24:03 UTC 2014
Saggi Mizrahi has posted comments on this change.
Change subject: replace configure_libvirt.py with python code.
......................................................................
Patch Set 23:
(8 comments)
http://gerrit.ovirt.org/#/c/27298/23//COMMIT_MSG
Commit Message:
Line 6:
Line 7: replace configure_libvirt.py with python code.
Line 8:
Line 9: The port is to behave as similar as possible to the old
Line 10: sh code. python should allow easier testing (tests added
Seems like ) got moved around
Line 11: ) and better overall readability.
Line 12:
Line 13: A testing matrix [1] documents my tests of this feature.
Line 14:
http://gerrit.ovirt.org/#/c/27298/23/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 30: from .. import utils
Line 31: from . import service, expose, validate_ovirt_certs
Line 32: from . import NotRootError, UsageError
Line 33: from configfile import ConfigFile, ParserWrapper
Line 34: from ..constants import QEMU_PROCESS_GROUP, \
If you are already working on this line please change it to
from .. constants import \
QEMU_PROCESS_GROUP,
SANLOCK_USER,
.....
So that future diffs are easier to read.
Line 35: SANLOCK_USER, VDSM_GROUP, SYSCONF_PATH, P_VDSM_CERT, \
Line 36: SANLOCK_ENABLED, LIBVIRT_SELINUX, P_VDSM
Line 37: from vdsm.config import config
Line 38:
Line 35: SANLOCK_USER, VDSM_GROUP, SYSCONF_PATH, P_VDSM_CERT, \
Line 36: SANLOCK_ENABLED, LIBVIRT_SELINUX, P_VDSM
Line 37: from vdsm.config import config
Line 38:
Line 39: if utils.isOvirtNode:
Again, I think it's a function.
Line 40: from ovirt.node.utils.fs import Config
Line 41:
Line 42:
Line 43: class InvalidConfig(UsageError):
Line 117: # write configuration
Line 118: for cfile, content in self.FILES.items():
Line 119: content['configure'](self, content, vdsmConfiguration)
Line 120:
Line 121: sys.stdout.write("Reconfiguration of libvirt is done.")
end with \n?
Why write to stdout directly?
Line 122:
Line 123: def validate(self):
Line 124: """
Line 125: Validate conflict in configured files
Line 179: LIBVIRTD_UPSTART
os.path.basename(fname) == LIBVIRTD_UPSTART
is much more accurate representation of what you actually trying to do.
Line 183: if os.path.isfile(packaged):
Line 184: if not os.path.isfile(TARGET):
Line 185: service.service_stop('libvirtd')
Line 186: if (not os.path.isfile(TARGET) or
Line 187: not filecmp.cmp(packaged, TARGET)):
if not os.path.isfile(TARGET):
service.service_stop('libvirtd')
if not filecmp.cmp(packaged, TARGET):
....
Line 188: oldmod = None
Line 189: if os.path.isfile(TARGET):
Line 190: oldmod = os.stat(TARGET).st_mode
Line 191:
Line 197:
Line 198: if (oldmod is not None and
Line 199: oldmod != os.stat(TARGET).st_mode):
Line 200: os.chmod(TARGET, oldmod)
Line 201: rc, out, err = utils.execCmd((INITCTL,
Factor out to a proper function
Line 202: "reload-configuration"))
Line 203: if rc != 0:
Line 204: sys.stdout.write(out)
Line 205: sys.stderr.write(err)
Line 215: ssl = config.getboolean('vars', 'ssl')
Line 216:
Line 217: lconf_p = ParserWrapper({
Line 218: 'listen_tcp': '0',
Line 219: 'auth_tcp': 'sasl'
Add comma at EOL
Line 220: })
Line 221: lconf_p.read(self._getFile('LCONF'))
Line 222: listen_tcp = lconf_p.getint('listen_tcp')
Line 223: auth_tcp = lconf_p.get('auth_tcp')
--
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: 23
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: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.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