On Wed, Apr 04, 2018 at 09:50:29AM +0000, Zhangzijian wrote:
> -----Original Message-----
> From: libvir-list-bounces(a)redhat.com [mailto:libvir-list-bounces@redhat.com]
> On Behalf Of Michal Privoznik
> Sent: Wednesday, April 04, 2018 5:21 PM
> To: zhangzijian (Cloud); libvir-list(a)redhat.com
> Cc: huangyong (Cloud)
> Subject: Re: [libvirt] hash: failed on concurrent iterating.
>
> On 04/04/2018 11:10 AM, Zhangzijian wrote:
> > In function 'virHashForEach/virHashSearch',
'table->iterating' prevent
> concurrent iterating. But most caller evoke it, after hold a read lock.
> > This will lead the second caller failed to iterate the table. So, the caller
should
> hold a write lock, then iterate the table.
>
> Looks like there's another thread discussing this issue:
>
Yes, the issue is still exist in v4.2.0.
Is it applicable to using write lock instead?
I hit this a month ago when working on mediated devices and creating a ton of
them. You can't replace it with write lock because it would kill the sole
purpose of having RW locks in our lists using hash tables in the first place.
IMHO the correct way to fix this is to get rid of the table->iterating flag
completely. In order to do that, you need to convert hash tables to lockable
objects so that even modules that don't define any lockable abstractions on top
of hash tables could benefit from this. There are several caveats like dealing
with NWFilter and for example the virHashForEach method which allows calling
the 'Remove' callback which of course requires a write lock, however, if you
issue a standard query, this could have been (and also should) achieved with a
read lock to allow concurrent reading of elements in the table. This would also
require basically having a *Locked version of each virHash API because the
existing APIs are being called both outside and inside of the module, so you
need to take into account that you might already be holding a lock, etc.
Erik