[PATCH v3 0/6] python/machine: use socketpair() for console socket

John Snow posted 6 patches 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230928044943.849073-1-jsnow@redhat.com
Maintainers: John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Beraldo Leal <bleal@redhat.com>, Ani Sinha <anisinha@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Wainer dos Santos Moschetta <wainersm@redhat.com>, "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
python/qemu/machine/console_socket.py      | 29 ++++++++---
python/qemu/machine/machine.py             | 58 +++++++++++++---------
python/qemu/machine/qtest.py               | 54 +++++++++++++++-----
tests/avocado/acpi-bits.py                 |  5 +-
tests/avocado/avocado_qemu/__init__.py     |  2 +-
tests/avocado/machine_aspeed.py            |  5 +-
tests/qemu-iotests/iotests.py              |  2 +-
tests/qemu-iotests/tests/copy-before-write |  3 +-
8 files changed, 104 insertions(+), 54 deletions(-)
[PATCH v3 0/6] python/machine: use socketpair() for console socket
Posted by John Snow 7 months ago
Like we did for the QMP socket, use socketpair() for the console socket
so that hopefully there isn't a race condition during early boot where
data might get dropped on the floor.

May or may not help with various race conditions where early console
output is not showing up in the logs and/or potentially being missed by
wait_for_console_pattern.

V3:
  - Rebased.

V2:
  - Fixed some Socket ownership/garbage collection problems
  - Fixed callers of now-dropped VM arguments/properties
  - added a dedicated sock_fd arg to ConsoleSocket()
  - now using socketpair() for qtest console, too.
  - dropped sock_dir arg from *all* machine.py classes
  - Tested quite a bit more thoroughly ...

CI: https://gitlab.com/jsnow/qemu/-/pipelines/1019123030

John Snow (6):
  python/machine: move socket setup out of _base_args property
  python/machine: close sock_pair in cleanup path
  python/console_socket: accept existing FD in initializer
  python/machine: use socketpair() for console connections
  python/machine: use socketpair() for qtest connection
  python/machine: remove unused sock_dir argument

 python/qemu/machine/console_socket.py      | 29 ++++++++---
 python/qemu/machine/machine.py             | 58 +++++++++++++---------
 python/qemu/machine/qtest.py               | 54 +++++++++++++++-----
 tests/avocado/acpi-bits.py                 |  5 +-
 tests/avocado/avocado_qemu/__init__.py     |  2 +-
 tests/avocado/machine_aspeed.py            |  5 +-
 tests/qemu-iotests/iotests.py              |  2 +-
 tests/qemu-iotests/tests/copy-before-write |  3 +-
 8 files changed, 104 insertions(+), 54 deletions(-)

-- 
2.41.0

Re: [PATCH v3 0/6] python/machine: use socketpair() for console socket
Posted by Daniel P. Berrangé 7 months ago
On Thu, Sep 28, 2023 at 12:49:37AM -0400, John Snow wrote:
> Like we did for the QMP socket, use socketpair() for the console socket
> so that hopefully there isn't a race condition during early boot where
> data might get dropped on the floor.
> 
> May or may not help with various race conditions where early console
> output is not showing up in the logs and/or potentially being missed by
> wait_for_console_pattern.
> 
> V3:
>   - Rebased.

V3 has R-B on every single patch already. Should this
just have been a PULL instead ?

> 
> V2:
>   - Fixed some Socket ownership/garbage collection problems
>   - Fixed callers of now-dropped VM arguments/properties
>   - added a dedicated sock_fd arg to ConsoleSocket()
>   - now using socketpair() for qtest console, too.
>   - dropped sock_dir arg from *all* machine.py classes
>   - Tested quite a bit more thoroughly ...
> 
> CI: https://gitlab.com/jsnow/qemu/-/pipelines/1019123030
> 
> John Snow (6):
>   python/machine: move socket setup out of _base_args property
>   python/machine: close sock_pair in cleanup path
>   python/console_socket: accept existing FD in initializer
>   python/machine: use socketpair() for console connections
>   python/machine: use socketpair() for qtest connection
>   python/machine: remove unused sock_dir argument
> 
>  python/qemu/machine/console_socket.py      | 29 ++++++++---
>  python/qemu/machine/machine.py             | 58 +++++++++++++---------
>  python/qemu/machine/qtest.py               | 54 +++++++++++++++-----
>  tests/avocado/acpi-bits.py                 |  5 +-
>  tests/avocado/avocado_qemu/__init__.py     |  2 +-
>  tests/avocado/machine_aspeed.py            |  5 +-
>  tests/qemu-iotests/iotests.py              |  2 +-
>  tests/qemu-iotests/tests/copy-before-write |  3 +-
>  8 files changed, 104 insertions(+), 54 deletions(-)
> 
> -- 
> 2.41.0
> 
> 

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 :|
Re: [PATCH v3 0/6] python/machine: use socketpair() for console socket
Posted by John Snow 7 months ago
On Thu, Sep 28, 2023, 4:12 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Thu, Sep 28, 2023 at 12:49:37AM -0400, John Snow wrote:
> > Like we did for the QMP socket, use socketpair() for the console socket
> > so that hopefully there isn't a race condition during early boot where
> > data might get dropped on the floor.
> >
> > May or may not help with various race conditions where early console
> > output is not showing up in the logs and/or potentially being missed by
> > wait_for_console_pattern.
> >
> > V3:
> >   - Rebased.
>
> V3 has R-B on every single patch already. Should this
> just have been a PULL instead ?
>

I guess just erring on the side of caution with some fresh patches for
patchew since it had been a while since V2 and I did have to rebase this.

I'll send the PR soon if there's no objections.


> >
> > V2:
> >   - Fixed some Socket ownership/garbage collection problems
> >   - Fixed callers of now-dropped VM arguments/properties
> >   - added a dedicated sock_fd arg to ConsoleSocket()
> >   - now using socketpair() for qtest console, too.
> >   - dropped sock_dir arg from *all* machine.py classes
> >   - Tested quite a bit more thoroughly ...
> >
> > CI: https://gitlab.com/jsnow/qemu/-/pipelines/1019123030
> >
> > John Snow (6):
> >   python/machine: move socket setup out of _base_args property
> >   python/machine: close sock_pair in cleanup path
> >   python/console_socket: accept existing FD in initializer
> >   python/machine: use socketpair() for console connections
> >   python/machine: use socketpair() for qtest connection
> >   python/machine: remove unused sock_dir argument
> >
> >  python/qemu/machine/console_socket.py      | 29 ++++++++---
> >  python/qemu/machine/machine.py             | 58 +++++++++++++---------
> >  python/qemu/machine/qtest.py               | 54 +++++++++++++++-----
> >  tests/avocado/acpi-bits.py                 |  5 +-
> >  tests/avocado/avocado_qemu/__init__.py     |  2 +-
> >  tests/avocado/machine_aspeed.py            |  5 +-
> >  tests/qemu-iotests/iotests.py              |  2 +-
> >  tests/qemu-iotests/tests/copy-before-write |  3 +-
> >  8 files changed, 104 insertions(+), 54 deletions(-)
> >
> > --
> > 2.41.0
> >
> >
>
> 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 :|
>
>