In looking at a problem with domain object cleanup in virt-manager I came across a problem in the remote driver, well the internal driver API itself actually. Specifically the implmenetation of virDomainFree() never calls into the driver API - it simply uses virFreeDomain() release the memory associated with the virDomainPtr object. Couple this with the remote driver though, and virDomainPtr objects in the remote daemon never get released, because the virDomainFree call is never propagated over the wire to the server. Its quite easy to see this in practice. Simply add a printf to the impl of virDomainLookupByName which prints out the ref count. Then run either virsh or virt-manager for a while Get info QEMUGuest1 69 c7a5fdbd-edaf-9455-926a-d65c16db1809 Get info QEMUGuest1 70 c7a5fdbd-edaf-9455-926a-d65c16db1809 Get info QEMUGuest1 71 c7a5fdbd-edaf-9455-926a-d65c16db1809 We need to make virDomainFree call into the driver API, and also make sure that the remote driver implements it.

On Tue, Jul 24, 2007 at 03:41:32AM +0100, Daniel P. Berrange wrote: > On Tue, Jul 24, 2007 at 02:38:40AM +0100, Daniel P. Berrange wrote: > > In looking at a problem with domain object cleanup in virt-manager I came > > across a problem in the remote driver, well the internal driver API itself > > actually. Specifically the implmenetation of virDomainFree() never calls > > into the driver API - it simply uses virFreeDomain() release the memory > > associated with the virDomainPtr object. > > > > Couple this with the remote driver though, and virDomainPtr objects in the > > remote daemon never get released, because the virDomainFree call is never > > propagated over the wire to the server. > > > > Its quite easy to see this in practice. Simply add a printf to the impl > > of virDomainLookupByName which prints out the ref count. Then run either > > virsh or virt-manager for a while > > > > Get info QEMUGuest1 69 c7a5fdbd-edaf-9455-926a-d65c16db1809 > > Get info QEMUGuest1 70 c7a5fdbd-edaf-9455-926a-d65c16db1809 > > Get info QEMUGuest1 71 c7a5fdbd-edaf-9455-926a-d65c16db1809 > > > > We need to make virDomainFree call into the driver API, and also make sure > > that the remote driver implements it. > > Actually I was wrong in this. It is not just that the virDomainLookupByName > which cause virDomainPtr objects to be allocated. Every single API call into > the daemon which uses get_nonnull_domain() causes a virDomainPtr object > to have its ref count increased. So we don't want to pass the virDomainFree > API through the driver API at all. > > Instead in the server side of the remote driver, any API call which returns > a virDomainPtr *and* any call which accepts a virDomainPtr needs to have a > corresponding call to virDomainFree to reduce the ref-count in all of its > exit paths. The same goes for virNetwork objects & its calls. > > I'm attaching a patch which I believe hits all the neccessary codepaths. > With a few prints in virGetDomain, I can see the refcount correctly being > incremented & decremented per API call, rather than growing without bound. Dohh, giant memory leak ! Good catch !