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

On 14.08.26 10:06, Thomas Huth wrote:
> 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)\
Yes, that looks better, will do, thanks!

-- 
Best regards,
Vladimir