5:24 p.m.
On Thu, 2009-09-24 at 17:11 -0400, Laine Stump wrote:
>> @@ -737,6 +737,64 @@ char *drv_xml_desc(struct netcf_if
*nif) {
>> goto done;
>> }
>>
>> +/* return the current live configuration state - a combination of
>> + * drv_xml_desc + results of querying the interface directly */
>> +
>> +char *drv_xml_state(struct netcf_if *nif) {
>> + char *result = NULL;
>> + int r, result_len;
>> + struct augeas *aug;
>> + struct netcf *ncf;
>> + char **devs = NULL, **intf = NULL;
>> + xmlDocPtr aug_xml = NULL, ncf_xml = NULL;
>> + int ndevs = 0, nint = 0;
>> + unsigned int ipv4;
>> + int prefix;
>> +
>> + ncf = nif->ncf;
>> + aug = get_augeas(ncf);
>> + ERR_BAIL(ncf);
>> +
>> + ndevs = aug_fmt_match(ncf,&devs,
>> + "%s[ DEVICE = '%s' or BRIDGE = '%s' or
MASTER = '%s']/DEVICE",
>> + ifcfg_path, nif->name, nif->name, nif->name);
>> + ERR_BAIL(ncf);
>> +
>> + nint = uniq_ifcfg_paths(ncf, ndevs, devs,&intf);
>> + ERR_BAIL(ncf);
>> +
>> + aug_xml = aug_get_xml(ncf, nint, intf);
>> +
>> +
>> + /* same as drv_xml_desc() up to this point, a bit different from now on...
*/
>>
> Can that be factored into some helper function that drv_xml_desc and
> drv_xml_state both use ?
>
Does "aug_get_xml" is already taken by the function that gets the
aug_xml for all interfaces. How about I name this function
aug_get_one_xml? Or do you have a better idea?
How about aug_get_xml_for_nif, since it takes a netcf_if and returns the
parts of the Augeas tree that are important for that interface.
>
>> + ncf_xml = apply_stylesheet(ncf, ncf->driver->put, aug_xml);
>> + ERR_BAIL(ncf);
>> +
>> + /* get the current IP address. If it's non-zero, also get the
>> + * current prefix, and add both to the document */
>> + ipv4 = if_ipv4_address(ncf, nif->name);
>>
> if_ipv4_address should set ncf->errcode (with one of the ERR_ macros),
> and you should use ERR_BAIL(ncf) after the invocation to make sure
> errors ar propagated. That also means you don't have the 'if (ipv4 !=
> 0)' check.
>
Actually I was assuming that, in the case of an interface that didn't
yet have an IP address ifconfiged, the ioctl would return a 0 result,
but no error. In this case you wouldn't want to add 'ip
address="0.0.0.0"' to the document. So I think I should have the if_*
functions setting errcode, but I still need to check for != 0.
Agreed - you need to handle both, some sort of error (OOM etc.) and that
the interface has no address assigned.
Yeah, I agree. I actually dislike goto's and labels (knee-jerk
reflex
from from my first quarter freshman year data structures class (so long
ago it was taught in Pascal)), but I've seen so many lately it's become
background noise.
Dijkstra lives ! I try to only use goto's where you'd throw an xception
in other languages, and have only at most two labels in a function, one
for successful exit, and one for error exit + cleanup.
> The interface.rng currently also doesn't allow both
a<dhcp/> element
> and an<ip/> entry - in the future it will, but with a different
> meaning.
Ah, right. So since I'm modifying the tree just before spitting it out
as ASCII, it never gets validated, and never sees that I've broken the
rules!
I am not particularly worried about that - just something to keep in
mind.
> So, I wonder if there is a need to add something to
the<ip/>
> element to indicate that the address is the live address.
>
I think it's implied in which function is called that it's the live
address. What I wonder is if anyone will ever want to know that, for
example, the config file has one particular static IP, but the interface
is currently set to something different. If that's important to get in a
single call, then we would need something added to <ip/> to
differentiate between static and live addresses.
For that, they can always call the two different XML-generating
functions and hunt through the docs themselves.
Yo'ure just suggesting adding something like
'live="true"' (or whatever,
maybe 'source="device"' vs. 'source="config"'?) to an
otherwise
identically formatted node, right?
Yeah, something like that - might be overkill, but I like the
'source="device"' annotation.
David