[Spice-commits] 3 commits - server/agent-msg-filter.c server/agent-msg-filter.h server/reds.c

Frediano Ziglio fziglio at kemper.freedesktop.org
Mon Mar 21 10:36:49 UTC 2016


 server/agent-msg-filter.c |   17 ++++++++++++----
 server/agent-msg-filter.h |    6 ++++-
 server/reds.c             |   48 +++++++++++++++++++++++++++++++++-------------
 3 files changed, 53 insertions(+), 18 deletions(-)

New commits:
commit 4bf53c23d82fd6dd6650c525051fe7b92491cb63
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Fri Mar 18 16:29:39 2016 +0100

    Remove dependency of vdi_port_read_buf_process on RedsState
    
    This makes it easier to move the VDIPort API to a different file, and
    make it as self-contained as possible.
    
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/reds.c b/server/reds.c
index 91c7851..65e5197 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -634,11 +634,12 @@ static void vdi_port_read_buf_release(uint8_t *data, void *opaque)
 }
 
 /* returns TRUE if the buffer can be forwarded */
-static int vdi_port_read_buf_process(RedsState *reds, VDIReadBuf *buf)
+static gboolean vdi_port_read_buf_process(VDIPortState *state, VDIReadBuf *buf, gboolean *error)
 {
-    VDIPortState *state = &reds->agent_state;
     int res;
 
+    *error = FALSE;
+
     switch (state->vdi_chunk_header.port) {
     case VDP_CLIENT_PORT: {
         res = agent_msg_filter_process_data(&state->read_filter,
@@ -649,7 +650,7 @@ static int vdi_port_read_buf_process(RedsState *reds, VDIReadBuf *buf)
         case AGENT_MSG_FILTER_DISCARD:
             return FALSE;
         case AGENT_MSG_FILTER_PROTO_ERROR:
-            reds_agent_remove(reds);
+            *error = TRUE;
             return FALSE;
         }
     }
@@ -657,7 +658,7 @@ static int vdi_port_read_buf_process(RedsState *reds, VDIReadBuf *buf)
         return FALSE;
     default:
         spice_warning("invalid port");
-        reds_agent_remove(reds);
+        *error = TRUE;
         return FALSE;
     }
 }
@@ -741,7 +742,8 @@ static SpiceCharDeviceMsgToClient *vdi_port_read_one_msg_from_device(SpiceCharDe
             state->message_receive_len -= state->receive_len;
             state->read_state = VDI_PORT_READ_STATE_READ_DATA;
         }
-        case VDI_PORT_READ_STATE_READ_DATA:
+        case VDI_PORT_READ_STATE_READ_DATA: {
+            gboolean error = FALSE;
             n = sif->read(reds->vdagent, state->receive_pos, state->receive_len);
             if (!n) {
                 return NULL;
@@ -760,11 +762,15 @@ static SpiceCharDeviceMsgToClient *vdi_port_read_one_msg_from_device(SpiceCharDe
             } else {
                 state->read_state = VDI_PORT_READ_STATE_GET_BUFF;
             }
-            if (vdi_port_read_buf_process(reds, dispatch_buf)) {
+            if (vdi_port_read_buf_process(&reds->agent_state, dispatch_buf, &error)) {
                 return dispatch_buf;
             } else {
+                if (error) {
+                    reds_agent_remove(reds);
+                }
                 vdi_port_read_buf_unref(dispatch_buf);
             }
+        }
         } /* END switch */
     } /* END while */
     return NULL;
@@ -1129,18 +1135,22 @@ void reds_on_main_channel_migrate(RedsState *reds, MainChannelClient *mcc)
     if (agent_state->read_filter.msg_data_to_read ||
         read_data_len > sizeof(VDAgentMessage)) { /* msg header has been read */
         VDIReadBuf *read_buf = agent_state->current_read_buf;
+        gboolean error = FALSE;
 
         spice_debug("push partial read %u (msg first chunk? %d)", read_data_len,
                     !agent_state->read_filter.msg_data_to_read);
 
         read_buf->len = read_data_len;
-        if (vdi_port_read_buf_process(reds, read_buf)) {
+        if (vdi_port_read_buf_process(&reds->agent_state, read_buf, &error)) {
             main_channel_client_push_agent_data(mcc,
                                                 read_buf->data,
                                                 read_buf->len,
                                                 vdi_port_read_buf_release,
                                                 read_buf);
         } else {
+            if (error) {
+               reds_agent_remove(reds);
+            }
             vdi_port_read_buf_unref(read_buf);
         }
 
commit 0df59fcfc992c6fe3f7a3140a13b8de227f36edc
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Fri Mar 18 16:29:38 2016 +0100

    agent: Sync AgentMsgFilter state upon agent connection
    
    AgentMsgFilter needs to know whether monitors config messages need to be
    filtered or not. This used to be done from within
    agent_msg_filter_config() using the global RedsState, but this got more
    tricky as it was removed.
    A first attempt a1e62fa5ae9 caused crashes on qemu startup with
    "qemu-system-x86_64 -spice port=5900" (without -vga qxl). A second
    attempt added a RedsState* argument to agent_msg_filter_config() which
    in my opinion is not really nice from a layering point of view.
    
    This new attempt makes sure AgentMsgFilter state is correct when the
    filter is set to stop discarding all data, which allows to remove direct
    use of RedsState from within AgentMsgFilter.
    
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c
index db8526e..498864c 100644
--- a/server/agent-msg-filter.c
+++ b/server/agent-msg-filter.c
@@ -27,15 +27,23 @@
 #include "reds.h"
 #include "red-qxl.h"
 
+void agent_msg_filter_config(struct AgentMsgFilter *filter,
+                             gboolean copy_paste, gboolean file_xfer,
+                             gboolean use_client_monitors_config)
+{
+    filter->copy_paste_enabled = copy_paste;
+    filter->file_xfer_enabled = file_xfer;
+    filter->use_client_monitors_config = use_client_monitors_config;
+}
+
 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;
+    agent_msg_filter_config(filter, copy_paste, file_xfer,
+                            use_client_monitors_config);
     filter->discard_all = discard_all;
 }
 
diff --git a/server/agent-msg-filter.h b/server/agent-msg-filter.h
index c04face..2ee055c 100644
--- a/server/agent-msg-filter.h
+++ b/server/agent-msg-filter.h
@@ -45,6 +45,9 @@ void agent_msg_filter_init(struct AgentMsgFilter *filter,
                            gboolean copy_paste, gboolean file_xfer,
                            gboolean use_client_monitors_config,
                            gboolean discard_all);
+void agent_msg_filter_config(struct AgentMsgFilter *filter,
+                             gboolean copy_paste, gboolean file_xfer,
+                             gboolean use_client_monitors_config);
 int agent_msg_filter_process_data(struct AgentMsgFilter *filter,
                                   uint8_t *data, uint32_t len);
 
diff --git a/server/reds.c b/server/reds.c
index f10f218..91c7851 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -953,6 +953,10 @@ void reds_on_main_agent_start(RedsState *reds, MainChannelClient *mcc, uint32_t
                                                     rcc->client,
                                                     num_tokens);
     }
+
+    agent_msg_filter_config(&reds->agent_state.write_filter, reds->agent_copypaste,
+                            reds->agent_file_xfer,
+                            reds_use_client_monitors_config(reds));
     reds->agent_state.write_filter.discard_all = FALSE;
 }
 
@@ -1680,6 +1684,10 @@ static void reds_handle_main_link(RedsState *reds, RedLinkInfo *link)
         if (mig_target) {
             spice_warning("unexpected: vdagent attached to destination during migration");
         }
+        agent_msg_filter_config(&reds->agent_state.read_filter,
+                                reds->agent_copypaste,
+                                reds->agent_file_xfer,
+                                reds_use_client_monitors_config(reds));
         reds->agent_state.read_filter.discard_all = FALSE;
         reds->agent_state.plug_generation++;
     }
@@ -3190,8 +3198,6 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
         qxl = SPICE_CONTAINEROF(sin, QXLInstance, base);
         red_qxl_init(reds, qxl);
         reds->qxl_instances = g_list_prepend(reds->qxl_instances, qxl);
-        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 qxl is on the list
          * as QXLInstance clients expect the qxl to be on the list when
commit 79e02e98ab52c95f588b30689952451a4c09ff09
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Fri Mar 18 16:29:37 2016 +0100

    Revert "fix regression due to callback called earlier"
    
    Passing Reds into agent-msg-filter.[ch] isn't the right thing to do from
    a layering point of view.
    
    This reverts commit a1e62fa5ae983b7b69cb437b2635ce84b2471383.
    
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c
index a6aee9e..db8526e 100644
--- a/server/agent-msg-filter.c
+++ b/server/agent-msg-filter.c
@@ -29,16 +29,17 @@
 
 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;
@@ -95,7 +96,7 @@ data_to_read:
             }
             break;
         case VD_AGENT_MONITORS_CONFIG:
-            if (reds_use_client_monitors_config(reds)) {
+            if (filter->use_client_monitors_config) {
                 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 a4fc4cf..c04face 100644
--- a/server/agent-msg-filter.h
+++ b/server/agent-msg-filter.h
@@ -37,14 +37,15 @@ 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 e4ee1cf..f10f218 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -407,7 +407,8 @@ 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, TRUE);
+                          reds->agent_file_xfer,
+                          reds_use_client_monitors_config(reds), 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
@@ -521,7 +522,8 @@ 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, TRUE);
+                              reds->agent_file_xfer,
+                              reds_use_client_monitors_config(reds), TRUE);
 
         /* Throw away pending chunks from the current (if any) and future
          *  messages read from the agent */
@@ -639,7 +641,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, reds,
+        res = agent_msg_filter_process_data(&state->read_filter,
                                             buf->data, buf->len);
         switch (res) {
         case AGENT_MSG_FILTER_OK:
@@ -1049,7 +1051,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, reds,
+    res = agent_msg_filter_process_data(&reds->agent_state.write_filter,
                                         message, size);
     switch (res) {
     case AGENT_MSG_FILTER_OK:
@@ -3188,6 +3190,8 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
         qxl = SPICE_CONTAINEROF(sin, QXLInstance, base);
         red_qxl_init(reds, qxl);
         reds->qxl_instances = g_list_prepend(reds->qxl_instances, qxl);
+        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 qxl is on the list
          * as QXLInstance clients expect the qxl to be on the list when
@@ -3293,9 +3297,11 @@ 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, TRUE);
+                          reds->agent_file_xfer,
+                          reds_use_client_monitors_config(reds), TRUE);
     agent_msg_filter_init(&state->read_filter, reds->agent_copypaste,
-                          reds->agent_file_xfer, TRUE);
+                          reds->agent_file_xfer,
+                          reds_use_client_monitors_config(reds), TRUE);
 
     state->read_state = VDI_PORT_READ_STATE_READ_HEADER;
     state->receive_pos = (uint8_t *)&state->vdi_chunk_header;


More information about the Spice-commits mailing list