qemu-block
[Top][All Lists]
Advanced

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

Re: [PATCH] tests/qemu_iotests: Minimize usage of used ports


From: Eric Blake
Subject: Re: [PATCH] tests/qemu_iotests: Minimize usage of used ports
Date: Mon, 3 Feb 2020 09:32:29 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 2/3/20 1:59 AM, Lukáš Doktor wrote:
Using a range of ports from 32768 to 65538 is dangerous as some
application might already be listening there and interfere with the
testing. There is no way to reserve ports, but let's decrease the chance
of interactions by only using ports that were free at the time of
importing this module.

Without this patch CI occasionally fails with used ports. Additionally I
tried listening on the first port to be tried via "nc -l localhost
$port" and no matter how many other ports were available it always
hanged for infinity.

Signed-off-by: Lukáš Doktor <address@hidden>
---
  tests/qemu-iotests/147        | 43 ++++++++++++++++-------
  tests/qemu-iotests/iotests.py | 64 +++++++++++++++++++++++++++++++++++
  2 files changed, 94 insertions(+), 13 deletions(-)

Is it worth sharing the logic already present in common.nbd's nbd_server_start_tcp_socket shell function? (Oh right, that's shell, this is python). It seems like we keep reinventing logic to pick a safe port.


diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
index 2b6f859a09..4d0e1418bb 100755
--- a/tests/qemu-iotests/147
+++ b/tests/qemu-iotests/147
@@ -26,10 +26,8 @@ import time
  import iotests
  from iotests import cachemode, aiomode, imgfmt, qemu_img, qemu_nbd, 
qemu_nbd_early_pipe
-NBD_PORT_START = 32768
-NBD_PORT_END        = NBD_PORT_START + 1024
-NBD_IPV6_PORT_START = NBD_PORT_END
-NBD_IPV6_PORT_END   = NBD_IPV6_PORT_START + 1024
+NBD_PORTS = iotests.find_free_ports(32768, 65536, 1024)
+NBD_IPV6_PORTS = iotests.find_free_ports(NBD_PORTS[-1] + 1, 65536, 1024)

The changes here look sane...

+++ b/tests/qemu-iotests/iotests.py
@@ -20,6 +20,7 @@ from __future__ import print_function
  import errno
  import os
  import re
+import socket
  import subprocess
  import string
  import unittest
@@ -75,6 +76,69 @@ luks_default_secret_object = 'secret,id=keysec0,data=' + \
  luks_default_key_secret_opt = 'key-secret=keysec0'
+def is_port_free(port, address):

...and I'm glad you're adding a reusable method here.

+    """
+    Return True if the given port is available for use.
+
+    Currently we only check for TCP/UDP connections on IPv4/6
+    Backported from avocado.utils.network
+
+    :param port: Port number
+    :param address: Socket address to bind or connect
+    """
+    families = (socket.AF_INET, socket.AF_INET6)
+    if address == "localhost" or not address:
+        localhost = True
+        protocols = (socket.SOCK_STREAM, socket.SOCK_DGRAM)
+    else:
+        localhost = False
+        # sock.connect always connects for UDP
+        protocols = (socket.SOCK_STREAM, )
+    sock = None
+    try:
+        for family in families:
+            for protocol in protocols:
+                try:
+                    sock = socket.socket(family, protocol)
+                    if localhost:
+                        sock.bind(("", port))
+                    else:
+                        sock.connect((address, port))
+                        return False
+                except socket.error as exc:
+                    if exc.errno in (93, 94):   # Unsupported combinations

Ouch - that seems rather hard-coded (not all the world uses the same errno values as Linux). Does python have symbolic names for EPROTONOSUPPORT and ESOCKTNOSUPPORT?

+                        continue
+                    if localhost:
+                        return False
+                sock.close()
+        return True
+    finally:
+        if sock is not None:
+            sock.close()
+
+
+def find_free_ports(start_port, end_port, count, address="localhost"):
+    """
+    Return count of host free ports in the range [start_port, end_port].
+
+    Backported from avocado.utils.network
+
+    :param start_port: header of candidate port range
+    :param end_port: ender of candidate port range

s/ender/end/

+    :param count: Initial number of ports known to be free in the range.
+    :param address: Socket address to bind or connect
+    """
+    ports = []
+    port_range = range(start_port, end_port)
+    for i in port_range:
+        if is_port_free(i, address):
+            ports.append(i)
+        if len(ports) >= count:
+            break
+
+    return ports
+
+
  def qemu_img(*args):
      '''Run qemu-img and return the exit code'''
      devnull = open('/dev/null', 'r+')


I'd like a second review on this (my python is tolerable, but not strong), but I'm happy to take it through my NBD tree if no one else speaks up first.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




reply via email to

[Prev in Thread] Current Thread [Next in Thread]