1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH v3 00/10] use VIR_AUTO*/g_auto* all around qemu_driver.c

Daniel Henrique Barboza posted 10 patches 6 years, 3 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20191015200852.15778-1-danielhb413@gmail.com
src/qemu/qemu_driver.c | 755 ++++++++++++++---------------------------
1 file changed, 259 insertions(+), 496 deletions(-)
[libvirt] [PATCH v3 00/10] use VIR_AUTO*/g_auto* all around qemu_driver.c
Posted by Daniel Henrique Barboza 6 years, 3 months ago 
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
Re: [libvirt] [PATCH v3 00/10] use VIR_AUTO*/g_auto* all around qemu_driver.c
Posted by Daniel Henrique Barboza 6 years, 3 months ago 
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
Re: [libvirt] [PATCH v3 00/10] use VIR_AUTO*/g_auto* all around qemu_driver.c
Posted by Ján Tomko 6 years, 3 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.