Re: [PATCH 1/5] python/qemu/machine: add possibility to postpone qmp connect

On 29/07/2026 11.31, Vladimir Sementsov-Ogievskiy wrote:
> This is need to test cpr-transfer migration, as on started
> target QMP is not available until we call "migrate" on
> source.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> ---
>   python/qemu/machine/machine.py | 53 ++++++++++++++++++----------------
>   1 file changed, 28 insertions(+), 25 deletions(-)
> 
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index bfb59d4c0bf..00154b8591f 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -449,17 +449,17 @@ def _post_shutdown(self) -> None:
>           self._user_killed = False
>           self._launched = False
>   
> -    def launch(self) -> None:
> +    def launch(self, do_start_process=True, do_qmp_connect=True) -> None:

Could you drop the "do_" prefix from the parameter names? ... that does not 
provide any additional value here, I think.

>           """
>           Launch the VM and make sure we cleanup and expose the
>           command line/output in case of exception
>           """
>   
> -        if self._launched:
> +        if self._launched and do_start_process:
>               raise QEMUMachineError('VM already launched')
>   
>           try:
> -            self._launch()
> +            self._launch(do_start_process, do_qmp_connect)
>           except BaseException as exc:
>               # We may have launched the process but it may
>               # have exited before we could connect via QMP.
> @@ -482,31 +482,34 @@ def launch(self) -> None:
>               # that exception. However, we still want to clean up.
>               raise
>   
> -    def _launch(self) -> None:
> +    def _launch(self, do_start_process=True, do_qmp_connect=True) -> None:
>           """
>           Launch the VM and establish a QMP connection
>           """
> -        self._pre_launch()
> -        LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
> -        # Log a simplified, developer-runnable command:
> -        # Exclude harness-managed infrastructure args (harness_args)
> -        # and wrapper.
> -        debug_cmd = [self._binary]
> -        debug_cmd.extend(self._console_args(interactive=True))
> -        debug_cmd.extend(self._base_args)
> -        debug_cmd.extend(self._args)
> -        LOG.debug('Developer-runnable command: %r', ' '.join(debug_cmd))
> -
> -        # Cleaning up of this subprocess is guaranteed by _do_shutdown.
> -        # pylint: disable=consider-using-with
> -        self._popen = subprocess.Popen(self._qemu_full_args,
> -                                       stdin=subprocess.DEVNULL,
> -                                       stdout=self._qemu_log_file,
> -                                       stderr=subprocess.STDOUT,
> -                                       shell=False,
> -                                       close_fds=False)
> -        self._launched = True
> -        self._post_launch()
> +        if do_start_process:
> +            self._pre_launch()
> +            LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
> +            # Log a simplified, developer-runnable command:
> +            # Exclude harness-managed infrastructure args (harness_args)
> +            # and wrapper.
> +            debug_cmd = [self._binary]
> +            debug_cmd.extend(self._console_args(interactive=True))
> +            debug_cmd.extend(self._base_args)
> +            debug_cmd.extend(self._args)
> +            LOG.debug('Developer-runnable command: %r', ' '.join(debug_cmd))
> +
> +            # Cleaning up of this subprocess is guaranteed by _do_shutdown.
> +            # pylint: disable=consider-using-with
> +            self._popen = subprocess.Popen(self._qemu_full_args,
> +                                           stdin=subprocess.DEVNULL,
> +                                           stdout=self._qemu_log_file,
> +                                           stderr=subprocess.STDOUT,
> +                                           shell=False,
> +                                           close_fds=False)
> +            self._launched = True
> +
> +        if do_qmp_connect:
> +            self._post_launch()
Honestly, I think this is quite confusing that callers now have to run 
"launch()" twice if they want to have a staged initialization.

Could you maybe drop the do_start_process parameter, just keep the 
qmp_connect parameter here. Then wrap the QMP connection stuff in a new 
separate function. So callers could do:

    vm.launch(qmp_connect=False)
    ... do other stuff ...
    vm.qmp_connect()
    # instead of vm.launch(do_start_process=False, do_qmp_connect=True)

WDYT?

  Thomas