On 05/24/2012 06:14 AM, Daniel P. Berrange wrote: >> I just realized: Since we are executing this under the driver lock, and >> no VM can change state until we let go of the driver lock, it is not >> necessary to lock vms in this loop. That will help things go faster in >> computing the list. > > Hmm, this feels slightly dangerous to me. Saying that is in effect saying > you can do reads on a virDomainObjPtr without locking. This would be ok > if it were impossible for the fields we're reading to be changed without > driver lock held. > > 1. Thread A lock(driver) > 2. Thread A lock(vm1) > 3. Thread A unlock(driver) > 4. Thread B lock(driver) > 5. Thread B ...starts getting the list of domains... > > Now consider Thread A changes the 'id' feld in a virDomainPtr eg due > to the guest shutting down. > > Won't thread B be doing unsafe reads of 'id' now ? > > The only way I can see that this is safe, is if we are sure that > every change of the 'name', 'uuid' and 'id' fields is protected > by the driver lock.
Okay, you may have a point about 'id'. But I still think 'name' and 'uuid' are safe - we hash virDomainObjPtr via the 'uuid', and changing the 'uuid' would invalidate the hash, and so it must only be done while holding driver lock. Likewise, we don't yet support the ability to change a 'name', other than deleting the old name and defining a new domain with the new name. But it's easier to be safe (albeit potentially slower) than it is to audit everyone else and ensure that they follow the rules. You've convinced me - let's keep the lock for now; the only way we could drop the lock here is if we refactor 'id', 'name', and 'uuid' to be accessible only through accessor functions that guarantee appropriate locking. -- Eric Blake ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
