Re: [libvirt] [PATCH v2 2/4] util: Allow port allocator to skip bind() check

[Jim Fehlig <jfehlig@suse.com>]

Daniel P. Berrange wrote:
> Test suites using the port allocator don't want to have different
> behaviour depending on whether a port is in use on the host. Add
> a VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK which test suites can use
> to skip the bind() test. The port allocator will thus only track
> ports in use by the test suite process itself. This is fine when
> using the port allocator to generate guest configs which won't
> actually be launched
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  src/libxl/libxl_driver.c     |  3 ++-
>  src/qemu/qemu_driver.c       |  9 ++++++---
>  src/util/virportallocator.c  | 14 ++++++++++----
>  src/util/virportallocator.h  |  7 ++++++-
>  tests/virportallocatortest.c |  4 ++--
>  5 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 515d5c9..70f9bb8 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -298,7 +298,8 @@ libxlStateInitialize(bool privileged,
>      if (!(libxl_driver->reservedVNCPorts =
>            virPortAllocatorNew(_("VNC"),
>                                LIBXL_VNC_PORT_MIN,
> -                              LIBXL_VNC_PORT_MAX)))
> +                              LIBXL_VNC_PORT_MAX,
> +                              0)))
>          goto error;
>  
>      if (!(libxl_driver->domains = virDomainObjListNew()))
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3a7622a..b3a9036 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -680,19 +680,22 @@ qemuStateInitialize(bool privileged,
>      if ((qemu_driver->remotePorts =
>           virPortAllocatorNew(_("display"),
>                               cfg->remotePortMin,
> -                             cfg->remotePortMax)) == NULL)
> +                             cfg->remotePortMax,
> +                             0)) == NULL)
>          goto error;
>  
>      if ((qemu_driver->webSocketPorts =
>           virPortAllocatorNew(_("webSocket"),
>                               cfg->webSocketPortMin,
> -                             cfg->webSocketPortMax)) == NULL)
> +                             cfg->webSocketPortMax,
> +                             0)) == NULL)
>          goto error;
>  
>      if ((qemu_driver->migrationPorts =
>           virPortAllocatorNew(_("migration"),
>                               cfg->migrationPortMin,
> -                             cfg->migrationPortMax)) == NULL)
> +                             cfg->migrationPortMax,
> +                             0)) == NULL)
>          goto error;
>  
>      if (qemuSecurityInit(qemu_driver) < 0)
> diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
> index b68133a..fa17475 100644
> --- a/src/util/virportallocator.c
> +++ b/src/util/virportallocator.c
> @@ -43,6 +43,8 @@ struct _virPortAllocator {
>  
>      unsigned short start;
>      unsigned short end;
> +
> +    unsigned int flags;
>  };
>  
>  static virClassPtr virPortAllocatorClass;
> @@ -71,7 +73,8 @@ VIR_ONCE_GLOBAL_INIT(virPortAllocator)
>  
>  virPortAllocatorPtr virPortAllocatorNew(const char *name,
>                                          unsigned short start,
> -                                        unsigned short end)
> +                                        unsigned short end,
> +                                        unsigned int flags)
>  {
>      virPortAllocatorPtr pa;
>  
> @@ -87,6 +90,7 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name,
>      if (!(pa = virObjectLockableNew(virPortAllocatorClass)))
>          return NULL;
>  
> +    pa->flags = flags;
>      pa->start = start;
>      pa->end = end;
>  
> @@ -193,9 +197,11 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
>          if (used)
>              continue;
>  
> -        if (virPortAllocatorBindToPort(&v6used, i, AF_INET6) < 0 ||
> -            virPortAllocatorBindToPort(&used, i, AF_INET) < 0)
> -            goto cleanup;
> +        if (!(pa->flags & VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK)) {
> +            if (virPortAllocatorBindToPort(&v6used, i, AF_INET6) < 0 ||
> +                virPortAllocatorBindToPort(&used, i, AF_INET) < 0)
> +                goto cleanup;
> +        }
>  
>          if (!used && !v6used) {
>              /* Add port to bitmap of reserved ports */
> diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h
> index c8aa6de..2fdb8d9 100644
> --- a/src/util/virportallocator.h
> +++ b/src/util/virportallocator.h
> @@ -28,9 +28,14 @@
>  typedef struct _virPortAllocator virPortAllocator;
>  typedef virPortAllocator *virPortAllocatorPtr;
>  
> +typedef enum {
> +    VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK = (1 << 0),
> +} virPortAllocatorFlags;
> +
>  virPortAllocatorPtr virPortAllocatorNew(const char *name,
>                                          unsigned short start,
> -                                        unsigned short end);
> +                                        unsigned short end,
> +                                        unsigned int flags);
>  
>  int virPortAllocatorAcquire(virPortAllocatorPtr pa,
>                              unsigned short *port);
> diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c
> index 48d2c9a..96d2ade 100644
> --- a/tests/virportallocatortest.c
> +++ b/tests/virportallocatortest.c
> @@ -122,7 +122,7 @@ VIR_LOG_INIT("tests.portallocatortest");
>  
>  static int testAllocAll(const void *args ATTRIBUTE_UNUSED)
>  {
> -    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5909);
> +    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5909, 0);
>   

Should these tests be changed to use the new
VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK flag?

Regards,
Jim

>      int ret = -1;
>      unsigned short p1, p2, p3, p4, p5, p6, p7;
>  
> @@ -193,7 +193,7 @@ static int testAllocAll(const void *args ATTRIBUTE_UNUSED)
>  
>  static int testAllocReuse(const void *args ATTRIBUTE_UNUSED)
>  {
> -    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5910);
> +    virPortAllocatorPtr alloc = virPortAllocatorNew("test", 5900, 5910, 0);
>      int ret = -1;
>      unsigned short p1, p2, p3, p4;
>  
>