Re: [PATCH] fix regression due to callback called earlier

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

 



On Fri, 2016-02-19 at 08:20 +0100, Pavel Grunt wrote:
> On Thu, 2016-02-18 at 08:08 -0500, Frediano Ziglio wrote:
> > > 
> > > > 
> > > > Patch 1f210080609f2c00b4d1859eb0b363a38838e7d3 ("Remove use of
> > > > global
> > > > 'reds' from AgentMsgFilter") introduced a regression. This
> > > > because
> > > > QXLInterface->client_monitors_config was called before
> > > > returning
> > > > from spice_add_interface. Some client of spice-server expect
> > > > the
> > > > spice_add_interface already returned and some state change was
> > > > done
> > > > before client_monitors_config was called.
> > > > 
> > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > 
> > > This fixes https://bugs.freedesktop.org/show_bug.cgi?id=94192.
> > > 
> > > I was wondering if would be better to revert previous and then
> > > add this implementation but in this case we'll lose the
> > > int -> gboolean changes.
> > > 
> > 
> > Diff after the revert is
> Acked-by: Pavel Grunt <pgrunt@xxxxxxxxxx>

Frediano, seems that the revert may not be needed:

https://lists.freedesktop.org/archives/spice-devel/2016-February/026777
.html

Pavel

> > 
> > 
> > diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c
> > index 069822b..d72a3e4 100644
> > --- a/server/agent-msg-filter.c
> > +++ b/server/agent-msg-filter.c
> > @@ -28,7 +28,8 @@
> >  #include "red-dispatcher.h"
> >  
> >  void agent_msg_filter_init(struct AgentMsgFilter *filter,
> > -                           int copy_paste, int file_xfer, int
> > discard_all)
> > +                           gboolean copy_paste, gboolean
> > file_xfer,
> > +                           int discard_all)
> >  {
> >      memset(filter, 0, sizeof(*filter));
> >      filter->copy_paste_enabled = copy_paste;
> > @@ -37,6 +38,7 @@ void agent_msg_filter_init(struct AgentMsgFilter
> > *filter,
> >  }
> >  
> >  int agent_msg_filter_process_data(struct AgentMsgFilter *filter,
> > +                                  RedsState *reds,
> >                                    uint8_t *data, uint32_t len)
> >  {
> >      struct VDAgentMessage msg_header;
> > diff --git a/server/agent-msg-filter.h b/server/agent-msg-filter.h
> > index 92aabce..a4fc4cf 100644
> > --- a/server/agent-msg-filter.h
> > +++ b/server/agent-msg-filter.h
> > @@ -35,14 +35,16 @@ enum {
> >  typedef struct AgentMsgFilter {
> >      int msg_data_to_read;
> >      int result;
> > -    int copy_paste_enabled;
> > -    int file_xfer_enabled;
> > -    int discard_all;
> > +    gboolean copy_paste_enabled;
> > +    gboolean file_xfer_enabled;
> > +    gboolean discard_all;
> >  } AgentMsgFilter;
> >  
> >  void agent_msg_filter_init(struct AgentMsgFilter *filter,
> > -                           int copy_paste, int file_xfer, int
> > discard_all);
> > +                           gboolean copy_paste, gboolean
> > file_xfer,
> > +                           gboolean discard_all);
> >  int agent_msg_filter_process_data(struct AgentMsgFilter *filter,
> > +                                  SpiceServer *reds,
> >                                    uint8_t *data, uint32_t len);
> >  
> >  #endif
> > diff --git a/server/reds.c b/server/reds.c
> > index bb61a28..b917f26 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -637,7 +637,7 @@ static int vdi_port_read_buf_process(RedsState
> > *reds, VDIReadBuf *buf)
> >  
> >      switch (state->vdi_chunk_header.port) {
> >      case VDP_CLIENT_PORT: {
> > -        res = agent_msg_filter_process_data(&state->read_filter,
> > +        res = agent_msg_filter_process_data(&state->read_filter,
> > reds,
> >                                              buf->data, buf->len);
> >          switch (res) {
> >          case AGENT_MSG_FILTER_OK:
> > @@ -1047,7 +1047,7 @@ void reds_on_main_agent_data(RedsState *reds,
> > MainChannelClient *mcc, void *mess
> >      VDIChunkHeader *header;
> >      int res;
> >  
> > -    res = agent_msg_filter_process_data(&reds-
> > > agent_state.write_filter,
> > +    res = agent_msg_filter_process_data(&reds-
> > > agent_state.write_filter, reds,
> >                                          message, size);
> >      switch (res) {
> >      case AGENT_MSG_FILTER_OK:
> > 
> > 
> > Frediano
> > 
> > > 
> > > > ---
> > > >  server/agent-msg-filter.c |  5 ++---
> > > >  server/agent-msg-filter.h |  3 +--
> > > >  server/reds.c             | 18 ++++++------------
> > > >  3 files changed, 9 insertions(+), 17 deletions(-)
> > > > 
> > > > diff --git a/server/agent-msg-filter.c b/server/agent-msg-
> > > > filter.c
> > > > index 14d5100..d72a3e4 100644
> > > > --- a/server/agent-msg-filter.c
> > > > +++ b/server/agent-msg-filter.c
> > > > @@ -29,17 +29,16 @@
> > > >  
> > > >  void agent_msg_filter_init(struct AgentMsgFilter *filter,
> > > >                             gboolean copy_paste, gboolean
> > > > file_xfer,
> > > > -                           gboolean
> > > > use_client_monitors_config,
> > > >                             int discard_all)
> > > >  {
> > > >      memset(filter, 0, sizeof(*filter));
> > > >      filter->copy_paste_enabled = copy_paste;
> > > >      filter->file_xfer_enabled = file_xfer;
> > > > -    filter->use_client_monitors_config =
> > > > use_client_monitors_config;
> > > >      filter->discard_all = discard_all;
> > > >  }
> > > >  
> > > >  int agent_msg_filter_process_data(struct AgentMsgFilter
> > > > *filter,
> > > > +                                  RedsState *reds,
> > > >                                    uint8_t *data, uint32_t len)
> > > >  {
> > > >      struct VDAgentMessage msg_header;
> > > > @@ -96,7 +95,7 @@ data_to_read:
> > > >              }
> > > >              break;
> > > >          case VD_AGENT_MONITORS_CONFIG:
> > > > -            if (filter->use_client_monitors_config) {
> > > > +            if (reds_use_client_monitors_config(reds)) {
> > > >                  filter->result =
> > > > AGENT_MSG_FILTER_MONITORS_CONFIG;
> > > >              } else {
> > > >                  filter->result = AGENT_MSG_FILTER_OK;
> > > > diff --git a/server/agent-msg-filter.h b/server/agent-msg-
> > > > filter.h
> > > > index c04face..a4fc4cf 100644
> > > > --- a/server/agent-msg-filter.h
> > > > +++ b/server/agent-msg-filter.h
> > > > @@ -37,15 +37,14 @@ typedef struct AgentMsgFilter {
> > > >      int result;
> > > >      gboolean copy_paste_enabled;
> > > >      gboolean file_xfer_enabled;
> > > > -    gboolean use_client_monitors_config;
> > > >      gboolean discard_all;
> > > >  } AgentMsgFilter;
> > > >  
> > > >  void agent_msg_filter_init(struct AgentMsgFilter *filter,
> > > >                             gboolean copy_paste, gboolean
> > > > file_xfer,
> > > > -                           gboolean
> > > > use_client_monitors_config,
> > > >                             gboolean discard_all);
> > > >  int agent_msg_filter_process_data(struct AgentMsgFilter
> > > > *filter,
> > > > +                                  SpiceServer *reds,
> > > >                                    uint8_t *data, uint32_t
> > > > len);
> > > >  
> > > >  #endif
> > > > diff --git a/server/reds.c b/server/reds.c
> > > > index 5fb5825..b917f26 100644
> > > > --- a/server/reds.c
> > > > +++ b/server/reds.c
> > > > @@ -407,8 +407,7 @@ static void reds_reset_vdp(RedsState *reds)
> > > >      }
> > > >      /* Reset read filter to start with clean state when the
> > > > agent
> > > >      reconnects
> > > >      */
> > > >      agent_msg_filter_init(&state->read_filter, reds-
> > > > > agent_copypaste,
> > > > -                          reds->agent_file_xfer,
> > > > -                          reds_use_client_monitors_config(reds
> > > > ),
> > > > TRUE);
> > > > +                          reds->agent_file_xfer, TRUE);
> > > >      /* Throw away pending chunks from the current (if any) and
> > > > future
> > > >       * messages written by the client.
> > > >       * TODO: client should clear its agent messages queue when
> > > > the agent
> > > > @@ -522,8 +521,7 @@ void reds_client_disconnect(RedsState
> > > > *reds,
> > > > RedClient
> > > > *client)
> > > >  
> > > >          /* Reset write filter to start with clean state on
> > > > client
> > > >          reconnect
> > > >          */
> > > >          agent_msg_filter_init(&reds->agent_state.write_filter,
> > > >          reds->agent_copypaste,
> > > > -                              reds->agent_file_xfer,
> > > > -                              reds_use_client_monitors_config(
> > > > re
> > > > ds),
> > > > TRUE);
> > > > +                              reds->agent_file_xfer, TRUE);
> > > >  
> > > >          /* Throw away pending chunks from the current (if any)
> > > > and future
> > > >           *  messages read from the agent */
> > > > @@ -639,7 +637,7 @@ static int
> > > > vdi_port_read_buf_process(RedsState *reds,
> > > > VDIReadBuf *buf)
> > > >  
> > > >      switch (state->vdi_chunk_header.port) {
> > > >      case VDP_CLIENT_PORT: {
> > > > -        res = agent_msg_filter_process_data(&state-
> > > > >read_filter,
> > > > +        res = agent_msg_filter_process_data(&state-
> > > > >read_filter, 
> > > > reds,
> > > >                                              buf->data, buf-
> > > > > len);
> > > >          switch (res) {
> > > >          case AGENT_MSG_FILTER_OK:
> > > > @@ -1049,7 +1047,7 @@ void reds_on_main_agent_data(RedsState
> > > > *reds,
> > > > MainChannelClient *mcc, void *mess
> > > >      VDIChunkHeader *header;
> > > >      int res;
> > > >  
> > > > -    res = agent_msg_filter_process_data(&reds-
> > > > > agent_state.write_filter,
> > > > +    res = agent_msg_filter_process_data(&reds-
> > > > > agent_state.write_filter,
> > > > reds,
> > > >                                          message, size);
> > > >      switch (res) {
> > > >      case AGENT_MSG_FILTER_OK:
> > > > @@ -3214,8 +3212,6 @@ SPICE_GNUC_VISIBLE int
> > > > spice_server_add_interface(SpiceServer *s,
> > > >          red_dispatcher_init(qxl);
> > > >          dispatcher = qxl->st->dispatcher;
> > > >          reds->dispatchers = g_list_prepend(reds->dispatchers,
> > > > dispatcher);
> > > > -        reds-
> > > > > agent_state.write_filter.use_client_monitors_config =
> > > > reds_use_client_monitors_config(reds);
> > > > -        reds-
> > > > >agent_state.read_filter.use_client_monitors_config 
> > > > =
> > > > reds_use_client_monitors_config(reds);
> > > >  
> > > >          /* this function has to be called after the dispatcher
> > > > is on the
> > > >          list
> > > >           * as QXLInstance clients expect the dispatcher to be
> > > > on
> > > > the list
> > > >           when
> > > > @@ -3321,11 +3317,9 @@ static void
> > > > reds_init_vd_agent_resources(RedsState
> > > > *reds)
> > > >  
> > > >      ring_init(&state->read_bufs);
> > > >      agent_msg_filter_init(&state->write_filter, reds-
> > > > > agent_copypaste,
> > > > -                          reds->agent_file_xfer,
> > > > -                          reds_use_client_monitors_config(reds
> > > > ),
> > > > TRUE);
> > > > +                          reds->agent_file_xfer, TRUE);
> > > >      agent_msg_filter_init(&state->read_filter, reds-
> > > > > agent_copypaste,
> > > > -                          reds->agent_file_xfer,
> > > > -                          reds_use_client_monitors_config(reds
> > > > ),
> > > > TRUE);
> > > > +                          reds->agent_file_xfer, TRUE);
> > > >  
> > > >      state->read_state = VDI_PORT_READ_STATE_READ_HEADER;
> > > >      state->receive_pos = (uint8_t *)&state->vdi_chunk_header;
> > > > --
> > > > 2.5.0
> > > > 
> > > > 
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]