On 11/16/2010 06:20 AM, Stefan Berger wrote: >>> Did I convince you? >> Yes& no. On the surface it looks ok, but I'm still a little worried >> about >> the implications, because I put this code in there to avoid some nasty >> re-entrancy problems. I guessing some later re-factoring might have fixed >> those problems making this code redundant. So I'll ACK for nwo, but if we >> notice any unusual behaviour with the monitor, then this is the place to >> look... >> > The re-entrancy that was previously avoided with the 'closed' flag is > now checked with the file descriptor being >= 0, both with the lock > being held, so no concurrency is possible. > > The most dangerous thing IMO is that someone changes the locking later > on for whatever reason. There really should be a comment on the > functions reading and writing from/to the mon->fd that they need to be > called with that lock held. Also that the 'mon-fd' is protected by the > existing lock may be worth a comment. > > I'd push unless you want me to add comments to the file descriptor and > functions requiring the lock being held. We need the comments; no one will remember to dig up this email conversation, but in-code documentation will remind us to be careful. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list