[libvirt] [PATCH] replace last instances of close()

Eric Blake eblake at redhat.com
Tue Nov 16 15:56:25 UTC 2010


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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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


More information about the libvir-list mailing list