[netcf-devel] [PATCH 3/3] Add a new API function ncf_if_xml_state().

David Lutterkort lutter at redhat.com
Thu Sep 24 22:24:23 UTC 2009


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




More information about the netcf-devel mailing list