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

[John Ferlan]

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
>