[PATCH 00/21] Refactoring conf to use XPath

Kristina Hanicova posted 21 patches 3 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1618495577.git.khanicov@redhat.com
There is a newer version of this series
src/conf/domain_conf.c | 1786 +++++++++++++++++-----------------------
1 file changed, 777 insertions(+), 1009 deletions(-)
[PATCH 00/21] Refactoring conf to use XPath
Posted by Kristina Hanicova 3 years ago
This series reworks the outdated way of parsing XML to parsing by XPath,
which is more obvious and saves a few lines of code.

Kristina Hanicova (21):
  conf: Propagate xmlXPathContextPtr into
    virDomainHostdevSubsysUSBDefParseXML()
  Refactoring virDomainHostdevSubsysUSBDefParseXML() to use XPath
  conf: Propagate xmlXPathContextPtr into virDomainBlkioDeviceParseXML()
  Refactoring virDomainBlkioDeviceParseXML() to use XPath
  conf: Propagate xmlXPathContextPtr into
    virDomainHostdevSubsysPCIDefParseXML()
  Refactoring virDomainHostdevSubsysPCIDefParseXML() to use XPath
  conf: Propagate xmlXPathContextPtr into virDomainLeaseDefParseXML()
  Refactoring virDomainLeaseDefParseXML() to use XPath
  Refactoring virDomainDiskDefParseXML() to use XPath
  Refactoring virDomainControllerDefParseXML() to use XPath
  Refactoring virDomainFSDefParseXML() to use XPath
  Refactoring virDomainNetDefParseXML() to use XPath
  conf: Propagate xmlXPathContextPtr into
    virDomainChrDefParseTargetXML()
  Refactoring virDomainChrDefParseTargetXML() to use XPath
  Refactoring virDomainChrSourceDefParseXML() to use XPath
  Refactoring virDomainChrDefParseXML() to use XPath
  Refactoring virDomainSmartcardDefParseXML() to use XPath
  Refactoring virDomainGraphicsDefParseXMLSpice() to use XPath
  conf: Propagate xmlXPathContextPtr into
    virDomainVideoDriverDefParseXML()
  Refactoring virDomainVideoDriverDefParseXML() to use XPath
  Refactoring virDomainVideoDefParseXML() to use XPath

 src/conf/domain_conf.c | 1786 +++++++++++++++++-----------------------
 1 file changed, 777 insertions(+), 1009 deletions(-)

-- 
2.30.2

Re: [PATCH 00/21] Refactoring conf to use XPath
Posted by Tim Wiederhake 3 years ago
On Thu, 2021-04-15 at 16:26 +0200, Kristina Hanicova wrote:
> This series reworks the outdated way of parsing XML to parsing by
> XPath,
> which is more obvious and saves a few lines of code.
> 
> Kristina Hanicova (21):
>   conf: Propagate xmlXPathContextPtr into
>     virDomainHostdevSubsysUSBDefParseXML()
>   Refactoring virDomainHostdevSubsysUSBDefParseXML() to use XPath
>   conf: Propagate xmlXPathContextPtr into
> virDomainBlkioDeviceParseXML()
>   Refactoring virDomainBlkioDeviceParseXML() to use XPath
>   conf: Propagate xmlXPathContextPtr into
>     virDomainHostdevSubsysPCIDefParseXML()
>   Refactoring virDomainHostdevSubsysPCIDefParseXML() to use XPath
>   conf: Propagate xmlXPathContextPtr into virDomainLeaseDefParseXML()
>   Refactoring virDomainLeaseDefParseXML() to use XPath
>   Refactoring virDomainDiskDefParseXML() to use XPath
>   Refactoring virDomainControllerDefParseXML() to use XPath
>   Refactoring virDomainFSDefParseXML() to use XPath
>   Refactoring virDomainNetDefParseXML() to use XPath
>   conf: Propagate xmlXPathContextPtr into
>     virDomainChrDefParseTargetXML()
>   Refactoring virDomainChrDefParseTargetXML() to use XPath
>   Refactoring virDomainChrSourceDefParseXML() to use XPath
>   Refactoring virDomainChrDefParseXML() to use XPath
>   Refactoring virDomainSmartcardDefParseXML() to use XPath
>   Refactoring virDomainGraphicsDefParseXMLSpice() to use XPath
>   conf: Propagate xmlXPathContextPtr into
>     virDomainVideoDriverDefParseXML()
>   Refactoring virDomainVideoDriverDefParseXML() to use XPath
>   Refactoring virDomainVideoDefParseXML() to use XPath
> 
>  src/conf/domain_conf.c | 1786 +++++++++++++++++---------------------
> --
>  1 file changed, 777 insertions(+), 1009 deletions(-)
> 

Great to see that other people interested in cleaning up this part of
libvirt a bit, too!

Your approach seems to be going in the opposite direction of what I did
in the series I posted back in March [1]. That series rather reduces
xpath usage, than increasing it.

If you want to compare our versions, check out [2] and [3].

Kind regards,
Tim

[1] 
https://listman.redhat.com/archives/libvir-list/2021-March/msg00853.html
[2] 
https://listman.redhat.com/archives/libvir-list/2021-April/msg00232.html
[3] https://gitlab.com/twiederh/libvirt/-/compare/master...refactor_i


Re: [PATCH 00/21] Refactoring conf to use XPath
Posted by Michal Privoznik 3 years ago
On 4/15/21 6:27 PM, Tim Wiederhake wrote:
> On Thu, 2021-04-15 at 16:26 +0200, Kristina Hanicova wrote:
>> This series reworks the outdated way of parsing XML to parsing by
>> XPath,
>> which is more obvious and saves a few lines of code.
>>
>> Kristina Hanicova (21):
>>    conf: Propagate xmlXPathContextPtr into
>>      virDomainHostdevSubsysUSBDefParseXML()
>>    Refactoring virDomainHostdevSubsysUSBDefParseXML() to use XPath
>>    conf: Propagate xmlXPathContextPtr into
>> virDomainBlkioDeviceParseXML()
>>    Refactoring virDomainBlkioDeviceParseXML() to use XPath
>>    conf: Propagate xmlXPathContextPtr into
>>      virDomainHostdevSubsysPCIDefParseXML()
>>    Refactoring virDomainHostdevSubsysPCIDefParseXML() to use XPath
>>    conf: Propagate xmlXPathContextPtr into virDomainLeaseDefParseXML()
>>    Refactoring virDomainLeaseDefParseXML() to use XPath
>>    Refactoring virDomainDiskDefParseXML() to use XPath
>>    Refactoring virDomainControllerDefParseXML() to use XPath
>>    Refactoring virDomainFSDefParseXML() to use XPath
>>    Refactoring virDomainNetDefParseXML() to use XPath
>>    conf: Propagate xmlXPathContextPtr into
>>      virDomainChrDefParseTargetXML()
>>    Refactoring virDomainChrDefParseTargetXML() to use XPath
>>    Refactoring virDomainChrSourceDefParseXML() to use XPath
>>    Refactoring virDomainChrDefParseXML() to use XPath
>>    Refactoring virDomainSmartcardDefParseXML() to use XPath
>>    Refactoring virDomainGraphicsDefParseXMLSpice() to use XPath
>>    conf: Propagate xmlXPathContextPtr into
>>      virDomainVideoDriverDefParseXML()
>>    Refactoring virDomainVideoDriverDefParseXML() to use XPath
>>    Refactoring virDomainVideoDefParseXML() to use XPath
>>
>>   src/conf/domain_conf.c | 1786 +++++++++++++++++---------------------
>> --
>>   1 file changed, 777 insertions(+), 1009 deletions(-)
>>
> 
> Great to see that other people interested in cleaning up this part of
> libvirt a bit, too!
> 
> Your approach seems to be going in the opposite direction of what I did
> in the series I posted back in March [1]. That series rather reduces
> xpath usage, than increasing it.
> 
> If you want to compare our versions, check out [2] and [3].
> 
> Kind regards,
> Tim
> 
> [1]
> https://listman.redhat.com/archives/libvir-list/2021-March/msg00853.html
> [2]
> https://listman.redhat.com/archives/libvir-list/2021-April/msg00232.html
> [3] https://gitlab.com/twiederh/libvirt/-/compare/master...refactor_i
> 
> 

Let me review your patches, but your's and Kristina's doesn't look like 
opposing to me. I think Kristina's focus was mainly on removing while() 
style of traversing elements in favor of XPATH. Your's is to replace 
XMLPropString() + str2int/str2enum combo with one function.

Michal