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

ybronhei at redhat.com ybronhei at redhat.com
Wed Feb 26 09:20:26 UTC 2014


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 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