1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH v2 0/6] Alter domain_conf to make use of autofree

John Ferlan posted 6 patches 5 years, 1 month ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190220183404.31810-1-jferlan@redhat.com
There is a newer version of this series
src/conf/domain_conf.c | 1937 ++++++++++++++--------------------------
1 file changed, 676 insertions(+), 1261 deletions(-)
[libvirt] [PATCH v2 0/6] Alter domain_conf to make use of autofree
Posted by John Ferlan 5 years, 1 month ago 
v1: https://www.redhat.com/archives/libvir-list/2019-February/msg01160.html

Changes since v1:

 * Push patch 1 

 * Split patch 2 to follow code review guidance:

   * (Patch1) Pull out virDomainEmulatorPinDefParseXML changes to
               use VIR_STEAL_PTR
   * (Patch2) Use VIR_AUTOPTR(virBitmap) 
   * (Patch3) Handle a couple cases where goto's no longer necessary

 * Drop Patch 3

 * Split Patch 4 to follow code review guidance:

   * (Patch4) Remove unused variables
   * (Patch5) Just use VIR_AUTOFREE
   * (Patch6) Handle the removal of goto logic that's no longer necessary
  

John Ferlan (6):
  conf: Rework virDomainEmulatorPinDefParseXML
  conf: Use VIR_AUTOPTR(virBitmap) in domain_conf
  conf: Clean up some unnecessary goto paths
  conf: Remove a few unused variables in domain_conf
  conf: Use VIR_AUTOFREE in domain_conf
  conf: Clean up some unnecessary goto paths

 src/conf/domain_conf.c | 1937 ++++++++++++++--------------------------
 1 file changed, 676 insertions(+), 1261 deletions(-)

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/6] Alter domain_conf to make use of autofree
Posted by John Ferlan 5 years, 1 month ago 
ping?

I also think that w/ Peter's addition of VIR_XPATH_NODE_AUTORESTORE even
more changes could be done, but I'd leave those for either Peter to
finish what he started or the mythical future someone else.

Tks -

John

On 2/20/19 1:33 PM, John Ferlan wrote:
> v1: https://www.redhat.com/archives/libvir-list/2019-February/msg01160.html
> 
> Changes since v1:
> 
>  * Push patch 1 
> 
>  * Split patch 2 to follow code review guidance:
> 
>    * (Patch1) Pull out virDomainEmulatorPinDefParseXML changes to
>                use VIR_STEAL_PTR
>    * (Patch2) Use VIR_AUTOPTR(virBitmap) 
>    * (Patch3) Handle a couple cases where goto's no longer necessary
> 
>  * Drop Patch 3
> 
>  * Split Patch 4 to follow code review guidance:
> 
>    * (Patch4) Remove unused variables
>    * (Patch5) Just use VIR_AUTOFREE
>    * (Patch6) Handle the removal of goto logic that's no longer necessary
>   
> 
> John Ferlan (6):
>   conf: Rework virDomainEmulatorPinDefParseXML
>   conf: Use VIR_AUTOPTR(virBitmap) in domain_conf
>   conf: Clean up some unnecessary goto paths
>   conf: Remove a few unused variables in domain_conf
>   conf: Use VIR_AUTOFREE in domain_conf
>   conf: Clean up some unnecessary goto paths
> 
>  src/conf/domain_conf.c | 1937 ++++++++++++++--------------------------
>  1 file changed, 676 insertions(+), 1261 deletions(-)
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/6] Alter domain_conf to make use of autofree
Posted by Erik Skultety 5 years, 1 month ago 
On Wed, Feb 27, 2019 at 12:31:56PM -0500, John Ferlan wrote:
> ping?
>
> I also think that w/ Peter's addition of VIR_XPATH_NODE_AUTORESTORE even
> more changes could be done, but I'd leave those for either Peter to
> finish what he started or the mythical future someone else.

I also found some occurrences of virObjectUnref which may be replaced by
VIR_AUTOUNREF, so if you wouldn't mind posting a follow up so that we don't
need to revisit this module for the same purpose for a while. A few more notes
for a potential follow-up:

- in virDomainKeyWrapDefParseXML we could introduce a VIR_AUTOFREE helper for
  keywrap and do VIR_STEALPTR so that we can get rid of the whole cleanup
  section
- virDomainVsockDefNew could make use of VIR_AUTOPTR
- virDomainDefBootOrderPostParse could make use of VIR_AUTOPTR
- virDomainDefValidateAliases (VIR_AUTOPTR)
- virDomainDeviceValidateAliasImpl (VIR_AUTOPTR)
- virDomainDiskSourcePoolDefParse (VIR_AUTOPTR)
- virSysinfoChassisParseXML (VIR_AUTOPTR)
- virDomainCachetuneDefParse (VIR_AUTOUNREF)
- virDomainMemorytuneDefParse (VIR_AUTOUNREF)
- virDomainDefAddImplicitVideo (VIR_AUTOPTR)

And a bunch of others which I didn't bother checking thoroughly.
Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/6] Alter domain_conf to make use of autofree
Posted by John Ferlan 5 years, 1 month ago 

On 3/1/19 8:38 AM, Erik Skultety wrote:
> On Wed, Feb 27, 2019 at 12:31:56PM -0500, John Ferlan wrote:
>> ping?
>>
>> I also think that w/ Peter's addition of VIR_XPATH_NODE_AUTORESTORE even
>> more changes could be done, but I'd leave those for either Peter to
>> finish what he started or the mythical future someone else.
> 
> I also found some occurrences of virObjectUnref which may be replaced by
> VIR_AUTOUNREF, so if you wouldn't mind posting a follow up so that we don't
> need to revisit this module for the same purpose for a while. A few more notes
> for a potential follow-up:

A bit of history - based on the storage changes I made, I started
thinking maybe it's time to just start the process of getting more code
to use the VIR_AUTO* stuff.  I figured domain_conf would be the most
challenging and it's where I started (and before Peter wrote the
VIR_AUTOUNREF patches).

My initial pass was look for virBitmapPtr and VIR_AUTOFREE. I thought I
got most although I do see a couple more that would be possible:

 * virDomainHugepagesFormatBuf for @nodeset
 * virDomainChrTargetDefFormat for @addr within TYPE_GUESTFWD
 * virDomainNetDefFormat for @str, @gueststr, & @hoststr within
def->model processing
 * virDomainChannelDefCheckABIStability for @saddr & @daddr within
TYPE_GUESTFWD

> 
> - in virDomainKeyWrapDefParseXML we could introduce a VIR_AUTOFREE helper for
>   keywrap and do VIR_STEALPTR so that we can get rid of the whole cleanup
>   section

Sure this is simple enough to do. Hopefully acceptible in one patch at
the end of the series and not a multipatch step...

> - virDomainVsockDefNew could make use of VIR_AUTOPTR
> - virDomainDefBootOrderPostParse could make use of VIR_AUTOPTR
> - virDomainDefValidateAliases (VIR_AUTOPTR)
> - virDomainDeviceValidateAliasImpl (VIR_AUTOPTR)
> - virDomainDiskSourcePoolDefParse (VIR_AUTOPTR)
> - virSysinfoChassisParseXML (VIR_AUTOPTR)
> - virDomainCachetuneDefParse (VIR_AUTOUNREF)
> - virDomainMemorytuneDefParse (VIR_AUTOUNREF)
> - virDomainDefAddImplicitVideo (VIR_AUTOPTR)
> 

VIR_AUTOPTR is beyond the scope of what I was doing and altering all the
places for virObjectUnref really could have been done by when
VIR_AUTOUNREF was introduced (VIR_XPATH_NODE_AUTORESTORE too).
We keep adding this VIR_AUTO* things, but without someone actively
trying to complete the work, it will never happen and we'll be
continually stuck in this limbo state. For some it's more than first
patch type contribution.

I don't mind adding VIR_AUTOUNREF since it's already pushed, but adding
new VIR_AUTOPTR's is an exercise for someone else eventually as far more
than domain_conf would need to be touched in order to do it right.

John

I'll post a v3 shortly with the diffs requested as part of review and
the few extra VIR_AUTOFREE's I found for you to look at.

> And a bunch of others which I didn't bother checking thoroughly.
> Erik
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.