Yaniv Bronhaim has posted comments on this change.
Change subject: Adding remove\disable verbs to vdsm-tool for admin usages
......................................................................
Patch Set 2:
(3 comments)
I know we took over of existing patch, but this original implementation is wrong in my
opinion. there are much simpler ways to remove whatever vdsm configs. in gerenal this
patch should do the opposite\clean of vdsm-tool sebool-config, vdsm-tool set-saslpasswd,
vdsm-tool configure.
http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/constants.py.in
File lib/vdsm/constants.py.in:
Line 86: P_SYSTEMCTL_CONF = SYSCONF_PATH + '/sysctl.d/vdsm'
Line 87: P_VDSM_LCONF = SYSCONF_PATH + '/libvirt/libvirtd.conf'
Line 88: P_VDSM_LDCONF = SYSCONF_PATH + '/sysconfig/libvirtd'
Line 89: P_VDSM_QCONF = SYSCONF_PATH + '/libvirt/qemu.conf'
Line 90:
no need for that
Line 91: #
Line 92: # External programs (sorted, please keep in order).
Line 93: #
Line 94: EXT_BLKID = '@BLKID_PATH@'
http://gerrit.ovirt.org/#/c/21772/2/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 33: removeSectionFromFile
are we sure that its better than just executing ed command as we do in
libvirt_configure.sh?
ed -s "${confFile}" >/dev/null 2>&1 <<EOF
/${start_conf_section}/,/${end_conf_section}/d
wq
EOF
ed -s "${confFile}" >/dev/null 2>&1 <<EOF
g/${by_vdsm}/d
wq
maybe its better to expose remove_conf in libvirt_configure.sh and call that by
_exec_libvirt_configure
what do you think ?
anyway, we don't need such general function for that. only call to this
libvirt_configure remove_conf and that's all for this part of removal.
Line 147: conf_prefix = '## beginning of configuration section by vdsm' +
\
Line 148: vdsm_ver
Line 149: conf_suffix = '## end of configuration section by vdsm' +
vdsm_ver
Line 150:
Line 151: conf_paths = [
Done
libvirt_configure.sh already keeps those files names in
it, remove_conf verb call is enough without all this bunch of code.
Line 152: P_VDSM_LCONF,
Line 153: P_VDSM_QCONF,
Line 154: P_VDSM_LDCONF,
Line 155: ]
--
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes