[Libvir] Remote daemon & virDomainFree interaction
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. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
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. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
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 ! A couple more may be needed and not in the list for the upcoming DomainMigrate entry points, +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Tue, Jul 24, 2007 at 03:29:40AM -0400, Daniel Veillard wrote:
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 !
It is not a permanent leak - when the virConnectPtr object is free'd the virDomainPtr objects are all freed too. Fortunately the virConnectPtr is free'd whenever the client disconnects. It causes problems if a single client creates a VM, destroys it and creates another with the same name though, because you get different UUID. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Yeah, good one. +1 One process per connection would reduce the impact of bugs similar to this. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones