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

[Daniel Veillard]

On Fri, Oct 30, 2009 at 10:34:03AM -0400, Laine Stump wrote:
> 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".

  Ah right, I didn't though about that side of the checking

> 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?

  yup,

> 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).

  Let's just make a cleanup patch on top of the current one !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/