Re: [PATCH v2] tests/avocado: starts PhoneServer upfront

[Thomas Huth <thuth@redhat.com>]

On 11/03/2022 18.11, Beraldo Leal wrote:
> Race conditions can happen with the current code, because the port that
> was available might not be anymore by the time the server is started.
> 
> By setting the port to 0, PhoneServer it will use the OS default
> behavior to get a free port, then we save this information so we can
> later configure the guest.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Beraldo Leal <bleal@redhat.com>
> ---
>   tests/avocado/avocado_qemu/__init__.py | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
> index 9b056b5ce5..ac85e36a4d 100644
> --- a/tests/avocado/avocado_qemu/__init__.py
> +++ b/tests/avocado/avocado_qemu/__init__.py
> @@ -18,7 +18,7 @@
>   import uuid
>   
>   import avocado
> -from avocado.utils import cloudinit, datadrainer, network, process, ssh, vmimage
> +from avocado.utils import cloudinit, datadrainer, process, ssh, vmimage
>   from avocado.utils.path import find_command
>   
>   #: The QEMU build root directory.  It may also be the source directory
> @@ -602,9 +602,6 @@ def prepare_cloudinit(self, ssh_pubkey=None):
>           self.log.info('Preparing cloudinit image')
>           try:
>               cloudinit_iso = os.path.join(self.workdir, 'cloudinit.iso')
> -            self.phone_home_port = network.find_free_port()
> -            if not self.phone_home_port:
> -                self.cancel('Failed to get a free port')
>               pubkey_content = None
>               if ssh_pubkey:
>                   with open(ssh_pubkey) as pubkey:
> @@ -614,7 +611,7 @@ def prepare_cloudinit(self, ssh_pubkey=None):
>                             password=self.password,
>                             # QEMU's hard coded usermode router address
>                             phone_home_host='10.0.2.2',
> -                          phone_home_port=self.phone_home_port,
> +                          phone_home_port=self.phone_server.server_port,
>                             authorized_key=pubkey_content)
>           except Exception:
>               self.cancel('Failed to prepare the cloudinit image')
> @@ -625,6 +622,8 @@ def set_up_boot(self):
>           self.vm.add_args('-drive', 'file=%s' % path)
>   
>       def set_up_cloudinit(self, ssh_pubkey=None):
> +        self.phone_server = cloudinit.PhoneHomeServer(('0.0.0.0', 0),
> +                                                      self.name)
>           cloudinit_iso = self.prepare_cloudinit(ssh_pubkey)
>           self.vm.add_args('-drive', 'file=%s,format=raw' % cloudinit_iso)
>   
> @@ -635,7 +634,9 @@ def launch_and_wait(self, set_up_ssh_connection=True):
>                                                    logger=self.log.getChild('console'))
>           console_drainer.start()
>           self.log.info('VM launched, waiting for boot confirmation from guest')
> -        cloudinit.wait_for_phone_home(('0.0.0.0', self.phone_home_port), self.name)
> +        while not self.phone_server.instance_phoned_back:
> +            self.phone_server.handle_request()
> +
>           if set_up_ssh_connection:
>               self.log.info('Setting up the SSH connection')
>               self.ssh_connect(self.username, self.ssh_key)

Thank you very much, this indeed fixes my problem with find_free_port() in 
the related tests:

Tested-by: Thomas Huth <thuth@redhat.com>