Richard W.M. Jones wrote:
On Tue, May 20, 2008 at 02:39:52PM +0100, Daniel P. Berrange wrote:
> I'm not entirely convinced yet - the code certainly suggests to me that
> we need to free these. Only a couple of lines further up we obtain a referenced
> object
>
> /* Get the domain and network, if set. */
> dom = err->dom ? get_nonnull_domain (conn, *err->dom) : NULL;
> net = err->net ? get_nonnull_network (conn, *err->net) : NULL;
>
> And __virRaiseError() does *not* own the reference after being called, thus
> we need to explicitly release the reference ourselves.
Agreed with Dan ... I'm not sure how the leaks can go away with the
other patch. OTOH the ref-counting stuff is complicated.
Rich.
I haven't really traced the code path in detail to figure out what
was taking place but I'll explain what I was seeing:
Connect to qemu driver with virsh. I have one defined vm currently
shutoff. Trying to 'shutdown' the vm seems to cause no reference
count leaks, and I can do it until I'm blue in the face. But once
I try to 'destroy' the domain, a reference leak pops up. Now every
subsequent 'destroy' or 'shutdown' command creates another leak (I
didn't try any other commands). I thought I traced it to the error
and remote.c leaks, and when fixed sequentially all problems went
away. But now I'm not so sure what state all the code was in.
I'll try this again from the original unpatched state and try to
figure out exactly what was happening.