[PATCH v2 0/3] python/qemu/machine: fix potential hang in QMP accept

marcandre.lureau@redhat.com posted 3 patches 1 year, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220630123419.1019367-1-marcandre.lureau@redhat.com
Maintainers: John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Beraldo Leal <bleal@redhat.com>
There is a newer version of this series
python/qemu/machine/machine.py | 24 ++++++++++++++++--------
python/qemu/qmp/legacy.py      | 18 +++++++++++++++---
python/qemu/qmp/protocol.py    | 25 ++++++++++++++++++++-----
3 files changed, 51 insertions(+), 16 deletions(-)
[PATCH v2 0/3] python/qemu/machine: fix potential hang in QMP accept
Posted by marcandre.lureau@redhat.com 1 year, 10 months ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

As reported earlier by Richard Henderson ("virgl avocado hang" thread), avocado
tests may hang when QEMU exits before the QMP connection is established.

v2:
 - use a socketpair() for QMP (instead of async concurrent code from v1) as
   suggested by Daniel Berrange.
 - should not regress (hopefully)

Marc-André Lureau (3):
  python/qmp/protocol: add open_with_socket()
  python/qmp/legacy: make QEMUMonitorProtocol accept a socket
  python/qemu/machine: use socketpair() for QMP by default

 python/qemu/machine/machine.py | 24 ++++++++++++++++--------
 python/qemu/qmp/legacy.py      | 18 +++++++++++++++---
 python/qemu/qmp/protocol.py    | 25 ++++++++++++++++++++-----
 3 files changed, 51 insertions(+), 16 deletions(-)

-- 
2.37.0.rc0


Re: [PATCH v2 0/3] python/qemu/machine: fix potential hang in QMP accept
Posted by John Snow 1 year, 10 months ago
On Thu, Jun 30, 2022 at 8:34 AM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> As reported earlier by Richard Henderson ("virgl avocado hang" thread), avocado
> tests may hang when QEMU exits before the QMP connection is established.
>
> v2:
>  - use a socketpair() for QMP (instead of async concurrent code from v1) as
>    suggested by Daniel Berrange.
>  - should not regress (hopefully)
>
> Marc-André Lureau (3):
>   python/qmp/protocol: add open_with_socket()
>   python/qmp/legacy: make QEMUMonitorProtocol accept a socket
>   python/qemu/machine: use socketpair() for QMP by default
>
>  python/qemu/machine/machine.py | 24 ++++++++++++++++--------
>  python/qemu/qmp/legacy.py      | 18 +++++++++++++++---
>  python/qemu/qmp/protocol.py    | 25 ++++++++++++++++++++-----
>  3 files changed, 51 insertions(+), 16 deletions(-)
>
> --
> 2.37.0.rc0
>

For anything that touches python/qemu/qmp/*, may I please ask that you
submit them to https://gitlab.com/qemu-project/python-qemu-qmp ?

(I'll review them in the meantime on-list just in the interest of
moving things along.)

--js
Re: [PATCH v2 0/3] python/qemu/machine: fix potential hang in QMP accept
Posted by Marc-André Lureau 1 year, 10 months ago
Hi

On Fri, Jul 1, 2022 at 2:51 AM John Snow <jsnow@redhat.com> wrote:

> On Thu, Jun 30, 2022 at 8:34 AM <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Hi,
> >
> > As reported earlier by Richard Henderson ("virgl avocado hang" thread),
> avocado
> > tests may hang when QEMU exits before the QMP connection is established.
> >
> > v2:
> >  - use a socketpair() for QMP (instead of async concurrent code from v1)
> as
> >    suggested by Daniel Berrange.
> >  - should not regress (hopefully)
> >
> > Marc-André Lureau (3):
> >   python/qmp/protocol: add open_with_socket()
> >   python/qmp/legacy: make QEMUMonitorProtocol accept a socket
> >   python/qemu/machine: use socketpair() for QMP by default
> >
> >  python/qemu/machine/machine.py | 24 ++++++++++++++++--------
> >  python/qemu/qmp/legacy.py      | 18 +++++++++++++++---
> >  python/qemu/qmp/protocol.py    | 25 ++++++++++++++++++++-----
> >  3 files changed, 51 insertions(+), 16 deletions(-)
> >
> > --
> > 2.37.0.rc0
> >
>
> For anything that touches python/qemu/qmp/*, may I please ask that you
> submit them to https://gitlab.com/qemu-project/python-qemu-qmp ?
>
>
Ok


> (I'll review them in the meantime on-list just in the interest of
> moving things along.)
>

I was waiting for a review before updating the patches / moving to
python-qemu-qmp.

thanks

-- 
Marc-André Lureau
Re: [PATCH v2 0/3] python/qemu/machine: fix potential hang in QMP accept
Posted by John Snow 1 year, 4 months ago
On Mon, Jul 25, 2022 at 7:23 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Fri, Jul 1, 2022 at 2:51 AM John Snow <jsnow@redhat.com> wrote:
>>
>> On Thu, Jun 30, 2022 at 8:34 AM <marcandre.lureau@redhat.com> wrote:
>> >
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > Hi,
>> >
>> > As reported earlier by Richard Henderson ("virgl avocado hang" thread), avocado
>> > tests may hang when QEMU exits before the QMP connection is established.
>> >
>> > v2:
>> >  - use a socketpair() for QMP (instead of async concurrent code from v1) as
>> >    suggested by Daniel Berrange.
>> >  - should not regress (hopefully)
>> >
>> > Marc-André Lureau (3):
>> >   python/qmp/protocol: add open_with_socket()
>> >   python/qmp/legacy: make QEMUMonitorProtocol accept a socket
>> >   python/qemu/machine: use socketpair() for QMP by default
>> >
>> >  python/qemu/machine/machine.py | 24 ++++++++++++++++--------
>> >  python/qemu/qmp/legacy.py      | 18 +++++++++++++++---
>> >  python/qemu/qmp/protocol.py    | 25 ++++++++++++++++++++-----
>> >  3 files changed, 51 insertions(+), 16 deletions(-)
>> >
>> > --
>> > 2.37.0.rc0
>> >
>>
>> For anything that touches python/qemu/qmp/*, may I please ask that you
>> submit them to https://gitlab.com/qemu-project/python-qemu-qmp ?
>>
>
> Ok
>
>>
>> (I'll review them in the meantime on-list just in the interest of
>> moving things along.)
>
>
> I was waiting for a review before updating the patches / moving to python-qemu-qmp.
>

Fair enough - can I kindly ask you to resend, though? My apologies and
Happy New Year!

--js
Re: [PATCH v2 0/3] python/qemu/machine: fix potential hang in QMP accept
Posted by Marc-André Lureau 1 year, 4 months ago
Hi John

On Tue, Jan 10, 2023 at 1:06 AM John Snow <jsnow@redhat.com> wrote:
>
> On Mon, Jul 25, 2022 at 7:23 AM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi
> >
> > On Fri, Jul 1, 2022 at 2:51 AM John Snow <jsnow@redhat.com> wrote:
> >>
> >> On Thu, Jun 30, 2022 at 8:34 AM <marcandre.lureau@redhat.com> wrote:
> >> >
> >> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> >
> >> > Hi,
> >> >
> >> > As reported earlier by Richard Henderson ("virgl avocado hang" thread), avocado
> >> > tests may hang when QEMU exits before the QMP connection is established.
> >> >
> >> > v2:
> >> >  - use a socketpair() for QMP (instead of async concurrent code from v1) as
> >> >    suggested by Daniel Berrange.
> >> >  - should not regress (hopefully)
> >> >
> >> > Marc-André Lureau (3):
> >> >   python/qmp/protocol: add open_with_socket()
> >> >   python/qmp/legacy: make QEMUMonitorProtocol accept a socket
> >> >   python/qemu/machine: use socketpair() for QMP by default
> >> >
> >> >  python/qemu/machine/machine.py | 24 ++++++++++++++++--------
> >> >  python/qemu/qmp/legacy.py      | 18 +++++++++++++++---
> >> >  python/qemu/qmp/protocol.py    | 25 ++++++++++++++++++++-----
> >> >  3 files changed, 51 insertions(+), 16 deletions(-)
> >> >
> >> > --
> >> > 2.37.0.rc0
> >> >
> >>
> >> For anything that touches python/qemu/qmp/*, may I please ask that you
> >> submit them to https://gitlab.com/qemu-project/python-qemu-qmp ?
> >>
> >
> > Ok
> >
> >>
> >> (I'll review them in the meantime on-list just in the interest of
> >> moving things along.)
> >
> >
> > I was waiting for a review before updating the patches / moving to python-qemu-qmp.
> >
>
> Fair enough - can I kindly ask you to resend, though? My apologies and
> Happy New Year!

I am confused, what's the relation between QEMU python/qemu/qmp and
https://gitlab.com/qemu-project/python-qemu-qmp ?

When / how is the code sync ?

-- 
Marc-André Lureau
Re: [PATCH v2 0/3] python/qemu/machine: fix potential hang in QMP accept
Posted by John Snow 1 year, 4 months ago
On Tue, Jan 10, 2023 at 2:05 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi John
>
> On Tue, Jan 10, 2023 at 1:06 AM John Snow <jsnow@redhat.com> wrote:
> >
> > On Mon, Jul 25, 2022 at 7:23 AM Marc-André Lureau
> > <marcandre.lureau@gmail.com> wrote:
> > >
> > > Hi
> > >
> > > On Fri, Jul 1, 2022 at 2:51 AM John Snow <jsnow@redhat.com> wrote:
> > >>
> > >> On Thu, Jun 30, 2022 at 8:34 AM <marcandre.lureau@redhat.com> wrote:
> > >> >
> > >> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >> >
> > >> > Hi,
> > >> >
> > >> > As reported earlier by Richard Henderson ("virgl avocado hang" thread), avocado
> > >> > tests may hang when QEMU exits before the QMP connection is established.
> > >> >
> > >> > v2:
> > >> >  - use a socketpair() for QMP (instead of async concurrent code from v1) as
> > >> >    suggested by Daniel Berrange.
> > >> >  - should not regress (hopefully)
> > >> >
> > >> > Marc-André Lureau (3):
> > >> >   python/qmp/protocol: add open_with_socket()
> > >> >   python/qmp/legacy: make QEMUMonitorProtocol accept a socket
> > >> >   python/qemu/machine: use socketpair() for QMP by default
> > >> >
> > >> >  python/qemu/machine/machine.py | 24 ++++++++++++++++--------
> > >> >  python/qemu/qmp/legacy.py      | 18 +++++++++++++++---
> > >> >  python/qemu/qmp/protocol.py    | 25 ++++++++++++++++++++-----
> > >> >  3 files changed, 51 insertions(+), 16 deletions(-)
> > >> >
> > >> > --
> > >> > 2.37.0.rc0
> > >> >
> > >>
> > >> For anything that touches python/qemu/qmp/*, may I please ask that you
> > >> submit them to https://gitlab.com/qemu-project/python-qemu-qmp ?
> > >>
> > >
> > > Ok
> > >
> > >>
> > >> (I'll review them in the meantime on-list just in the interest of
> > >> moving things along.)
> > >
> > >
> > > I was waiting for a review before updating the patches / moving to python-qemu-qmp.
> > >
> >
> > Fair enough - can I kindly ask you to resend, though? My apologies and
> > Happy New Year!
>
> I am confused, what's the relation between QEMU python/qemu/qmp and
> https://gitlab.com/qemu-project/python-qemu-qmp ?
>
> When / how is the code sync ?
>

python-qemu-qmp supersedes the code that is in qemu.git.
qemu.git/python/qemu/qmp is scheduled to be deleted, but there are
changes that need to go in to configure etc to support the deletion
first, and I've been backlogged/waylaid on making those changes.

--js
Re: [PATCH v2 0/3] python/qemu/machine: fix potential hang in QMP accept
Posted by John Snow 1 year, 4 months ago
On Tue, Jan 10, 2023 at 12:45 PM John Snow <jsnow@redhat.com> wrote:
>
> On Tue, Jan 10, 2023 at 2:05 AM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi John
> >
> > On Tue, Jan 10, 2023 at 1:06 AM John Snow <jsnow@redhat.com> wrote:
> > >
> > > On Mon, Jul 25, 2022 at 7:23 AM Marc-André Lureau
> > > <marcandre.lureau@gmail.com> wrote:
> > > >
> > > > Hi
> > > >
> > > > On Fri, Jul 1, 2022 at 2:51 AM John Snow <jsnow@redhat.com> wrote:
> > > >>
> > > >> On Thu, Jun 30, 2022 at 8:34 AM <marcandre.lureau@redhat.com> wrote:
> > > >> >
> > > >> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > >> >
> > > >> > Hi,
> > > >> >
> > > >> > As reported earlier by Richard Henderson ("virgl avocado hang" thread), avocado
> > > >> > tests may hang when QEMU exits before the QMP connection is established.
> > > >> >
> > > >> > v2:
> > > >> >  - use a socketpair() for QMP (instead of async concurrent code from v1) as
> > > >> >    suggested by Daniel Berrange.
> > > >> >  - should not regress (hopefully)
> > > >> >
> > > >> > Marc-André Lureau (3):
> > > >> >   python/qmp/protocol: add open_with_socket()
> > > >> >   python/qmp/legacy: make QEMUMonitorProtocol accept a socket
> > > >> >   python/qemu/machine: use socketpair() for QMP by default
> > > >> >
> > > >> >  python/qemu/machine/machine.py | 24 ++++++++++++++++--------
> > > >> >  python/qemu/qmp/legacy.py      | 18 +++++++++++++++---
> > > >> >  python/qemu/qmp/protocol.py    | 25 ++++++++++++++++++++-----
> > > >> >  3 files changed, 51 insertions(+), 16 deletions(-)
> > > >> >
> > > >> > --
> > > >> > 2.37.0.rc0
> > > >> >
> > > >>
> > > >> For anything that touches python/qemu/qmp/*, may I please ask that you
> > > >> submit them to https://gitlab.com/qemu-project/python-qemu-qmp ?
> > > >>
> > > >
> > > > Ok
> > > >
> > > >>
> > > >> (I'll review them in the meantime on-list just in the interest of
> > > >> moving things along.)
> > > >
> > > >
> > > > I was waiting for a review before updating the patches / moving to python-qemu-qmp.
> > > >
> > >
> > > Fair enough - can I kindly ask you to resend, though? My apologies and
> > > Happy New Year!
> >
> > I am confused, what's the relation between QEMU python/qemu/qmp and
> > https://gitlab.com/qemu-project/python-qemu-qmp ?
> >
> > When / how is the code sync ?
> >
>
> python-qemu-qmp supersedes the code that is in qemu.git.
> qemu.git/python/qemu/qmp is scheduled to be deleted, but there are
> changes that need to go in to configure etc to support the deletion
> first, and I've been backlogged/waylaid on making those changes.

... by which I mean, I generally do review and merge on the standalone
repo first, then backport to qemu.git. Or, that's what I'd prefer to
do, since the tooling and testing is more advanced on the standalone
repo.
Re: [PATCH v2 0/3] python/qemu/machine: fix potential hang in QMP accept
Posted by Daniel P. Berrangé 1 year, 10 months ago
On Mon, Jul 25, 2022 at 03:23:26PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Jul 1, 2022 at 2:51 AM John Snow <jsnow@redhat.com> wrote:
> 
> > On Thu, Jun 30, 2022 at 8:34 AM <marcandre.lureau@redhat.com> wrote:
> > >
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Hi,
> > >
> > > As reported earlier by Richard Henderson ("virgl avocado hang" thread),
> > avocado
> > > tests may hang when QEMU exits before the QMP connection is established.
> > >
> > > v2:
> > >  - use a socketpair() for QMP (instead of async concurrent code from v1)
> > as
> > >    suggested by Daniel Berrange.
> > >  - should not regress (hopefully)
> > >
> > > Marc-André Lureau (3):
> > >   python/qmp/protocol: add open_with_socket()
> > >   python/qmp/legacy: make QEMUMonitorProtocol accept a socket
> > >   python/qemu/machine: use socketpair() for QMP by default
> > >
> > >  python/qemu/machine/machine.py | 24 ++++++++++++++++--------
> > >  python/qemu/qmp/legacy.py      | 18 +++++++++++++++---
> > >  python/qemu/qmp/protocol.py    | 25 ++++++++++++++++++++-----
> > >  3 files changed, 51 insertions(+), 16 deletions(-)
> > >
> > > --
> > > 2.37.0.rc0
> > >
> >
> > For anything that touches python/qemu/qmp/*, may I please ask that you
> > submit them to https://gitlab.com/qemu-project/python-qemu-qmp ?
> >
> >
> Ok
> 
> 
> > (I'll review them in the meantime on-list just in the interest of
> > moving things along.)
> >
> 
> I was waiting for a review before updating the patches / moving to
> python-qemu-qmp.

This code looks decent to me

  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|