2010/6/16 Daniel P. Berrange <berrange@xxxxxxxxxx>: > On Sat, Jun 12, 2010 at 06:32:55PM +0200, Matthias Bolte wrote: >> Justin Clift reported a problem with adding virStoragePoolIsPersistent >> to virsh's pool-info command, resulting in a strange problem. Here's >> an example: >> >> virsh # pool-create-as images_dir3 dir - - - - "/home/images2" >> Pool images_dir3 created >> >> virsh # pool-info images_dir3 >> Name: images_dir3 >> UUID: 90301885-94eb-4ca7-14c2-f30b25a29a36 >> State: running >> Capacity: 395.20 GB >> Allocation: 30.88 GB >> Available: 364.33 GB >> >> virsh # pool-destroy images_dir3 >> Pool images_dir3 destroyed >> >> At this point the images_dir3 pool should be gone (because it was >> transient) and we should be able to create a new pool with the same name: >> >> virsh # pool-create-as images_dir3 dir - - - - "/home/images2" >> Pool images_dir3 created >> >> virsh # pool-info images_dir3 >> Name: images_dir3 >> UUID: 90301885-94eb-4ca7-14c2-f30b25a29a36 >> error: Storage pool not found >> >> The new pool got the same UUID as the first one, but we didn't specify >> one. libvirt should have picked a random UUID, but it didn't. >> >> It turned out that virStoragePoolIsPersistent leaks a reference to the >> storage pool object (actually remoteDispatchStoragePoolIsPersistent does). >> As a result, pool-destroy doesn't remove the virStoragePool for the >> "images_dir3" pool from the virConnectPtr's storagePools hash on libvirtd's >> side. Then the second pool-create-as get's the stale virStoragePool object >> associated with the "images_dir3" name. But this object has the old UUID. >> >> This commit ensures that all get_nonnull_* and make_nonnull_* calls for >> libvirt objects are matched properly with vir*Free calls. This fixes the >> reference leaks and the reported problem. >> >> All remoteDispatch*IsActive and remoteDispatch*IsPersistent functions were >> affected. But also remoteDispatchDomainMigrateFinish2 was affected in the >> success path. I wonder why that didn't surface earlier. Probably because >> domainMigrateFinish2 is executed on the destination host and in the common >> case this connection is opened especially for the migration and gets closed >> after the migration is done. So there was no chance to run into a problem >> because of the leaked reference. > > ACK, looks good. > > Wonder why coverity hadn't flagged these memory leaks before.. > > Daniel > Probably because the reference leak doesn't result in a memory leak. The old virStoragePool objects in the virConnectPtr's storagePools hash are freed in virReleaseConnect. Thanks, pushed. Matthias -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list