Hi Jonas,
On Fri, 2009-06-26 at 10:16 +0200, Jonas Eriksson wrote:
The things I have tried worked. I have a few problems, though:
' and " are not handeled. As bridges SuSE have their own ifcfg-file, the BRIDGE_PORT contain the interfaces used in that bridge, separated by whitespaces. My problem is mostly whether to extend shellvars.aug to handle ' and ", or if this should be handled in C/XSLT.
I currently have a similar problem with BONDING_OPTS in the initscripts driver; I worked around that with an XSLT extension function bond:option - though I am thinking that changing the Shellvars lens to handle that would be much cleaner. It would require shipping a custom shellvars.aug with netcf, but that's not a big issue.
In SLE10, interfaces could be named after their mac address using eth-id-<mac>. This is now defunct and handled by udev-rules, but I have somewhat retained handling of this, but convert it to ethX-syntax.
Are you sure that this is worth it ? According to [1], openSuSE 10.3 will be EOL'd at the end of October, and the eth-id-<mac> form seems to lead to quite a bit of internal headaches. At the very least, it's probably better to write the driver for OpenSuSE 11.x first, and add support for OpenSuSE 10 later.
However, when facing an interface-element in netcf-xml with a mac address, I convert this to a eth-id-interface in XSLT, and then to ethX in C by looking up the mac. Instead of doing this, the driver should retain the original ethX-name as an fallback in case the mac is not found.
So the eth-id-<mac> files contain no indication what the device name should be ? You could also address the above headache with an XSLT extension function - that all assumes of course that the MAC uniquely identifies one device.
Whenever using define in netcf, i get: error: unspecified error error: aug_save failed After doing some dry runs with augtool, I have concluded that this is most probably due to augeas trying to save the address-files in /sys. Is this a problem when using the initscript-driver as well?
No, not at all; if your driver doesn't modify entries under /files/sys/, Augeas won't ever try to write them. If you have an augtool script that reproduces the problem, do a 'print /augeas//error' after the faild save.
Currently, there is a mix between functions that accept name or path. I am planning to clean up this mess, preferably standardizing on name as argument.
I also have a few questions/opinions:
Should we include some general debug macros? I stole the ones from libvirt and removed them before this post, and i had alot of use for them, for example to see the <forest>-xml when doing a define.
Yeah, that might be generally useful. For the specific issue of debugging the XSL transforms, I added a little helper program in git (ncftransform) that lets you run either direction, e.g.
netcf>NETCF_DATADIR=$PWD/data/ ./src/ncftransform put tests/initscripts/bridge.xml netcf>NETCF_DATADIR=$PWD/data/ ./src/ncftransform get tests/interface/bridge.xml
We also need to run the transformation tests in tests/test-initscripts.c for the SuSE driver, which means we need files for tests/suse/
Alot of code are the same between the initscript driver, and probably will be for other drivers as well. I recommend that this code is broken out, for example to a Linux-helper? The same goes for both C-code (e.g. handling of mac-address lookups) and XSLT (e.g. bonding-options).
Yes, that's my biggest comment about your patches: we need to reduce the amount of duplicated code, since that will just become an enormous maintenance headache. The code needs to be broken up in a major way. For review purposes it's best if you structure that into lots of small patches.
I changed the augeas save-behaviour to "backup" in the augeas init function (which probably also could be lifted out to a general helper file).
Yes, get_augeas needs to be factored out; it needs to get the augeas_xfm array as an additional input parameter, but other than that, there should be no difference between the two drivers.
I left it in there, since I like it alot, but we should probably discuss it further.
For debugging/testing it's definitely helpful, though for production use, I would feel very uncomfortable littering the filesystem with lots of .augsave files. This would be a perfect use for a debug macro.
David