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.
--
Thanks,
Hu Tao