[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] ui: fix spice display regression
From: |
Marc-André Lureau |
Subject: |
Re: [PATCH] ui: fix spice display regression |
Date: |
Thu, 28 Jan 2021 15:12:18 +0400 |
Hi
On Wed, Jan 27, 2021 at 2:54 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 27/01/21 11:18, Marc-André Lureau wrote:
> > Hi
> >
> > On Wed, Jan 27, 2021 at 2:03 PM <marcandre.lureau@redhat.com> wrote:
> >>
> >> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>
> >> Since commit b4e1a342112e50e05b609e857f38c1f2b7aafdc4 ("vl: remove
> >> separate preconfig main_loop"), spice initialization is a bit dodgy, and
> >> the client get stuck waiting for server events.
> >>
> >> The initialization order changed, so that qemu_spice_display_start() is
> >> called before the display interfaces are added. The new interfaces
> >> aren't started by spice-server automatically (yet?), so we have to tell
> >> the server explicitely when we are already running.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> What is the exact different in initialization order once you add commit
> facf7c60ee ("vl: initialize displays _after_ exiting preconfiguration",
> 2021-01-02)?
>
We used to run qemu_spice.display_init() before vm_start() (in 5.2).
Actually that commit didn't help in this case! It's a bit hard to
track when things broke and how, since various commits created
different issues.
The current initialization order looks better, I am sending another
patch solving this by delaying starting Spice.
> Thanks,
>
> Paolo
>
> >> ---
> >> ui/spice-core.c | 10 +++++++++-
> >> 1 file changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/ui/spice-core.c b/ui/spice-core.c
> >> index 5746d0aae7..6eebf12e3c 100644
> >> --- a/ui/spice-core.c
> >> +++ b/ui/spice-core.c
> >> @@ -830,6 +830,8 @@ static void qemu_spice_init(void)
> >>
> >> static int qemu_spice_add_interface(SpiceBaseInstance *sin)
> >> {
> >> + int ret;
> >> +
> >> if (!spice_server) {
> >> if (QTAILQ_FIRST(&qemu_spice_opts.head) != NULL) {
> >> error_report("Oops: spice configured but not active");
> >> @@ -848,7 +850,13 @@ static int qemu_spice_add_interface(SpiceBaseInstance
> >> *sin)
> >> qemu_add_vm_change_state_handler(vm_change_state_handler, NULL);
> >> }
> >>
> >> - return spice_server_add_interface(spice_server, sin);
> >> + ret = spice_server_add_interface(spice_server, sin);
> >> + /* make sure the newly added interface is started */
> >> + if (ret == 0 && spice_display_is_running) {
> >> + spice_server_vm_start(spice_server);
> >> + }
> >> +
> >> + return ret;
> >> }
> >>
> >> static GSList *spice_consoles;
> >> --
> >> 2.29.0
> >>
> >>
> >
> > Oops, it doesn't work reliably. There is some race in spice server now.
> >
> > spice_server_vm_start() sends RED_WORKER_MESSAGE_START to the QXL
> > worker thread. But if two of those come, it will assert... It should
> > probably not, I will send a patch to spice.
> >
> > I am looking for other options for QEMU though.
> >
>
--
Marc-André Lureau