On Thu, Nov 11, 2010 at 04:55:03PM +0000, Daniel P. Berrange wrote:
On Thu, Nov 11, 2010 at 11:44:42AM -0500, Dave Allan wrote:
On Thu, Nov 11, 2010 at 04:20:43PM +0000, Daniel P. Berrange wrote:
On Fri, Oct 29, 2010 at 12:52:18PM +0100, Daniel P. Berrange wrote:
The virConnectPtr struct will cache instances of all other objects. APIs like virDomainLookupByUUID will return a cached object, so if you do virDomainLookupByUUID twice in a row, you'll get the same exact virDomainPtr instance.
This does not have any performance benefit, since the actual logic in virDomainLookupByUUID (and other APIs returning virDomainPtr, etc instances) is not short-circuited. All it does is to ensure there is only one single virDomainPtr in existance for any given UUID.
The caching has a number of downsides though, all relating to stale data. If APIs aren't careful to always overwrite the 'id' field in virDomainPtr it may become out of data. Likewise for the name field, if a guest is renamed, or if a guest is deleted, and then a new one created with the same UUID but different name.
This has been an ongoing, endless source of bugs for all applications using libvirt from languages with garbage collection, causing them to get virDomainPtr instances from long ago with stale data.
The caching is also a waste of memory resources, since both applications, and language bindings often maintain their own hashtable caches of object instances.
This patch removes all the hash table caching, so all APIs return brand new virDomainPtr (etc) object instances.
No one has any comments on this change I thought would be hugely controversial... ?
From what you've decribed, it seems like a good change. I would have thought that there would be a performance hit, but from what you've said that's not the case. Why did you think it would be controversial?
Technically it is a semantic API change, because you can no longer rely on two virDomainPtr for the same domain having the same pointer address. I think this is pretty unlikely to hurt anything, and any app relying on this can be argued to be broken, but you never know....
Ah, now I see. I agree with your assessment. I haven't reviewed the code, but ACK to the design. Dave