[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