Change in vdsm[master]: move get-conf-item/set-conf-item to vdsm-tool

zhshzhou at linux.vnet.ibm.com zhshzhou at linux.vnet.ibm.com
Wed Oct 17 09:29:46 UTC 2012


Zhou Zheng Sheng has posted comments on this change.

Change subject: move get-conf-item/set-conf-item to vdsm-tool
......................................................................


Patch Set 4: I would prefer that you didn't submit this

(2 inline comments)

-1 for the the way of writing configuration text, but it's just a my opinion. We need to discuss more.

....................................................
File .gitignore
Line 49: vdsm/vdsm-gencerts.sh
Line 50: vdsm/vdsm-logrotate.conf
Line 51: vdsm/vdsm.rwtab
Line 52: vdsm/vdsm-sosplugin.py
Line 53: vdsm/vdsm-store-net-config
Maybe sorting the list can be split to a separate patch for easy reviewing.


....................................................
File vdsm-tool/conf-item.py.in
Line 50:     else:
Line 51:         print val
Line 52: 
Line 53: 
Line 54: def _edit_conf_item(lines, section, item, value):
The original set-conf-item uses ConfigParser in the official python library to write out the configuration lines. ConfigParser will discard comments and extra blank lines, which makes the resulting file hard to read for human.

This function implement an ad-hoc algorithm to operate on the configuration lines, it retains the comments and extra blank lines. However this kind of algorithm is twisted and hard to understand, can will break on corner cases. We can not afford to implement a sophisticated algorithm for this small patch.

I suggest we continue to use ConfigParser to write the configuration file, and keep a vdsm.conf.sample in the /etc/vdsm/ for the user. Let the programme operate on vdsm.conf, and let vdsm.conf be a human friendly documentation.
Line 55:     try:
Line 56:         pos = [line.strip() for line in lines].index('[%s]' % section)
Line 57:         if pos + 1 > len(lines):
Line 58:             lines.extend(["%s = %s\n" % (item, value), "\n"])


--
To view, visit http://gerrit.ovirt.org/7695
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5c64de097bbaea6a8cf862b43243377e10e00391
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Bing Bu Cao <mars at linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh at us.ibm.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi at linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list