David Lutterkort
Tue, 03 Nov 2009 18:18:47 -0800
On Mon, 2009-11-02 at 00:08 -0500, Laine Stump wrote:
> diff --git a/src/dutil.c b/src/dutil.c
> index d4eea90..d82b3d8 100644
> --- a/src/dutil.c
> +++ b/src/dutil.c
> @@ -509,40 +513,94 @@ int if_is_active(struct netcf *ncf, const char *intf) {
> +char *if_bridge_phys_name(struct netcf *ncf, const char *intf) {
> + /* We can learn the name of the physical interface associated
> + * with this bridge by looking for the name of the one and
> + * only link in /sys/class/net/$ifname/brif.
> + *
> + * The caller of this function must free the string that is
> + * returned.
> + *
> + * For efficiency's sake, this function assumes we've already
> + * checked that this is a bridge interface.
> + */
> + char *ret = NULL;
> + char *dirpath = NULL;
> + DIR *dir = NULL;
> +
> + xasprintf(&dirpath, "/sys/class/net/%s/brif", intf);
> + ERR_NOMEM(dirpath == NULL, ncf);
> +
> + dir = opendir(dirpath);
> + if (dir != NULL) {
> + struct dirent *d;
> +
> + while ((d = readdir (dir)) != NULL) {
> + if (STRNEQ(d->d_name, ".") && STRNEQ(d->d_name, "..")) {
> + xasprintf(&ret, "%s", d->d_name);
> + ERR_NOMEM(ret == NULL, ncf);
> + break;
> + }
> + }
> + }
It's not common in the virtualization world, but perfectly permissible
to have more than one interface in a bridge. This code will report only
one.
> +static void add_ethernet_info_cb(struct nl_object *obj, void *arg) {
> + struct nl_ethernet_callback_data *cb_data = arg;
> struct rtnl_link *iflink = (struct rtnl_link *)obj;
> - struct netcf *ncf = cb_data->nif->ncf;
> + struct netcf *ncf = cb_data->ncf;
>
> struct nl_addr *addr;
> - char mac_str[64];
> xmlNodePtr cur;
> xmlAttrPtr prop = NULL;
>
> - if (cb_data->mac != NULL)
> + /* look for mac address */
> + if ((cb_data->mac == NULL)
> + && ((addr = rtnl_link_get_addr(iflink)) != NULL)
> + && !nl_addr_iszero(addr)) {
> +
> + char mac_str[64];
> +
> + nl_addr2str(addr, mac_str, sizeof(mac_str));
> +
> + for (cur = cb_data->root->children; cur != NULL; cur = cur->next) {
> + if ((cur->type == XML_ELEMENT_NODE) &&
> + xmlStrEqual(cur->name, BAD_CAST "mac")) {
> + cb_data->mac = cur;
> + break;
> + }
> + }
> +
> + if (cb_data->mac == NULL) {
> + /* No mac node exists in the document, create a new one.
> + */
> + cb_data->mac = xmlNewDocNode(cb_data->doc, NULL,
> + BAD_CAST "mac", NULL);
> + ERR_NOMEM(cb_data->mac == NULL, ncf);
> +
> + cur = xmlAddChild(cb_data->root, cb_data->mac);
> + if (cur == NULL) {
> + xmlFreeNode(cb_data->mac);
> + cb_data->mac = NULL;
> + report_error(ncf, NETCF_ENOMEM, NULL);
> + goto error;
> + }
> + }
This type of code (search for an element, add one if it doesn't exist)
appears in quite a few places now - could that be factored into its own
static function ?
> + /* Add in type-specific info of master interface */
> + master_ifindex = rtnl_link_name2i(ncf->driver->link_cache, master_name);
> + ERR_THROW((master_ifindex == RTNL_LINK_NOT_FOUND), ncf, ENETLINK,
> + "couldn't find ifindex for vlan master interface `%s`",
> + master_name);
Just a small stylistic nit: there's no need to enclose the conditions
for ERR_THROW/ERR_NOMEM in parens.
David
_______________________________________________
netcf-devel mailing list
netcf-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/netcf-devel