src/qemu/qemu_driver.c | 755 ++++++++++++++--------------------------- 1 file changed, 259 insertions(+), 496 deletions(-)
changes from v2: - rebased with newer master (67e72053c1) - added an extra patch to convert the existing VIR_AUTO* macros to g_auto* ones, all at once, to avoid the case where a method will have both VIR_AUTO* and g_auto* macros at the same time. Note: the conversion in patch 10 wasn't 100% due to a handful of methods that I was unable to use g_autoptr. Take for example the method qemuDomainSaveInternal: -- qemuDomainObjPrivatePtr priv = vm->privateData; VIR_AUTOUNREF(virCapsPtr) caps = NULL; virQEMUSaveDataPtr data = NULL; VIR_AUTOUNREF(qemuDomainSaveCookiePtr) cookie = NULL; -- Changing the 'cookie' variable to use g_autoptr() causes an error: make[3]: Entering directory '/home/danielhb/kvm-project/libvirt/src' CC qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9, from /usr/include/glib-2.0/glib/gtypes.h:32, from /usr/include/glib-2.0/glib/galloca.h:32, from /usr/include/glib-2.0/glib.h:30, from ./internal.h:31, from qemu/qemu_agent.h:24, from qemu/qemu_driver.c:40: qemu/qemu_driver.c: In function 'qemuDomainSaveInternal': qemu/qemu_driver.c:3282:15: error: unknown type name 'qemuDomainSaveCookie_autoptr' 3282 | g_autoptr(qemuDomainSaveCookie) cookie = NULL; I tried doing it with the 'Ptr' in the name, same error: CC qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9, from /usr/include/glib-2.0/glib/gtypes.h:32, from /usr/include/glib-2.0/glib/galloca.h:32, from /usr/include/glib-2.0/glib.h:30, from ./internal.h:31, from qemu/qemu_agent.h:24, from qemu/qemu_driver.c:40: qemu/qemu_driver.c: In function 'qemuDomainSaveInternal': qemu/qemu_driver.c:3282:15: error: unknown type name 'qemuDomainSaveCookiePtr_autoptr' 3282 | g_autoptr(qemuDomainSaveCookiePtr) cookie = NULL; Similar situation happens with qemuDomainSaveImageStartVM and with qemuSecurityInit methods. They are mentioned in the commit message of patch 10 for reference. These methods are still using VIR_AUTO* macros. changes from v1: - addressed review concerns made by Erik - added more cleanups, as suggested by Erik v2: https://www.redhat.com/archives/libvir-list/2019-September/msg01452.html v1: https://www.redhat.com/archives/libvir-list/2019-September/msg00719.html Daniel Henrique Barboza (10): qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 1/3 qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 2/3 qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 3/3 qemu_driver: use VIR_AUTOUNREF() with virCapsPtr qemu_driver: use VIR_AUTOUNREF() with more pointer types qemu_driver: use VIR_AUTOFREE() with strings 1/3 qemu_driver: use VIR_AUTOFREE() with strings 2/3 qemu_driver: use VIR_AUTOFREE() with strings 3/3 qemu_driver: VIR_AUTOFREE on other scalar pointers qemu_driver.c: use GLib macros src/qemu/qemu_driver.c | 755 ++++++++++++++--------------------------- 1 file changed, 259 insertions(+), 496 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
I just realized by reading a patch from Jano that VIR_AUTOFREE is simply an alias for g_autofree. This means that it is not worth to add more VIR_AUTOFREE() macros, and that it is ok to mix g_autofree with other VIR_ macros such as VIR_AUTOUNREF(). Jano is also removing VIR_AUTOFREE() from the code in his series, which will make this whole series obsolete. I'll re-send this series to not add more VIR_AUTOFREE() in any step of the way. DHB On 10/15/19 5:08 PM, Daniel Henrique Barboza wrote: > changes from v2: > - rebased with newer master (67e72053c1) > - added an extra patch to convert the existing VIR_AUTO* macros > to g_auto* ones, all at once, to avoid the case where a method > will have both VIR_AUTO* and g_auto* macros at the same time. > > > Note: the conversion in patch 10 wasn't 100% due to a handful of > methods that I was unable to use g_autoptr. Take for example > the method qemuDomainSaveInternal: > -- > qemuDomainObjPrivatePtr priv = vm->privateData; > VIR_AUTOUNREF(virCapsPtr) caps = NULL; > virQEMUSaveDataPtr data = NULL; > VIR_AUTOUNREF(qemuDomainSaveCookiePtr) cookie = NULL; > -- > > Changing the 'cookie' variable to use g_autoptr() causes an error: > > make[3]: Entering directory '/home/danielhb/kvm-project/libvirt/src' > CC qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo > In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9, > from /usr/include/glib-2.0/glib/gtypes.h:32, > from /usr/include/glib-2.0/glib/galloca.h:32, > from /usr/include/glib-2.0/glib.h:30, > from ./internal.h:31, > from qemu/qemu_agent.h:24, > from qemu/qemu_driver.c:40: > qemu/qemu_driver.c: In function 'qemuDomainSaveInternal': > qemu/qemu_driver.c:3282:15: error: unknown type name 'qemuDomainSaveCookie_autoptr' > 3282 | g_autoptr(qemuDomainSaveCookie) cookie = NULL; > > > I tried doing it with the 'Ptr' in the name, same error: > > > CC qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo > In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9, > from /usr/include/glib-2.0/glib/gtypes.h:32, > from /usr/include/glib-2.0/glib/galloca.h:32, > from /usr/include/glib-2.0/glib.h:30, > from ./internal.h:31, > from qemu/qemu_agent.h:24, > from qemu/qemu_driver.c:40: > qemu/qemu_driver.c: In function 'qemuDomainSaveInternal': > qemu/qemu_driver.c:3282:15: error: unknown type name 'qemuDomainSaveCookiePtr_autoptr' > 3282 | g_autoptr(qemuDomainSaveCookiePtr) cookie = NULL; > > > Similar situation happens with qemuDomainSaveImageStartVM and with > qemuSecurityInit methods. They are mentioned in the commit message > of patch 10 for reference. These methods are still using VIR_AUTO* > macros. > > > changes from v1: > - addressed review concerns made by Erik > - added more cleanups, as suggested by Erik > > v2: https://www.redhat.com/archives/libvir-list/2019-September/msg01452.html > v1: https://www.redhat.com/archives/libvir-list/2019-September/msg00719.html > > Daniel Henrique Barboza (10): > qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 1/3 > qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 2/3 > qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 3/3 > qemu_driver: use VIR_AUTOUNREF() with virCapsPtr > qemu_driver: use VIR_AUTOUNREF() with more pointer types > qemu_driver: use VIR_AUTOFREE() with strings 1/3 > qemu_driver: use VIR_AUTOFREE() with strings 2/3 > qemu_driver: use VIR_AUTOFREE() with strings 3/3 > qemu_driver: VIR_AUTOFREE on other scalar pointers > qemu_driver.c: use GLib macros > > src/qemu/qemu_driver.c | 755 ++++++++++++++--------------------------- > 1 file changed, 259 insertions(+), 496 deletions(-) > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Oct 16, 2019 at 09:24:13AM -0300, Daniel Henrique Barboza wrote: >I just realized by reading a patch from Jano that VIR_AUTOFREE >is simply an alias for g_autofree. This means that it is not worth >to add more VIR_AUTOFREE() macros, and that it is ok to mix >g_autofree with other VIR_ macros such as VIR_AUTOUNREF(). I deleted VIR_AUTOUNREF already. All usage of VIR_AUTOFREE was also removed, but I missed that the macro is definied in viralloc.h, patch removing it is pending. Per our HACKING style: https://libvirt.org/hacking.html#glib while the alloc/free functions should be interchangeable, it is bad style to mix the VIR_ and g_ allocation macros in a single function. I converted the VIR_AUTO_* group that had a GLib alternative, leaving VIR_AUTOSTRINGLIST and VIR_AUTOCLOSE behind. But to convert VIR_APPEND_ELEMENT usage a more fine grained approach will be needed, since it might involve using the GLib container types. >Jano is >also removing VIR_AUTOFREE() from the code in his series, which >will make this whole series obsolete. > >I'll re-send this series to not add more VIR_AUTOFREE() in any >step of the way. > > >DHB > > >On 10/15/19 5:08 PM, Daniel Henrique Barboza wrote: >>changes from v2: >>- rebased with newer master (67e72053c1) >>- added an extra patch to convert the existing VIR_AUTO* macros >>to g_auto* ones, all at once, to avoid the case where a method >>will have both VIR_AUTO* and g_auto* macros at the same time. >> >> >>Note: the conversion in patch 10 wasn't 100% due to a handful of >>methods that I was unable to use g_autoptr. Take for example >>the method qemuDomainSaveInternal: >>-- >> qemuDomainObjPrivatePtr priv = vm->privateData; >> VIR_AUTOUNREF(virCapsPtr) caps = NULL; >> virQEMUSaveDataPtr data = NULL; >> VIR_AUTOUNREF(qemuDomainSaveCookiePtr) cookie = NULL; >>-- >> >>Changing the 'cookie' variable to use g_autoptr() causes an error: >> >>make[3]: Entering directory '/home/danielhb/kvm-project/libvirt/src' >> CC qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo >>In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9, >> from /usr/include/glib-2.0/glib/gtypes.h:32, >> from /usr/include/glib-2.0/glib/galloca.h:32, >> from /usr/include/glib-2.0/glib.h:30, >> from ./internal.h:31, >> from qemu/qemu_agent.h:24, >> from qemu/qemu_driver.c:40: >>qemu/qemu_driver.c: In function 'qemuDomainSaveInternal': >>qemu/qemu_driver.c:3282:15: error: unknown type name 'qemuDomainSaveCookie_autoptr' >> 3282 | g_autoptr(qemuDomainSaveCookie) cookie = NULL; >> IIUC the original plan was to use VIR_AUTOPTR for the types needing unref so the cleanup function was defined for them, but then VIR_AUTOUNREF was introduced which always used virObjectUnref. To use g_autoptr with other types, you need to define the CLEANUP function, see: commit df4986b51bc88b65ae7f453814963e4340b168f3 Define G_DEFINE_AUTOPTR_CLEANUP_FUNC for virDomainCheckpointDef My objective was to get rid of the VIR_AUTOUNREF usage, not sure whether introducing g_autoptr for already existing objects until we convert them to GObject. >> >>I tried doing it with the 'Ptr' in the name, same error: >> >> >> CC qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo >>In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9, >> from /usr/include/glib-2.0/glib/gtypes.h:32, >> from /usr/include/glib-2.0/glib/galloca.h:32, >> from /usr/include/glib-2.0/glib.h:30, >> from ./internal.h:31, >> from qemu/qemu_agent.h:24, >> from qemu/qemu_driver.c:40: >>qemu/qemu_driver.c: In function 'qemuDomainSaveInternal': >>qemu/qemu_driver.c:3282:15: error: unknown type name 'qemuDomainSaveCookiePtr_autoptr' >> 3282 | g_autoptr(qemuDomainSaveCookiePtr) cookie = NULL; >> >> >>Similar situation happens with qemuDomainSaveImageStartVM and with >>qemuSecurityInit methods. They are mentioned in the commit message >>of patch 10 for reference. These methods are still using VIR_AUTO* >>macros. >> >> >>changes from v1: >>- addressed review concerns made by Erik >>- added more cleanups, as suggested by Erik >> >>v2: https://www.redhat.com/archives/libvir-list/2019-September/msg01452.html >>v1: https://www.redhat.com/archives/libvir-list/2019-September/msg00719.html >> >>Daniel Henrique Barboza (10): >> qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 1/3 >> qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 2/3 >> qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 3/3 >> qemu_driver: use VIR_AUTOUNREF() with virCapsPtr >> qemu_driver: use VIR_AUTOUNREF() with more pointer types >> qemu_driver: use VIR_AUTOFREE() with strings 1/3 >> qemu_driver: use VIR_AUTOFREE() with strings 2/3 >> qemu_driver: use VIR_AUTOFREE() with strings 3/3 >> qemu_driver: VIR_AUTOFREE on other scalar pointers >> qemu_driver.c: use GLib macros Fixing newly added lines seems odd, especially since you'll be replacing the exact text that you added earlier. Jano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.