[libvirt] [RFC 0/1] convert virStorageSource to GObject

Daniel P. Berrangé berrange at redhat.com
Tue Oct 15 17:15:35 UTC 2019


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 at redhat.com>
> >    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.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




More information about the libvir-list mailing list