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.
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 :|