On 2/7/2023 4:23 PM, John Snow wrote: > On Tue, Feb 7, 2023 at 4:04 PM Steven Sistare <steven.sist...@oracle.com> > wrote: >> >> On 2/7/2023 3:28 PM, John Snow wrote: >>> On Tue, Feb 7, 2023 at 2:03 PM Steve Sistare <steven.sist...@oracle.com> >>> wrote: >>>> >>>> Provide reopen_qmp_connection() to reopen a closed monitor connection. >>>> This will be needed by cpr, because qemu exec closes the monitor socket. >>>> >>>> Signed-off-by: Steve Sistare <steven.sist...@oracle.com> >>>> Reviewed-by: John Snow <js...@redhat.com> >>>> --- >>>> python/qemu/machine/machine.py | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/python/qemu/machine/machine.py >>>> b/python/qemu/machine/machine.py >>>> index ef94dcf..557209a 100644 >>>> --- a/python/qemu/machine/machine.py >>>> +++ b/python/qemu/machine/machine.py >>>> @@ -501,6 +501,16 @@ def _close_qmp_connection(self) -> None: >>>> finally: >>>> self._qmp_connection = None >>>> >>>> + def reopen_qmp_connection(self) -> None: >>>> + """Close and re-open the QMP connection.""" >>>> + self._close_qmp_connection() >>>> + self._qmp_connection = QEMUMonitorProtocol( >>>> + self._monitor_address, >>>> + server=True, >>>> + nickname=self._name >>>> + ) >>>> + self._qmp.accept(self._qmp_timer) >>>> + >>>> def _early_cleanup(self) -> None: >>>> """ >>>> Perform any cleanup that needs to happen before the VM exits. >>>> -- >>>> 1.8.3.1 >>>> >>> >>> This code is still mechanically fine as far as I can tell, but I lost >>> the plot on why it's needed - Can you please elaborate for me on what >>> you mean by "qemu exec will close the socket"? >>> >>> (R-B still stands, but since I am rewriting machine.py to be natively >>> async, I should be aware of your new use case.) >> >> Sure, thanks for asking. >> >> During cpr (aka live update), the old qemu process directly execv's the new >> qemu binary. The monitor socket fd is marked cloexec, so the kernel closes >> it >> during execv. >> >> - Steve > > Oho, I see. > > I wonder if you are helped at all by some of Marc-Andre's recent > changes to use socketpair() for QMP connections with machine.py. > Depending on how those sockets are managed, it might be possible to > hold onto one of them without having it closed alongside the old QEMU > process. Or, if that doesn't help, double-check that it doesn't *hurt* > you either. It should be the default as of about a week ago now -- > instead of using a local unix socket, we use socketpair and pass the > FD to help alleviate some race conditions. > > (Again, I'm fine with this patch either way, just a heads up.)
Thanks. Preserving the monitor connection and state across exec requires more work though, because of buffering, partially processed messages, and other state. I tried and abandoned that a while ago. Re-opening has been robust. - Steve
