[libvirt] [PATCH] rpc: Initialize a worker pool for max_workers=0 as well

Marc Hartmayer posted 1 patch 5 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180807113800.462-1-mhartmay@linux.ibm.com
Test syntax-check passed
src/rpc/virnetserver.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
[libvirt] [PATCH] rpc: Initialize a worker pool for max_workers=0 as well
Posted by Marc Hartmayer 5 years, 8 months ago
Semantically, there is no difference between an uninitialized worker
pool and an initialized worker pool with zero workers. Let's allow the
worker pool to be initialized for max_workers=0 as well then which
makes the API more symmetric and simplifies code. Validity of the
worker pool is delegated to virThreadPoolGetMaxWorkers instead.

This patch fixes segmentation faults in
virNetServerGetThreadPoolParameters and
virNetServerSetThreadPoolParameters for the case when no worker pool
is actually initialized (max_workers=0).

Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
---
 src/rpc/virnetserver.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index c26637ed031d..b4461b3803cf 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -205,7 +205,7 @@ virNetServerDispatchNewMessage(virNetServerClientPtr client,
     virObjectRef(srv);
     virObjectUnlock(srv);
 
-    if (srv->workers) {
+    if (virThreadPoolGetMaxWorkers(srv->workers) > 0)  {
         virNetServerJobPtr job;
 
         if (VIR_ALLOC(job) < 0)
@@ -367,8 +367,7 @@ virNetServerPtr virNetServerNew(const char *name,
     if (!(srv = virObjectLockableNew(virNetServerClass)))
         return NULL;
 
-    if (max_workers &&
-        !(srv->workers = virThreadPoolNew(min_workers, max_workers,
+    if (!(srv->workers = virThreadPoolNew(min_workers, max_workers,
                                           priority_workers,
                                           virNetServerHandleJob,
                                           srv)))
@@ -579,21 +578,18 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv)
         goto error;
 
     if (virJSONValueObjectAppendNumberUint(object, "min_workers",
-                                           srv->workers == NULL ? 0 :
                                            virThreadPoolGetMinWorkers(srv->workers)) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Cannot set min_workers data in JSON document"));
         goto error;
     }
     if (virJSONValueObjectAppendNumberUint(object, "max_workers",
-                                           srv->workers == NULL ? 0 :
                                            virThreadPoolGetMaxWorkers(srv->workers)) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Cannot set max_workers data in JSON document"));
         goto error;
     }
     if (virJSONValueObjectAppendNumberUint(object, "priority_workers",
-                                           srv->workers == NULL ? 0 :
                                            virThreadPoolGetPriorityWorkers(srv->workers)) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Cannot set priority_workers data in JSON document"));
-- 
2.13.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpc: Initialize a worker pool for max_workers=0 as well
Posted by John Ferlan 5 years, 8 months ago

On 08/07/2018 07:38 AM, Marc Hartmayer wrote:
> Semantically, there is no difference between an uninitialized worker
> pool and an initialized worker pool with zero workers. Let's allow the
> worker pool to be initialized for max_workers=0 as well then which
> makes the API more symmetric and simplifies code. Validity of the
> worker pool is delegated to virThreadPoolGetMaxWorkers instead.
> 
> This patch fixes segmentation faults in
> virNetServerGetThreadPoolParameters and
> virNetServerSetThreadPoolParameters for the case when no worker pool
> is actually initialized (max_workers=0).
> 
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
>  src/rpc/virnetserver.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 

Had to go refresh my memory on the previous series ;-)...

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

(and pushed)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpc: Initialize a worker pool for max_workers=0 as well
Posted by Marc Hartmayer 5 years, 8 months ago
On Tue, Aug 14, 2018 at 06:19 PM +0200, John Ferlan <jferlan@redhat.com> wrote:
> On 08/07/2018 07:38 AM, Marc Hartmayer wrote:
>> Semantically, there is no difference between an uninitialized worker
>> pool and an initialized worker pool with zero workers. Let's allow the
>> worker pool to be initialized for max_workers=0 as well then which
>> makes the API more symmetric and simplifies code. Validity of the
>> worker pool is delegated to virThreadPoolGetMaxWorkers instead.
>> 
>> This patch fixes segmentation faults in
>> virNetServerGetThreadPoolParameters and
>> virNetServerSetThreadPoolParameters for the case when no worker pool
>> is actually initialized (max_workers=0).
>> 
>> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
>> ---
>>  src/rpc/virnetserver.c | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>> 
>
> Had to go refresh my memory on the previous series ;-)...
>
> Reviewed-by: John Ferlan <jferlan@redhat.com>
>
> John
>
> (and pushed)
>

Thanks :)

-- 
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list