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 <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: mooli tayer <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
