Re: [PATCH] spice: delay starting until display are initialized - Marc-André Lureau

From: "Marc-André Lureau" <marcandre.lureau@gmail.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: "Bonzini, Paolo" <pbonzini@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [PATCH] spice: delay starting until display are initialized
Date: Thu, 28 Jan 2021 18:35:39 +0400	[thread overview]
Message-ID: <CAJ+F1CJ0Z378KCLGDzLYOfBor7HkHM2YemRj5F-3kTSnVK7ADg@mail.gmail.com> (raw)
In-Reply-To: <20210128142625.vzdrehzw6mufja3s@sirius.home.kraxel.org>

Hi

On Thu, Jan 28, 2021 at 6:28 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Thu, Jan 28, 2021 at 04:00:20PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Thu, Jan 28, 2021 at 3:57 PM Marc-André Lureau <
> > marcandre.lureau@redhat.com> wrote:
> >
> > > Hi
> > >
> > > On Thu, Jan 28, 2021 at 3:44 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > >>   Hi,
> > >>
> > >> > diff --git a/ui/spice-display.c b/ui/spice-display.c
> > >> > index 0178d5766d..3d3e3bcb22 100644
> > >> > --- a/ui/spice-display.c
> > >> > +++ b/ui/spice-display.c
> > >> > @@ -1188,4 +1188,6 @@ void qemu_spice_display_init(void)
> > >> >          }
> > >> >          qemu_spice_display_init_one(con);
> > >> >      }
> > >>
> > >>        if (runstate_is_running()) {
> > >>             qemu_spice_display_start();
> > >>        }
> > >>
> > >> Isn't that enough?
> > >>
> > >
> > > That should be fine too, I'll update the patch. thanks
> > >
> >
> > Actually no, we still need to prevent the initial
> > qemu_spice_display_start(), and do a single call when everything is ready
> > (since spice server doesn't handle adding interfaces dynamically when
> > running).
>
> I still think that moving these three lines to the correct place is
> enough.  Maybe even just qemu_spice_display_start() as it keeps track
> of the state and you can safely call this twice.
>

It's not enough, since the first time qemu_spice_display_start() is
called (on vm_start) the display interfaces aren't yet registered. And
spice server doesn't automatically start the newly added interfaces.

-- 
Marc-André Lureau