[libvirt] [PATCH] fix some XPath relative node resets

The virXPath... function take extra care to preserve the XPath context node (ctxt->node) but in the case of virXPathString and virXPathBoolean they forgot to do this on the error path. This patch fixes this and move all ctxt->node = relnode instuctions just after the xmlXPathEval() to make sure this doesn't happen if this code is modified. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 09/28/2009 11:30 AM, Daniel Veillard wrote:
The virXPath... function take extra care to preserve the XPath context node (ctxt->node) but in the case of virXPathString and virXPathBoolean they forgot to do this on the error path. This patch fixes this and move all ctxt->node = relnode instuctions just after the xmlXPathEval() to make sure this doesn't happen if this code is modified.
Daniel, I tried this patch with my tree, and it didn't change the behavior - in virInterfaceDefParseProtoIPv4 if I find a dhcp node with dhcp = virXPathNode(conn, "./dhcp", ctxt); any ip node will not be found with a subsequent ip = virXPathNode(conn, "./ip", ctxt); (if dhcp comes back null, the ip node *is* found). However, if I first make the call to look for the ip node, and then do the call for the dhcp node, both are found. Does that provide any clue to you? Also, you'll notice a new bit of code in virInterfaceDefParseIp: tmp = virXPathString(conn, "string(./ip[1]/@source)", ctxt); For some reason, tmp always comes back null, even if my ip node looks like the one in the example below: <interface type="ethernet" name="wlan0"> <start mode="none"/> <mac address="00:1f:3c:9d:87:7c"/> <protocol family="ipv4"> <dhcp/> <ip address="10.24.0.101" prefix="24" source="device"/> </protocol> </interface>

On Mon, Sep 28, 2009 at 05:19:12PM -0400, Laine Stump wrote:
On 09/28/2009 11:30 AM, Daniel Veillard wrote:
The virXPath... function take extra care to preserve the XPath context node (ctxt->node) but in the case of virXPathString and virXPathBoolean they forgot to do this on the error path. This patch fixes this and move all ctxt->node = relnode instuctions just after the xmlXPathEval() to make sure this doesn't happen if this code is modified.
Daniel,
I tried this patch with my tree, and it didn't change the behavior - in virInterfaceDefParseProtoIPv4 if I find a dhcp node with
dhcp = virXPathNode(conn, "./dhcp", ctxt);
any ip node will not be found with a subsequent
ip = virXPathNode(conn, "./ip", ctxt);
(if dhcp comes back null, the ip node *is* found).
However, if I first make the call to look for the ip node, and then do the call for the dhcp node, both are found. Does that provide any clue to you?
I still think there is a problem with ctxt->node can you verify that it's not modified by the all to parse Ip or Dhcp. I had a quick look and that's how I spotted the problem, but could you check within gdb ?
Also, you'll notice a new bit of code in virInterfaceDefParseIp:
tmp = virXPathString(conn, "string(./ip[1]/@source)", ctxt);
For some reason, tmp always comes back null, even if my ip node looks like the one in the example below:
that mean that at the time you call virXPathString, then the ctxt->node is not pointing to the <protocol> element anymore.
<interface type="ethernet" name="wlan0"> <start mode="none"/> <mac address="00:1f:3c:9d:87:7c"/> <protocol family="ipv4"> <dhcp/> <ip address="10.24.0.101" prefix="24" source="device"/> </protocol> </interface>
I really think something changes ctxt->node in virXPath... or one of the subparsing routines. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 09/28/2009 11:30 AM, Daniel Veillard wrote:
The virXPath... function take extra care to preserve the XPath context node (ctxt->node) but in the case of virXPathString and virXPathBoolean they forgot to do this on the error path. This patch fixes this and move all ctxt->node = relnode instuctions just after the xmlXPathEval() to make sure this doesn't happen if this code is modified.
ACK to this. The failure I encountered was due to a similar problem somewhere else (patch upcoming), but these are definitely correcting a problem.

On 09/28/2009 06:39 PM, Laine Stump wrote:
On 09/28/2009 11:30 AM, Daniel Veillard wrote:
The virXPath... function take extra care to preserve the XPath context node (ctxt->node) but in the case of virXPathString and virXPathBoolean they forgot to do this on the error path. This patch fixes this and move all ctxt->node = relnode instuctions just after the xmlXPathEval() to make sure this doesn't happen if this code is modified.
ACK to this. The failure I encountered was due to a similar problem somewhere else (patch upcoming), but these are definitely correcting a problem.
I take that back - one of my two problems *was* fixed by this patch ;-)

On Mon, Sep 28, 2009 at 06:42:14PM -0400, Laine Stump wrote:
On 09/28/2009 06:39 PM, Laine Stump wrote:
On 09/28/2009 11:30 AM, Daniel Veillard wrote:
The virXPath... function take extra care to preserve the XPath context node (ctxt->node) but in the case of virXPathString and virXPathBoolean they forgot to do this on the error path. This patch fixes this and move all ctxt->node = relnode instuctions just after the xmlXPathEval() to make sure this doesn't happen if this code is modified.
ACK to this. The failure I encountered was due to a similar problem somewhere else (patch upcoming), but these are definitely correcting a problem.
I take that back - one of my two problems *was* fixed by this patch ;-)
Okay, commited, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel Veillard
-
Laine Stump