Re: [libvirt] [PATCH 3/3] (REVISED) Support for IPv6 / multiple addresses per interface in virInterface

On 10/29/2009 02:53 AM, Laine Stump wrote: > On 10/28/2009 07:06 AM, Daniel Veillard wrote: >> On Fri, Oct 23, 2009 at 01:31:19PM -0400, Laine Stump wrote: >> > >> 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. Now that I'm actually doing it, I'm having doubts about this. Making this change leads to many other changes (mostly making the code simpler, but still changed lines are changed lines, and could lead to new bugs). More important, though, I notice that if we do it this way we lose the ability to report an error if someone botches up the family of a protocol - with this new method of parsing it will just be ignored; with the current setup in my patch, an error will be reported if there's a protocol that is missing family, or has family set to anything other than "ipv4" or "ipv6".

Would it maybe be better to still cycle through all the protocol nodes in the document with a loop as currently, but report an error if a particular protocol family is encountered a 2nd time?

If so, can/should that be submitted as a separate patch, rather than trying to get it into the patch that's already working? (If that's the case, I'll clean up and resend the patches sooner rather than later).