On 10/15/19 2:15 PM, Daniel P. Berrangé wrote:
On Tue, Oct 15, 2019 at 10:51:39AM -0300, Daniel Henrique Barboza wrote:
On 10/15/19 9:55 AM, Daniel P. Berrangé wrote:
On Tue, Oct 15, 2019 at 09:42:45AM -0300, Daniel Henrique Barboza wrote:
I was hoping to quickly re-send the qemu_driver cleanups I've
sent some time ago, now using Glib. I started by attempting to
change the first VIR_AUTOUNREF() call in qemu_driver.c to
g_autoptr(), which happens to be a virStorageSourcePtr type,
then I realized that it wasn't that simple.
It should be that simple with this commit:
commit 667ff797e8eb8d82f30ab430216a8d2eef6b915a
Author: Daniel P. Berrangé <berrange@xxxxxxxxxx>
Date: Fri Oct 4 17:14:10 2019 +0100
src: add support for g_autoptr with virObject instances
we should be able to use g_autoptr for any virObject, without
having to lock-step convert to GObject.
What actual problem did you find ?
I failed to notice this commit. Just tried it again and it worked.
What happened yesterday was that I attempted to do a simple
VIR_AUTOUNREF -> g_autopt replace, faced compile errors and then, since
I didn't notice this commit about, I assumed "I guess I need to convert
this guy to GObject".
In fact, the compile error happened because g_autoptr() does not operate
with a 'Ptr' type - something that I learned only during the conversion
process.
Yeah, you need to drop the 'Ptr' suffix in the type name when
converting to g_autoptr, as it adds the pointer itself.
This is being sent as RFC because x-I am aware that docs/hacking.html
mentions that we shouldn't mix up certain GLib macros with Libvirt
ones, thus I am uncertain of whether I have messed up or not.
'make check' works, did a few sanity checks with libvirtd as
well.
Yes, the need to not mix g_auto* with VIR_AUTO*, is why I did commit
667ff797e8eb8d82f30ab430216a8d2eef6b915a to let you use g_autoptr
with virObject, without first converting to GObject.
What if there are other object types in the same function using the VIR
macros?
For example, inside qemu_driver.c: qemuDomainBlockCopyCommon:
VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg =
virQEMUDriverGetConfig(driver);
const char *format = NULL;
bool mirror_reuse = !!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT);
bool mirror_shallow = !!(flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW);
bool existing = mirror_reuse;
qemuBlockJobDataPtr job = NULL;
VIR_AUTOUNREF(virStorageSourcePtr) mirror = mirrorsrc;
bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
bool mirror_initialized = false;
VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL;
VIR_AUTOPTR(qemuBlockStorageSourceChainData) crdata = NULL;
Let's say that I change the virStorageSourcePtr up there to
g_autoptr(virStorageSource) mirror = mirrorsrc;
As long as there are no VIR macros acting in the 'mirror' variable, is it to
use g_autoptr
there even when everyone else is using VIR_AUTO* macros?
You should change all variables in the method at the same time.
Both the VIR_AUTOUNEF calls here can use g_autoptr, as can the
two VIR_AUTOPTR calls.
Thanks for clarifying. I was going to re-send the patches adding GLib
macros instead of VIR_AUTO* ones, which would end up breaking this
rule because these patches are changing stuff in smaller steps.
What I'll end up is to basically re-send them as they are now, but with an
extra patch to change everything to GLib at once. That way we'll stay
compliant every step of the way.
DHB
Regards,
Daniel
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
