Re: [libvirt] [PATCH 3/3] (REVISED) Support for IPv6 / multiple addresses per interface in virInterface
On 10/28/2009 07:06 AM, Daniel Veillard wrote:
On Fri, Oct 23, 2009 at 01:31:19PM -0400, Laine Stump wrote:
> diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
> index d46f7ac..7cb71ed 100644
> --- a/src/conf/interface_conf.c
> +++ b/src/conf/interface_conf.c
> @@ -53,9 +53,31 @@ void virInterfaceBareDefFree(virInterfaceBareDefPtr def) {
> VIR_FREE(def);
> }
>
> +static
> +void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
> + if (def == NULL)
> + return;
> + VIR_FREE(def->address);
> +}
>
Hum ... shouldn't def be freed too there ???
Yes, you are correct. It will be fixed in the redo.
> + ctxt->node = ipNodes[ii];
> + ret = virInterfaceDefParseIp(conn, ip, ctxt);
> + if (ret != 0) {
> + virInterfaceIpDefFree(ip);
> + goto error;
>
Hum ....
> + }
> + def->ips[def->nips++] = ip;
> + }
> +
> + ret = 0;
> +
> +error:
> + VIR_FREE(ipNodes);
> + return(ret);
> +}
>
It's hard to be sure we aren't leaking there too. On second reading
this looks okay because def is passed by the caller and we are storing
the data there.
Correct. The caller will clean up if there's a failure.
Hum, we don't check there that an ipv6/4 protocol is not
defined
multiple time. Maybe this could be slightly refactored to search
for 1 protocol node with type IPv4 and the one protocol node for type
Ipv6 something like
v4 = virXPathNode(conn, "./protocol[@family = 'ipv4']", ctxt)
v6 = virXPathNode(conn, "./protocol[@family = 'ipv6']", ctxt)
Yeah, I can do that.
In the meantime, I've sent a netcf patch that gets vlan xml passing the
libvirt parse, but I'm having trouble finding the info for bridges and
bonds either in sysfs or netlink, and my brain is finished for the day.
Basically, for a bridge to pass, we need the name of the physical
interface, and for bond to pass, we need an miimon node with freq
property, or an arpmode node with interval and target properties, in
addition to the names of the physical interfaces. I'm not finding those
anywhere.