On 03/11/2017 08:16 PM, Laine Stump wrote: > On 03/10/2017 04:10 PM, John Ferlan wrote: >> Rather than use virXPathString, pass along an virXPathNode and alter >> the parsing to use virXMLPropString. > > Just so I understand the reasoning correctly - you're not doing this so you > can use virXMLPropString() instead of virXPathString(), but just so you can > remove the "adapter" from the path to each attribute (and in the cases where > that turns a "path" into simply the attribute name, you're switching to > virXMLPropString() because it's presumably slightly more efficient. Right? Or > is there some other reason you prefer virXMLPropString()? >
Missed this on the first pass through your review... Probably because your email client has stopped believing in line wrapping or my client isn't seeing some setting properly <sigh> I had originally wanted the ability to just parse an <adapter...> since that was how this would be represented in the domain; however, even though that got mothballed - I still felt what I'd done so far was at least a step up in readability, flow, etc. over what was there before. Secondary to that I find it easier to "read" the code when using virXMLPropString instead of virXPathString John >> >> Signed-off-by: John Ferlan <[email protected]> >> --- >> src/conf/storage_conf.c | 51 >> ++++++++++++++++++++++++++----------------------- >> 1 file changed, 27 insertions(+), 24 deletions(-) >> >> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c >> index 1993d3a..4fa7c12 100644 >> --- a/src/conf/storage_conf.c >> +++ b/src/conf/storage_conf.c >> @@ -464,13 +464,17 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools, >> >> static int >> virStoragePoolDefParseSourceAdapter(virStoragePoolSourcePtr source, >> + xmlNodePtr node, >> xmlXPathContextPtr ctxt) >> { >> int ret = -1; >> + xmlNodePtr relnode = ctxt->node; > > > (This is the first time I've seen "relnode" used to save a ctxt->node. When I > look through the source, the one other place I see it is in virxml.c (other > places use "node", "save_node", "oldnode", "save_ctxt", "tmpnode", and on an > on). Now I'm trying to figure out what "rel" stands for but nothing comes to > mind. How stupid am I?) > > >> [...] > > > ACK. > -- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
