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