Re: [libvirt] [libvirt PATCHv6 1/1] add DHCP snooping
On 03/22/2012 04:49 PM, David Stevens wrote:
Stefan Berger/Watson/IBM wrote on 03/22/2012 03:04:53 PM:
>
> I have some concerns about the cancelation of the thread. It can
> hold the snoop lock and get cancelled while holding it. Next time
> that lock is grabbed we will get a deadlock...
>
The snoop lock is acquired in virNWFilterDHCPSnoopEnd(), which
is where the pthread_cancel() directly (for valid leases) or the
freeReq()/pthread_cancel() is done. So, the canceler has the lock
and the canceling thread can't have it, then.
The only other case I see is when the config goes invalid and
we exit the snooper loop; he frees the snoop_lock() before removing
his own hash entry, which will cancel the same thread doing
the remove. Again, it can't have the snoop lock when canceled.
Is there some other case you think I'm missing?
pthread_cancel() tends to imply that you are properly managing signal
blocking across threads; we haven't used it anywhere else in libvirt,
and I'm extremely wary of pulling it in now, as there's probably a lot
of subtle bugs that it would expose. Are you sure you can't do this in
some other manner without dragging in pthread_cancel()?
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)