Re: [libvirt] [PATCH 3/6] qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 3/3
On Thu, Sep 26, 2019 at 01:15:16PM -0300, Daniel Henrique Barboza wrote: > > > On 9/26/19 6:18 AM, Erik Skultety wrote: > > On Wed, Sep 18, 2019 at 11:56:55AM -0300, Daniel Henrique Barboza wrote: > > > virQEMUDriverConfigPtr can be auto-unref for the great majority > > > of the uses made in qemu_driver, sparing us a virObjectUnref() > > > call and sometimes a whole 'cleanup' label. > > > > > > This patch changes virQEMUDriverConfigPtr declarations to > > > use VIR_AUTOUNREF(). 'cleanup' labels were deleted when > > > applicable. > > > > > > This is the last part of this change. All but one* instance of > > > virQEMUDriverConfigPtr were changed to use VIR_AUTOUNREF(). > > > 'cleanup' labels were deleted when applicable. > > > > > > * qemuStateInitialize: we can't auto-unref the pointer since we're > > > initializing the qemu_driver object with it. > > > > > > Signed-off-by: Daniel Henrique Barboza > > > --- > > I know you focused on virQEMUDriverConfigPtr primarily, but since you're > > following up with VIR_AUTOFREE and touching qemu_driver only, I'd like to > > do a > > better job and use VIR_AUTOUNREF at many more places across the file for: > > > > virCapsPtr caps > > virConnectPtr conn > > qemuDomainSaveCookiePtr cookie > > virQEMUCapsPtr qemuCaps > > qemuBlockJobDataPtr job > > virDomainCapsPtr domCaps > > virNetworkPtr network > > No problem. Should I squash the first 3 patches of this series into a single I would have squashed those 3 into a single one before pushing anyway. > one that will touch only virQEMUDriverConfigPtr and then make one patch > for each pointer type that's changed? Ultimately I'd have all the VIR_AUTOUNREF changes touching the same file in a single patch, but for review purposes it may be even better to have them split in order to separate the changes I've already reviewed and the ones you'll introduce. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 3/3
On 9/26/19 6:18 AM, Erik Skultety wrote: On Wed, Sep 18, 2019 at 11:56:55AM -0300, Daniel Henrique Barboza wrote: virQEMUDriverConfigPtr can be auto-unref for the great majority of the uses made in qemu_driver, sparing us a virObjectUnref() call and sometimes a whole 'cleanup' label. This patch changes virQEMUDriverConfigPtr declarations to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable. This is the last part of this change. All but one* instance of virQEMUDriverConfigPtr were changed to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable. * qemuStateInitialize: we can't auto-unref the pointer since we're initializing the qemu_driver object with it. Signed-off-by: Daniel Henrique Barboza --- I know you focused on virQEMUDriverConfigPtr primarily, but since you're following up with VIR_AUTOFREE and touching qemu_driver only, I'd like to do a better job and use VIR_AUTOUNREF at many more places across the file for: virCapsPtr caps virConnectPtr conn qemuDomainSaveCookiePtr cookie virQEMUCapsPtr qemuCaps qemuBlockJobDataPtr job virDomainCapsPtr domCaps virNetworkPtr network No problem. Should I squash the first 3 patches of this series into a single one that will touch only virQEMUDriverConfigPtr and then make one patch for each pointer type that's changed? Thanks, DHB (there may be a few more...) Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 3/3
On Wed, Sep 18, 2019 at 11:56:55AM -0300, Daniel Henrique Barboza wrote: > virQEMUDriverConfigPtr can be auto-unref for the great majority > of the uses made in qemu_driver, sparing us a virObjectUnref() > call and sometimes a whole 'cleanup' label. > > This patch changes virQEMUDriverConfigPtr declarations to > use VIR_AUTOUNREF(). 'cleanup' labels were deleted when > applicable. > > This is the last part of this change. All but one* instance of > virQEMUDriverConfigPtr were changed to use VIR_AUTOUNREF(). > 'cleanup' labels were deleted when applicable. > > * qemuStateInitialize: we can't auto-unref the pointer since we're > initializing the qemu_driver object with it. > > Signed-off-by: Daniel Henrique Barboza > --- I know you focused on virQEMUDriverConfigPtr primarily, but since you're following up with VIR_AUTOFREE and touching qemu_driver only, I'd like to do a better job and use VIR_AUTOUNREF at many more places across the file for: virCapsPtr caps virConnectPtr conn qemuDomainSaveCookiePtr cookie virQEMUCapsPtr qemuCaps qemuBlockJobDataPtr job virDomainCapsPtr domCaps virNetworkPtr network (there may be a few more...) Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 3/3
virQEMUDriverConfigPtr can be auto-unref for the great majority of the uses made in qemu_driver, sparing us a virObjectUnref() call and sometimes a whole 'cleanup' label. This patch changes virQEMUDriverConfigPtr declarations to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable. This is the last part of this change. All but one* instance of virQEMUDriverConfigPtr were changed to use VIR_AUTOUNREF(). 'cleanup' labels were deleted when applicable. * qemuStateInitialize: we can't auto-unref the pointer since we're initializing the qemu_driver object with it. Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_driver.c | 57 ++ 1 file changed, 19 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c7ed662ff1..bc2e2ccfc2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10209,7 +10209,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virDomainDefPtr persistentDef; virDomainObjPtr vm = NULL; int ret = -1; -virQEMUDriverConfigPtr cfg = NULL; +VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuDomainObjPrivatePtr priv; virBitmapPtr nodeset = NULL; virDomainNumatuneMemMode config_mode; @@ -10321,7 +10321,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom, cleanup: virBitmapFree(nodeset); virDomainObjEndAPI(&vm); -virObjectUnref(cfg); return ret; } @@ -10426,7 +10425,7 @@ qemuDomainSetPerfEvents(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; size_t i; virDomainObjPtr vm = NULL; -virQEMUDriverConfigPtr cfg = NULL; +VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuDomainObjPrivatePtr priv; virDomainDefPtr def; virDomainDefPtr persistentDef; @@ -10518,7 +10517,6 @@ qemuDomainSetPerfEvents(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); -virObjectUnref(cfg); return ret; } @@ -10705,7 +10703,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, long long value_l; int ret = -1; int rc; -virQEMUDriverConfigPtr cfg = NULL; +VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; virObjectEventPtr event = NULL; @@ -10999,7 +10997,6 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, if (eventNparams) virTypedParamsFree(eventParams, eventNparams); virObjectUnref(caps); -virObjectUnref(cfg); return ret; } #undef SCHED_RANGE_CHECK @@ -11664,7 +11661,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, int ret = -1; virDomainNetDefPtr net = NULL, persistentNet = NULL; virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL; -virQEMUDriverConfigPtr cfg = NULL; +VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; bool inboundSpecified = false, outboundSpecified = false; int actualType; bool qosSupported = true; @@ -11859,7 +11856,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virNetDevBandwidthFree(bandwidth); virNetDevBandwidthFree(newBandwidth); virDomainObjEndAPI(&vm); -virObjectUnref(cfg); return ret; } @@ -12121,7 +12117,7 @@ qemuDomainMemoryPeek(virDomainPtr dom, char *tmp = NULL; int fd = -1, ret = -1; qemuDomainObjPrivatePtr priv; -virQEMUDriverConfigPtr cfg = NULL; +VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; virCheckFlags(VIR_MEMORY_VIRTUAL | VIR_MEMORY_PHYSICAL, -1); @@ -12192,7 +12188,6 @@ qemuDomainMemoryPeek(virDomainPtr dom, unlink(tmp); VIR_FREE(tmp); virDomainObjEndAPI(&vm); -virObjectUnref(cfg); return ret; } @@ -12409,7 +12404,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; virDomainDiskDefPtr disk; -virQEMUDriverConfigPtr cfg = NULL; +VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuBlockStatsPtr entry = NULL; virCheckFlags(0, -1); @@ -12498,7 +12493,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom, cleanup: VIR_FREE(entry); virDomainObjEndAPI(&vm); -virObjectUnref(cfg); return ret; } @@ -12970,7 +12964,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, unsigned int flags) { virQEMUDriverPtr driver = dconn->privateData; -virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; @@ -13044,7 +13038,6 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, VIR_FREE(migrate_disks); VIR_FREE(origname); virDomainDefFree(def); -virObjectUnref(cfg); return ret; } @@ -14614,7 +14607,7 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, virCommandPtr cmd = NULL; const char *qemuImgPath; vir