Re: [libvirt] [PATCH v3 1/5] Add a threadpool implementation

Hi Eric, Thanks for your careful review of these patches. I'll post v4 patches tomorrow fixing all problems you pointed out. > > daemon/libvirtd.c already has a notion of worker threads; I'm wondering > how much overlap there is between your implementation and that one. A > better proof that this would be a useful API addition would be to have > the next patch in the series convert libvirtd.c over to using this API. OK. Will be in v4. <...snip...> > > +int virWorkerPoolSetMaxWorker(struct virWorkerPool *pool, int maxWorker) > > +{ > > + if (maxWorker < 0) > > + return -1; > > + > > + pthread_mutex_lock(&pool->mutex); > > + pool->nMaxWorker = maxWorker; > > + pthread_mutex_unlock(&pool->mutex); > > Does this do the right thing if maxWorker < pool->nMaxWorker, or does it > silently lose existing workers? In the case maxWorker < pool->nMaxWorker and there are pool->nMaxWorker threads running, (pool->nMaxWorker - maxWorker) threads will exit after the new nMaxWorker set. <...snip...> > > + > > +typedef void (*virWorkerFunc)(void *); > > pthread_create() takes a function that can return void*. Should worker > functions be allowed to return a value? threadpool doesn't care the return value, neither it has no way to pass the return value to threadpool creator, so it's meaningless for worker functions to return a value. Another example is virThreadFunc which does't return a value neither.