[libvirt] [PATCH 4/5] qemu: implement virConnectListAllDomains() for qemu driver

[Eric Blake]

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   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120524/c534171d/attachment-0001.sig>