[Spice-commits] Branch '0.8' - 9 commits - server/agent-msg-filter.c server/agent-msg-filter.h server/reds.c
Hans de Goede
jwrdegoede at kemper.freedesktop.org
Mon Apr 4 02:52:16 PDT 2011
server/agent-msg-filter.c | 30 +++++++++++--------
server/agent-msg-filter.h | 5 +--
server/reds.c | 70 ++++++++++++++++++++--------------------------
3 files changed, 52 insertions(+), 53 deletions(-)
New commits:
commit 443994ba87e0449925a618f9b707c226b6bb43e3
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Apr 1 16:46:55 2011 +0200
server: make sure we clear vdagent and update mouse mode on agent disconnect
The check this patch removes causes us to not set vdagent to NULL, nor
update the mouse mode when the guest agent disconnects when no client is
attached. Which leads to a non working mouse, and on agent reconnect a
"spice_server_char_device_add_interface: vdagent already attached" message
instead of a successful re-add of the agent interface .
diff --git a/server/reds.c b/server/reds.c
index b349d77..6c7df09 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1112,9 +1112,6 @@ static void reds_agent_remove()
reds_reset_vdp();
}
- if (!reds->agent_state.connected) {
- return;
- }
reds->agent_state.connected = 0;
vdagent = NULL;
reds_update_mouse_mode();
commit 620a02da318a5d571343443d0e6625c1893b28e4
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Apr 1 16:45:50 2011 +0200
server: ignore SPICE_MSGC_MAIN_AGENT_START messages when there is no agent
This can happen for example when a SPICE_MSGC_MAIN_AGENT_START message
from the client and the vdagent disconnecting race.
diff --git a/server/reds.c b/server/reds.c
index 05f52a0..b349d77 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1694,7 +1694,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
switch (type) {
case SPICE_MSGC_MAIN_AGENT_START:
red_printf("agent start");
- if (!reds->peer) {
+ if (!reds->peer || !vdagent) {
return;
}
reds->agent_state.write_filter.discard_all = FALSE;
commit 2df49c8a6762a28b0dff383f7f9d6d2d33fffe70
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Apr 1 16:31:37 2011 +0200
server: hookup agent-msg-filter discard-all functionality
This ensures that if the client or agent connects to the client-agent
"tunnel" while the other side is halfway through sending a multi part
message, the rest of the message gets discarded, and the connecting
party starts getting data at the beginning of the next message.
diff --git a/server/reds.c b/server/reds.c
index 40b6bb0..05f52a0 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -205,8 +205,6 @@ typedef struct VDIPortState {
AgentMsgFilter read_filter;
VDIChunkHeader vdi_chunk_header;
-
- int client_agent_started;
} VDIPortState;
typedef struct InputsState {
@@ -713,8 +711,12 @@ static void reds_reset_vdp()
ring_add(&state->read_bufs, &state->current_read_buf->link);
state->current_read_buf = NULL;
}
- agent_msg_filter_init(&state->read_filter, agent_copypaste, FALSE);
- state->client_agent_started = FALSE;
+ /* Reset read filter to start with clean state when the agent reconnects */
+ agent_msg_filter_init(&state->read_filter, agent_copypaste, TRUE);
+ /* Throw away pending chunks from the current (if any) and future
+ messages written by the client */
+ state->write_filter.result = AGENT_MSG_FILTER_DISCARD;
+ state->write_filter.discard_all = TRUE;
}
static void reds_reset_outgoing()
@@ -744,8 +746,13 @@ static void reds_disconnect()
red_printf("");
reds->disconnecting = TRUE;
reds_reset_outgoing();
+ /* Reset write filter to start with clean state on client reconnect */
agent_msg_filter_init(&reds->agent_state.write_filter, agent_copypaste,
- FALSE);
+ TRUE);
+ /* Throw away pending chunks from the current (if any) and future
+ messages read from the agent */
+ reds->agent_state.read_filter.result = AGENT_MSG_FILTER_DISCARD;
+ reds->agent_state.read_filter.discard_all = TRUE;
if (reds->agent_state.connected) {
SpiceCharDeviceInterface *sif;
@@ -754,7 +761,6 @@ static void reds_disconnect()
if (sif->state) {
sif->state(vdagent, reds->agent_state.connected);
}
- reds->agent_state.client_agent_started = FALSE;
}
reds_shatdown_channels();
@@ -1239,17 +1245,10 @@ static void dispatch_vdi_port_data(int port, VDIReadBuf *buf)
reds_agent_remove();
return;
}
-
- if (reds->agent_state.connected) {
- item = new_out_item(SPICE_MSG_MAIN_AGENT_DATA);
-
- spice_marshaller_add_ref_full(item->m, buf->data, buf->len,
- vdi_read_buf_release, buf);
- reds_push_pipe_item(item);
- } else {
- red_printf("throwing away, no client: %d", buf->len);
- vdi_read_buf_release(buf->data, buf);
- }
+ item = new_out_item(SPICE_MSG_MAIN_AGENT_DATA);
+ spice_marshaller_add_ref_full(item->m, buf->data, buf->len,
+ vdi_read_buf_release, buf);
+ reds_push_pipe_item(item);
break;
}
case VDP_SERVER_PORT:
@@ -1436,7 +1435,7 @@ static void main_channel_push_migrate_data_item()
data->ping_id = reds->ping_id;
data->agent_connected = !!state->connected;
- data->client_agent_started = state->client_agent_started;
+ data->client_agent_started = !state->write_filter.discard_all;
data->num_client_tokens = state->num_client_tokens;
data->send_tokens = ~0;
@@ -1676,7 +1675,7 @@ static void main_channel_recive_migrate_data(MainMigrateData *data, uint8_t *end
return;
}
- state->client_agent_started = data->client_agent_started;
+ state->write_filter.discard_all = !data->client_agent_started;
pos = (uint8_t *)(data + 1);
@@ -1698,7 +1697,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
if (!reds->peer) {
return;
}
- reds->agent_state.client_agent_started = TRUE;
+ reds->agent_state.write_filter.discard_all = FALSE;
break;
case SPICE_MSGC_MAIN_AGENT_DATA: {
RingItem *ring_item;
@@ -1724,17 +1723,6 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
return;
}
- if (!vdagent) {
- add_token();
- break;
- }
-
- if (!reds->agent_state.client_agent_started) {
- red_printf("SPICE_MSGC_MAIN_AGENT_DATA race");
- add_token();
- break;
- }
-
if (!(ring_item = ring_get_head(&reds->agent_state.external_bufs))) {
red_printf("no agent free bufs");
reds_disconnect();
@@ -2088,6 +2076,7 @@ static void reds_handle_main_link(RedLinkInfo *link)
SpiceCharDeviceInterface *sif;
sif = SPICE_CONTAINEROF(vdagent->base.sif, SpiceCharDeviceInterface, base);
reds->agent_state.connected = 1;
+ reds->agent_state.read_filter.discard_all = FALSE;
if (sif->state) {
sif->state(vdagent, reds->agent_state.connected);
}
@@ -3497,6 +3486,7 @@ static void attach_to_red_agent(SpiceCharDeviceInstance *sin)
}
sif = SPICE_CONTAINEROF(vdagent->base.sif, SpiceCharDeviceInterface, base);
state->connected = 1;
+ state->read_filter.discard_all = FALSE;
if (sif->state) {
sif->state(vdagent, state->connected);
}
@@ -3751,8 +3741,8 @@ static void init_vd_agent_resources()
ring_init(&state->internal_bufs);
ring_init(&state->write_queue);
ring_init(&state->read_bufs);
- agent_msg_filter_init(&state->write_filter, agent_copypaste, FALSE);
- agent_msg_filter_init(&state->read_filter, agent_copypaste, FALSE);
+ agent_msg_filter_init(&state->write_filter, agent_copypaste, TRUE);
+ agent_msg_filter_init(&state->read_filter, agent_copypaste, TRUE);
state->read_state = VDI_PORT_READ_STATE_READ_HADER;
state->recive_pos = (uint8_t *)&state->vdi_chunk_header;
commit 5d7cf4c002badec981ad7f6708aa9ef3102dcd1c
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Apr 1 14:19:25 2011 +0200
server: add discard all option to agent message filter
diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c
index 3867d11..cd1f78c 100644
--- a/server/agent-msg-filter.c
+++ b/server/agent-msg-filter.c
@@ -22,10 +22,12 @@
#include "red_common.h"
#include "agent-msg-filter.h"
-void agent_msg_filter_init(struct AgentMsgFilter *filter, int copy_paste)
+void agent_msg_filter_init(struct AgentMsgFilter *filter,
+ int copy_paste, int discard_all)
{
memset(filter, 0, sizeof(*filter));
filter->copy_paste_enabled = copy_paste;
+ filter->discard_all = discard_all;
}
int agent_msg_filter_process_data(struct AgentMsgFilter *filter,
@@ -61,19 +63,23 @@ data_to_read:
return AGENT_MSG_FILTER_PROTO_ERROR;
}
- switch (msg_header.type) {
- case VD_AGENT_CLIPBOARD:
- case VD_AGENT_CLIPBOARD_GRAB:
- case VD_AGENT_CLIPBOARD_REQUEST:
- case VD_AGENT_CLIPBOARD_RELEASE:
- if (filter->copy_paste_enabled) {
+ if (filter->discard_all) {
+ filter->result = AGENT_MSG_FILTER_DISCARD;
+ } else {
+ switch (msg_header.type) {
+ case VD_AGENT_CLIPBOARD:
+ case VD_AGENT_CLIPBOARD_GRAB:
+ case VD_AGENT_CLIPBOARD_REQUEST:
+ case VD_AGENT_CLIPBOARD_RELEASE:
+ if (filter->copy_paste_enabled) {
+ filter->result = AGENT_MSG_FILTER_OK;
+ } else {
+ filter->result = AGENT_MSG_FILTER_DISCARD;
+ }
+ break;
+ default:
filter->result = AGENT_MSG_FILTER_OK;
- } else {
- filter->result = AGENT_MSG_FILTER_DISCARD;
}
- break;
- default:
- filter->result = AGENT_MSG_FILTER_OK;
}
filter->msg_data_to_read = msg_header.size;
diff --git a/server/agent-msg-filter.h b/server/agent-msg-filter.h
index 99dbb8c..ecccfc7 100644
--- a/server/agent-msg-filter.h
+++ b/server/agent-msg-filter.h
@@ -32,13 +32,14 @@ enum {
};
typedef struct AgentMsgFilter {
- struct VDAgentMessage msg_header;
int msg_data_to_read;
int result;
int copy_paste_enabled;
+ int discard_all;
} AgentMsgFilter;
-void agent_msg_filter_init(struct AgentMsgFilter *filter, int copy_paste);
+void agent_msg_filter_init(struct AgentMsgFilter *filter,
+ int copy_paste, int discard_all);
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 7a3399e..40b6bb0 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -713,7 +713,7 @@ static void reds_reset_vdp()
ring_add(&state->read_bufs, &state->current_read_buf->link);
state->current_read_buf = NULL;
}
- agent_msg_filter_init(&state->read_filter, agent_copypaste);
+ agent_msg_filter_init(&state->read_filter, agent_copypaste, FALSE);
state->client_agent_started = FALSE;
}
@@ -744,7 +744,8 @@ static void reds_disconnect()
red_printf("");
reds->disconnecting = TRUE;
reds_reset_outgoing();
- agent_msg_filter_init(&reds->agent_state.write_filter, agent_copypaste);
+ agent_msg_filter_init(&reds->agent_state.write_filter, agent_copypaste,
+ FALSE);
if (reds->agent_state.connected) {
SpiceCharDeviceInterface *sif;
@@ -3750,8 +3751,8 @@ static void init_vd_agent_resources()
ring_init(&state->internal_bufs);
ring_init(&state->write_queue);
ring_init(&state->read_bufs);
- agent_msg_filter_init(&state->write_filter, agent_copypaste);
- agent_msg_filter_init(&state->read_filter, agent_copypaste);
+ agent_msg_filter_init(&state->write_filter, agent_copypaste, FALSE);
+ agent_msg_filter_init(&state->read_filter, agent_copypaste, FALSE);
state->read_state = VDI_PORT_READ_STATE_READ_HADER;
state->recive_pos = (uint8_t *)&state->vdi_chunk_header;
commit 1ec316e561c4e3e15abd604dd4ee5dd389aa1084
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Apr 1 14:14:00 2011 +0200
server: filter all data from client
Filter all data from client, even when there is no agent connected
to keep filter state correct.
diff --git a/server/reds.c b/server/reds.c
index 3c7077a..7a3399e 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1710,17 +1710,6 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
}
--reds->agent_state.num_client_tokens;
- if (!vdagent) {
- add_token();
- break;
- }
-
- if (!reds->agent_state.client_agent_started) {
- red_printf("SPICE_MSGC_MAIN_AGENT_DATA race");
- add_token();
- break;
- }
-
res = agent_msg_filter_process_data(&reds->agent_state.write_filter,
message, size);
switch (res) {
@@ -1734,6 +1723,17 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
return;
}
+ if (!vdagent) {
+ add_token();
+ break;
+ }
+
+ if (!reds->agent_state.client_agent_started) {
+ red_printf("SPICE_MSGC_MAIN_AGENT_DATA race");
+ add_token();
+ break;
+ }
+
if (!(ring_item = ring_get_head(&reds->agent_state.external_bufs))) {
red_printf("no agent free bufs");
reds_disconnect();
commit add4bedb2fcc2a3f956f2a2a11785fda80902735
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Apr 1 14:08:56 2011 +0200
server: reset read/write filter on agent/client disconnect
The agent message filter keeps track of messages as they are being send
reset the relevant filter to its initial state when one of the 2 ends
of the agent<->client "tunnel" disconnects.
diff --git a/server/reds.c b/server/reds.c
index 088683b..3c7077a 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -713,6 +713,7 @@ static void reds_reset_vdp()
ring_add(&state->read_bufs, &state->current_read_buf->link);
state->current_read_buf = NULL;
}
+ agent_msg_filter_init(&state->read_filter, agent_copypaste);
state->client_agent_started = FALSE;
}
@@ -743,6 +744,7 @@ static void reds_disconnect()
red_printf("");
reds->disconnecting = TRUE;
reds_reset_outgoing();
+ agent_msg_filter_init(&reds->agent_state.write_filter, agent_copypaste);
if (reds->agent_state.connected) {
SpiceCharDeviceInterface *sif;
commit a19f51265c04bfa33eaa07f499eda35a55844a8b
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Apr 1 10:34:24 2011 +0200
server: break read_from_vdi_port loop if the guest gets disconnected
read_from_vdi_port calls dispatch_vdi_port data, which will disconnect
the guest agent if it sends invalid data. It would then try to read more
data from the disconnected guest agent resulting in a NULL ptr dereference,
this patch fixes this.
diff --git a/server/reds.c b/server/reds.c
index b286809..088683b 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1292,7 +1292,7 @@ static int read_from_vdi_port(void)
}
sif = SPICE_CONTAINEROF(vdagent->base.sif, SpiceCharDeviceInterface, base);
- while (!quit_loop) {
+ while (!quit_loop && vdagent) {
switch (state->read_state) {
case VDI_PORT_READ_STATE_READ_HADER:
n = sif->read(vdagent, state->recive_pos, state->recive_len);
commit 7cc85f33be36eee3351618e53f9ba9fbc5ce7472
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Apr 1 10:16:29 2011 +0200
server: Don't stop writing agent data to the guest when the client disconnects
write_to_vdi_port() was checking for reds->agent_state.connected to determine
wether it could write queued data. But agent_state.connected reflects if
*both* ends are connected. If the client has disconnected, but the guest agent
is still connected and some data is still pending (like a final clipboard
release from the client), then this data should be written to the guest agent.
diff --git a/server/reds.c b/server/reds.c
index 1521bc5..b286809 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1170,12 +1170,12 @@ static int write_to_vdi_port()
int total = 0;
int n;
- if (!reds->agent_state.connected || reds->mig_target) {
+ if (!vdagent || reds->mig_target) {
return 0;
}
sif = SPICE_CONTAINEROF(vdagent->base.sif, SpiceCharDeviceInterface, base);
- while (reds->agent_state.connected) {
+ while (vdagent) {
if (!(ring_item = ring_get_tail(&state->write_queue))) {
break;
}
commit ef6886732e332042242a09c2787dd4a8ddcbfea3
Author: Hans de Goede <hdegoede at redhat.com>
Date: Thu Mar 31 19:13:28 2011 +0200
server: Don't reset agent state when the client disconnects
We were calling reds_reset_vdp on client disconnect, which is not a good
idea. reds_reset_vdp does 3 things:
1) It resets the state related to reading chunks from the spicevmc virtio
port. If the client disconnects while the guest agent is in the middle
of sending a chunk, this will lead to an inconsistent state, and lots
of printing of "dispatch_vdi_port_data: invalid port" messages caused
by this inconsistent state sometimes followed by a segfault.
This can be triggered by copy and pasting something large (say
a screenshot) from the guest to the spice-gtk client, as the spice-gtk
client currently has a bug causing it to crash when receiving a multi
chunk vdagent messages. Without this patch (and with the spice-gtk bug
present) I can consistently reproduce this.
2) It clears any buffered writes from the client to the guest still pending
because the virtio port cannot consume data fast enough. Since the agent
itself is still running fine, throwing away writes for it because the
client has disconnected makes no sense. Esp, since on clean exit the
client may very well send a clipboard release message directly
before closing the connection, and this may get lost this way.
3) It sets client_agent_started to false, this is the only thing which
actually makes sense to do on client disconnect.
Note that since we no longer reset the vdp state on client disconnect, we
must now reset it on agent disconnect even if we don't have a client. So
the reds_reset_vdp call in reds_agent_remove() gets moved to the top,
above both the agent_state.connected and reds->peer checks which will
both fail in the no client case.
diff --git a/server/reds.c b/server/reds.c
index a808390..1521bc5 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -751,7 +751,7 @@ static void reds_disconnect()
if (sif->state) {
sif->state(vdagent, reds->agent_state.connected);
}
- reds_reset_vdp();
+ reds->agent_state.client_agent_started = FALSE;
}
reds_shatdown_channels();
@@ -1099,13 +1099,16 @@ static void reds_agent_remove()
SpiceCharDeviceInstance *sin = vdagent;
SpiceCharDeviceInterface *sif;
+ if (!reds->mig_target) {
+ reds_reset_vdp();
+ }
+
if (!reds->agent_state.connected) {
return;
}
reds->agent_state.connected = 0;
vdagent = NULL;
reds_update_mouse_mode();
-
if (!reds->peer || !sin) {
return;
}
@@ -1117,7 +1120,6 @@ static void reds_agent_remove()
if (reds->mig_target) {
return;
}
- reds_reset_vdp();
reds_send_agent_disconnected();
}
More information about the Spice-commits
mailing list