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 <jferlan@xxxxxxxxxx>
>> ---
>> 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
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
