mbox series

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

Message ID 20230928044943.849073-1-jsnow@redhat.com (mailing list archive)
Headers show
Series python/machine: use socketpair() for console socket | expand

Message

John Snow Sept. 28, 2023, 4:49 a.m. UTC
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(-)

Comments

Daniel P. Berrangé Sept. 28, 2023, 8:12 a.m. UTC | #1
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
John Snow Sept. 28, 2023, 1:23 p.m. UTC | #2
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 :|
>
>