[libvirt] [PATCH] virPortAllocatorSetUsed: ignore port 0

Ján Tomko posted 1 patch 5 years, 3 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/5e4b345914ec080a940ade65e40a930a112e6836.1548082855.git.jtomko@redhat.com
src/util/virportallocator.c | 3 +++
1 file changed, 3 insertions(+)
[libvirt] [PATCH] virPortAllocatorSetUsed: ignore port 0
Posted by Ján Tomko 5 years, 3 months ago
Similar to what commit 86dba8f3 did for virPortAllocatorRelease,
ignore port 0 in virPortAllocatorSetUsed.

For all the reasonable use cases the callers already check that
the port is non-zero, however if the port from the XML overflows
unsigned short and turns into 0, it can be set as used by
virPortAllocatorSetUsed but not released by virPortAllocatorRelease.

Also skip port '0' in virPortAllocatorSetUsed to make this behavior
symmetric.

The serenity was disturbed by commit 5dbda5e9 which started using
virPortAllocatorRelease instead of virPortAllocatorSetUsed (false).

https://bugzilla.redhat.com/show_bug.cgi?id=1591645

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/util/virportallocator.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index db95a601c7..d48963c1ff 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -294,6 +294,9 @@ virPortAllocatorSetUsed(unsigned short port)
     if (!pa)
         return -1;
 
+    if (!port)
+        return 0;
+
     virObjectLock(pa);
 
     if (virBitmapIsBitSet(pa->bitmap, port) ||
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virPortAllocatorSetUsed: ignore port 0
Posted by Cole Robinson 5 years, 3 months ago
On 01/21/2019 10:00 AM, Ján Tomko wrote:
> Similar to what commit 86dba8f3 did for virPortAllocatorRelease,
> ignore port 0 in virPortAllocatorSetUsed.
> 
> For all the reasonable use cases the callers already check that
> the port is non-zero, however if the port from the XML overflows
> unsigned short and turns into 0, it can be set as used by
> virPortAllocatorSetUsed but not released by virPortAllocatorRelease.
> 
> Also skip port '0' in virPortAllocatorSetUsed to make this behavior
> symmetric.
> 
> The serenity was disturbed by commit 5dbda5e9 which started using
> virPortAllocatorRelease instead of virPortAllocatorSetUsed (false).
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1591645
> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  src/util/virportallocator.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
> index db95a601c7..d48963c1ff 100644
> --- a/src/util/virportallocator.c
> +++ b/src/util/virportallocator.c
> @@ -294,6 +294,9 @@ virPortAllocatorSetUsed(unsigned short port)
>      if (!pa)
>          return -1;
>  
> +    if (!port)
> +        return 0;
> +
>      virObjectLock(pa);
>  
>      if (virBitmapIsBitSet(pa->bitmap, port) ||
> 

Reviewed-by: Cole Robinson <crobinso@redhat.com>

There's a portallocator unit test file but no direct tests for this
function unfortunately

- Cole

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