[Libguestfs] virt-v2v valgrind errors in libosinfo

[Pino Toscano]

On Tuesday, 14 April 2020 11:53:30 CEST Richard W.M. Jones wrote:
> I've suppressed some OCaml and libosinfo valgrind errors in virt-v2v.
> 
> The remaining valgrind errors are here:
> 
>   http://oirase.annexia.org/tmp/v2vvg/
> 
> They all seem to be basically the same.  But I couldn't work out if
> these are expected leaks in the libosinfo code (in which case we
> should suppress them),

I think this might have to do with the glib allocator (libosinfo is
based on glib, so it allocate using it), that allocates in chunks by
default to avoid fragmentation, IIRC. Do you still get the same issues
if you export G_SLICE=always-malloc when running valgrind?

> or if they are actual bugs because we are
> missing a true destructor here:
> 
>   https://github.com/libguestfs/virt-v2v/blob/8e870da79b5a61513f568b0b81c773084b8d7997/v2v/libosinfo-c.c#L91
> 
> Perhaps there's a reason why we cannot have a destructor, for example
> that the C database is supposed to hold references to the OS objects?

This is correct according to the way we use OsinfoOs so far, i.e. only
by using what OsinfoDb has, because OsinfoDb holds references on the
OsinfoOs objects. I talked with Fabiano (main libosinfo developer)
about this, and sadly OsinfoDb has also references to other objects
(like the devices) inside each OsinfoOs, so it is not possible to get
the ownership of all the OsinfoOs (g_object_ref) and then get rid of
the OsinfoDb (g_object_unref).

> Unfortunately we never free the database.

Hm it is never freed? Wouldn't that result in actual leaks, since
OsinfoDb_t_finalize (g_object_unref'ing the OsinfoDb) wouldn't be
called?

> It could be that to express
> this properly we'd need to expose (db, os) tuples to the OCaml garbage
> collector.

I thought about this, and according to knowledge this would be needed
only if we want osinfo_os objects alive even when the osinfo_db gets
"out of scope". Considering that the osinfo_db is kept basically
statically this should be fine.

> BTW the informational string given here seems to be wrong - copy and
> paste error?
> 
>   https://github.com/libguestfs/virt-v2v/blob/8e870da79b5a61513f568b0b81c773084b8d7997/v2v/libosinfo-c.c#L90

Oops yes. "Db" and "Os" sadly look too similar...

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20200414/704a1863/attachment.sig>