[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]