On Thu, 2015-07-30 at 17:05 -0400, Jonathon Jongsma wrote: > > ----- Original Message ----- > > From: "Pavel Grunt" <pgrunt@xxxxxxxxxx> > > To: "Marc-André Lureau" <mlureau@xxxxxxxxxx> > > Cc: spice-devel@xxxxxxxxxxxxxxxxxxxxx > > Sent: Thursday, July 30, 2015 11:20:27 AM > > Subject: Re: [PATCH spice-gtk] Notify about existence of > > monitor for all display channels > > > > Hi > > > > On Thu, 2015-07-30 at 10:35 -0400, Marc-André Lureau wrote: > > > Hi > > > > > > ----- Original Message ----- > > > > Windows guest can have disabled display on the display chanel #0, in > > > > that case it will not be listed in virt-viewer's "View->Displays" menu > > > > > > > > Resolves: > > > > https://bugs.freedesktop.org/show_bug.cgi?id=91489 > > > > --- > > > > I think notify::monitors should stay in create_canvas(), because in that > > > > function the monitor > > > > configuration changes (if the channel does not have > > > > SPICE_DISPLAY_CAP_MONITORS_CONFIG), > > > > so the notify should be emitted. The notify is also emitted when > > > > SPICE_MSG_DISPLAY_MONITORS_CONFIG > > > > is recieved, Windows guests are getting this message only for enabled > > > > displays. > > > > --- > > > > src/channel-display.c | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > diff --git a/src/channel-display.c b/src/channel-display.c > > > > index e3f03fc..8837bd0 100644 > > > > --- a/src/channel-display.c > > > > +++ b/src/channel-display.c > > > > @@ -811,8 +811,7 @@ static void spice_display_channel_up(SpiceChannel > > > > *channel) > > > > > > > > /* if we are not using monitors config, notify of existence of > > > > this monitor */ > > > > - if (channel->priv->channel_id != 0) > > > > - g_coroutine_object_notify(G_OBJECT(channel), "monitors"); > > > > + g_coroutine_object_notify(G_OBJECT(channel), "monitors"); > > > > > > > > > > And for guests that do send MonitorConfig messages on channel 0, this is > > > not > > > going to race? > > > > There should not be the race, this notify happens in _channel_up() and > > always > > has monitors->len = 0, monitors_max = 1. virt-viewer then creates > > monitors_max > > spice display widgets, but does not enable any of them because > > monitors->len=0. > > Later the SPICE_MSG_DISPLAY_MONITORS_CONFIG is received and the displays are > > enabled in virt-viewer. > > > In addition to what Pavel said, if there was a potential for a race on channel > 0, that possibility would also exist for channels 1, 2, and 3 (since they can > already be enabled when virt-viewer connects). So I don't see any problem with > treating all channels the same. > > I'm comfortable ACKing this patch, but I'd like to have the comment above > this line of code changed or removed. It's a bit confusing. Ok, I will change it to "notify of existence of this monitor" Pavel > > Jonathon _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel