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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list