qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 0/6] ui/dbus: Share one listener for a console


From: Marc-André Lureau
Subject: Re: [PATCH 0/6] ui/dbus: Share one listener for a console
Date: Mon, 14 Feb 2022 16:06:57 +0400

Hi Akihiko

On Sun, Feb 13, 2022 at 6:44 AM Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
ui/dbus required to have multiple DisplayChangeListeners (possibly with OpenGL)
for a console but that caused several problems:
- It broke egl-headless, an unusual display which implements OpenGL rendering
  for non-OpenGL displays. The code to support multiple DisplayChangeListeners
  does not consider the case where non-OpenGL displays listens OpenGL consoles.

Can you provide instructions on what broke? Even better write a test, please.

"make check-avocado AVOCADO_TESTS=tests/avocado/virtio-gpu.py", which covers egl-headless usage, works.
 
- Multiple OpenGL DisplayChangeListeners of dbus shares one DisplaySurface and
  modifies its texture field, causing OpenGL texture leak and use-after-free.

Again, please provide instructions. I have regularly run -display dbus with multiple clients and qemu compiled with sanitizers. I don't see any leak or use after free.
 
- Introduced extra code to check the compatibility of OpenGL context providers
  and OpenGL texture renderers where those are often inherently tightly coupled
  since they must share the same hardware.

So code checks are meant to prevent misusage. They might be too limited or broken in some ways, but reverting is likely going to introduce other regressions I was trying to fix.

- Needed extra work to broadcast the same change to multiple dbus listeners.


Compared to what?
 
This series solve them by implementing the change broadcast in ui/dbus, which
knows exactly what is needed. Changes for the common code to support multiple
OpenGL DisplayChangeListeners were reverted to fix egl-headless and reduce
the code size.

Thanks a lot for your work, I am going to take a look at your approach. But please help us understand better what the problem actually is, by giving examples & tests to avoid future regressions and document the expected behaviour.


Akihiko Odaki (6):
  ui/dbus: Share one listener for a console
  Revert "console: save current scanout details"
  Revert "ui: split the GL context in a different object"
  Revert "ui: dispatch GL events to all listeners"
  Revert "ui: associate GL context outside of display listener
    registration"
  Revert "ui: factor out qemu_console_set_display_gl_ctx()"

 include/ui/console.h       |  60 +-----
 include/ui/egl-context.h   |   6 +-
 include/ui/gtk.h           |  11 +-
 include/ui/sdl2.h          |   7 +-
 include/ui/spice-display.h |   1 -
 ui/console.c               | 258 +++++++----------------
 ui/dbus-console.c          | 109 ++--------
 ui/dbus-listener.c         | 417 +++++++++++++++++++++++++++----------
 ui/dbus.c                  |  22 --
 ui/dbus.h                  |  36 +++-
 ui/egl-context.c           |   6 +-
 ui/egl-headless.c          |  20 +-
 ui/gtk-egl.c               |  10 +-
 ui/gtk-gl-area.c           |   8 +-
 ui/gtk.c                   |  25 +--
 ui/sdl2-gl.c               |  10 +-
 ui/sdl2.c                  |  14 +-
 ui/spice-display.c         |  19 +-
 18 files changed, 498 insertions(+), 541 deletions(-)

--
2.32.0 (Apple Git-132)




--
Marc-André Lureau

reply via email to

[Prev in Thread] Current Thread [Next in Thread]