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 <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Bing Bu Cao <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Ryan Harper <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Wenyi Gao <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to