Re: [libvirt PATCH] virThreadPoolExpand: Prevent expanding worker pool by zero

[Michal Prívozník <mprivozn@xxxxxxxxxx>]

On 7/12/21 12:57 PM, Tim Wiederhake wrote:
> On Mon, 2021-07-12 at 10:09 +0200, Peter Krempa wrote:
>> On Fri, Jul 09, 2021 at 15:43:06 +0200, Tim Wiederhake wrote:
>>> `virThreadPoolNewFull` may call `virThreadPoolExpand` with
>>> `prioWorkers` = 0.
>>
>> Could you elaborate in which situations this happens?
> 
> Sure!
> 

I'll just add that this happens by default only for admin server,
because it is explicitly called with prio_workers = 0:

    if (!(srvAdm = virNetServerNew("admin", 1,
                                   config->admin_min_workers,
                                   config->admin_max_workers,
                                   0,
                                   config->admin_max_clients,
                                   0,
                                   config->admin_keepalive_interval,
                                   config->admin_keepalive_count,
                                   remoteAdmClientNew,
                                   NULL,
                                   remoteAdmClientFree,
                                   dmn))) {

(see src/remote/remote_daemon.c:1091)

But since we allow users to configure number of workers (for other
servers too) it may happen with any server, not just admin.

I wonder how do you feel about guarding virThreadPoolExpand() with check
for #threads > 0, e.g. like this:

diff --git i/src/util/virthreadpool.c w/src/util/virthreadpool.c
index 9ddd86a679..18660576ef 100644
--- i/src/util/virthreadpool.c
+++ w/src/util/virthreadpool.c
@@ -250,7 +250,8 @@ virThreadPoolNewFull(size_t minWorkers,
     if (virThreadPoolExpand(pool, minWorkers, false) < 0)
         goto error;

-    if (virThreadPoolExpand(pool, prioWorkers, true) < 0)
+    if (prioWorkers > 0 &&
+        virThreadPoolExpand(pool, prioWorkers, true) < 0)
         goto error;

     return pool;

Michal