[Spice-devel] [PATCH spice-server] red_worker: Do not sent empty monitor config message

[Christophe Fergeau]

On Tue, Oct 06, 2015 at 11:18:56AM +0200, Pavel Grunt wrote:
> On Mon, 2015-10-05 at 18:39 +0200, Christophe Fergeau wrote:
> > On Thu, Oct 01, 2015 at 05:19:23PM +0200, Pavel Grunt wrote:
> > > Resolves:
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1061942
> > > ---
> > >  server/red_worker.c | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/server/red_worker.c b/server/red_worker.c
> > > index e2feb23..a106923 100644
> > > --- a/server/red_worker.c
> > > +++ b/server/red_worker.c
> > > @@ -9144,7 +9144,6 @@ static void
> > > red_marshall_monitors_config(RedChannelClient *rcc, SpiceMarshaller
> > >      SpiceMsgDisplayMonitorsConfig *msg = spice_malloc0(sizeof(*msg) +
> > > heads_size);
> > >      int count = 0; // ignore monitors_config->count, it may contain zero
> > > width monitors, remove them now
> > >  
> > > -    red_channel_client_init_send_data(rcc,
> > > SPICE_MSG_DISPLAY_MONITORS_CONFIG, NULL);
> > >      for (i = 0 ; i < monitors_config->count; ++i) {
> > >          if (monitors_config->heads[i].width == 0 || monitors_config-
> > > >heads[i].height == 0) {
> > >              continue;
> > > @@ -9157,9 +9156,12 @@ static void
> > > red_marshall_monitors_config(RedChannelClient *rcc, SpiceMarshaller
> > >          msg->heads[count].y = monitors_config->heads[i].y;
> > >          count++;
> > >      }
> > > -    msg->count = count;
> > > -    msg->max_allowed = monitors_config->max_allowed;
> > > -    spice_marshall_msg_display_monitors_config(base_marshaller, msg);
> > > +    if (count > 0) {
> > > +        red_channel_client_init_send_data(rcc,
> > > SPICE_MSG_DISPLAY_MONITORS_CONFIG, NULL);
> > > +        msg->count = count;
> > > +        msg->max_allowed = monitors_config->max_allowed;
> > > +        spice_marshall_msg_display_monitors_config(base_marshaller, msg);
> > > +    }
> > >      free(msg);
> > >  }
> > 
> > Do you know why we get this empty monitors config from the guest in the
> > first place?
> > 
> Hi Christophe,
> 
> It is happening when shutting down a VM (when its graphical session finished).
> But we don't get an empty monitors config message, we (spice server) make it.
> The original message just contains zero width & height monitors. These "zero
> size monitors" are filtered out, and we send the message w/o them, so it is
> empty. I don't know why it is necessary to filter these "zero size monitors".
> I try removing the filter, it also worked.

And it's expected that we get these 0 sized monitor because the guest is
shutting down? Any idea if it would be possible for the guest to stop
emitting this config with nothing useful in it?
Since this situation can be arbitrarily triggered from the guest, I'd
ACK this patch. If we can get well behaving guests to stop sending
problematic monitors configs, then we could have some debug log when we
get one.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151009/d00d6469/attachment.sig>