[PATCH 00/40] convert virObjects to GObject

Rafael Fonseca r4f4rfs at gmail.com
Wed Jun 3 11:49:54 UTC 2020


On Wed, Jun 3, 2020 at 11:38 AM Daniel P. Berrangé <berrange at redhat.com> wrote:
>
> On Fri, May 15, 2020 at 04:13:21PM +0100, Daniel P. Berrangé wrote:
> > On Fri, May 15, 2020 at 02:34:01PM +0200, Michal Privoznik wrote:
> > > On 5/13/20 1:56 PM, Rafael Fonseca wrote:
> > > > This patch series convert various simple instances of virObject to a
> > > > GObject equivalent.
> > >
> > > I'm sorry upfront for misusing your patchset and I'm also sorry for bringing
> > > this up again.
> > >
> > > I think we need to step back and re-evaluate whether this is worth it.
> > > GObject is horrible and frightening part of GLib. Not only one has to define
> > > empty functions, but we can't mix virObject and GObject. For instance:
> > > virObjectRef() called over GObject will corrupt memory.
> > >
> > > Worse, there is no way to check whether your patches converted everything
> > > (and clearly they did not because I see couple of tests crashing; or maybe
> > > they did at the time of send but now the code has changed - anyway, point
> > > proven).
> > >
> > > I started reviewing and stopped at the first patch realizing, I have no idea
> > > whether you converted every virObjectRef()/virObjectUnref() with
> > > corresponding glib call.
> > >
> > > I also wanted to write a cocci spatch that might at least identify places
> > > where that is happening, but apparently my spatch skills are poor.
> > >
> > > Does anybody have an idea how to verify these patches?
> >
> > My preferred option was to make virObject be a subclass of GObject.
> >
> > I tried this initially but hit a key problem - g_object_unref is void
> > and virObjectUnref returns bool - true if any refs still exist. We
> > rely on that for the virConnectPtr and virFDStream objects.
> >
> > I couldn't come up with a way to solve that at the time, but I've
> > just had another think and believe we can solve it using a thread
> > local set by the finalize. So I'll have another go at doing this
> > inheritance.
>
> My conversion  to make virObject a subclass of GObject has now merged,
> which eliminates the danger that Michael was concerned about.
>
> So we should be able to move forward with Rafael's patches now.

Cool. I'll rebase on master, run the CI and send a new version when
it's all green.


Att.
-- 
Rafael Fonseca





More information about the libvir-list mailing list