[Spice-commits] 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:51:59 PDT 2011
server/agent-msg-filter.c | 30 ++++++++++++--------
server/agent-msg-filter.h | 5 ++-
server/reds.c | 66 ++++++++++++++++++++++------------------------
3 files changed, 53 insertions(+), 48 deletions(-)
New commits:
commit 3953b9ff1b46441077471baed9ef5173cf4eb448
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 b24eb02..cd86ab9 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -717,9 +717,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 00a03b7633fd30180f7adc83afd376dc6765f32f
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 f04680d..b24eb02 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1013,6 +1013,9 @@ void reds_fill_channels(SpiceMsgChannels *channels_info)
void reds_on_main_agent_start()
{
+ if (!vdagent) {
+ return;
+ }
reds->agent_state.write_filter.discard_all = FALSE;
}
commit e472bc7d5a4feb773e8721724cf135fc282361fb
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 b1a97e6..f04680d 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -171,8 +171,6 @@ typedef struct VDIPortState {
AgentMsgFilter read_filter;
VDIChunkHeader vdi_chunk_header;
-
- int client_agent_started;
} VDIPortState;
#ifdef RED_STATISTICS
@@ -576,8 +574,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;
}
int reds_main_channel_connected()
@@ -595,8 +597,14 @@ void reds_disconnect()
reds->disconnecting = TRUE;
reds->link_id = 0;
+ /* 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;
sif = SPICE_CONTAINEROF(vdagent->base.sif, SpiceCharDeviceInterface, base);
@@ -604,7 +612,6 @@ void reds_disconnect()
if (sif->state) {
sif->state(vdagent, reds->agent_state.connected);
}
- reds->agent_state.client_agent_started = FALSE;
}
reds_shatdown_channels();
@@ -833,14 +840,8 @@ static void dispatch_vdi_port_data(int port, VDIReadBuf *buf)
reds_agent_remove();
return;
}
-
- if (reds->agent_state.connected) {
- main_channel_push_agent_data(reds->main_channel, buf->data, buf->len,
- vdi_read_buf_release, buf);
- } else {
- red_printf("throwing away, no client: %d", buf->len);
- ring_add(&state->read_bufs, &buf->link);
- }
+ main_channel_push_agent_data(reds->main_channel, buf->data, buf->len,
+ vdi_read_buf_release, buf);
break;
}
case VDP_SERVER_PORT:
@@ -1012,7 +1013,7 @@ void reds_fill_channels(SpiceMsgChannels *channels_info)
void reds_on_main_agent_start()
{
- reds->agent_state.client_agent_started = TRUE;
+ reds->agent_state.write_filter.discard_all = FALSE;
}
void reds_on_main_agent_data(void *message, size_t size)
@@ -1041,17 +1042,6 @@ void reds_on_main_agent_data(void *message, size_t size)
return;
}
- if (!vdagent) {
- add_token();
- return;
- }
-
- if (!reds->agent_state.client_agent_started) {
- red_printf("SPICE_MSGC_MAIN_AGENT_DATA race");
- add_token();
- return;
- }
-
if (!(ring_item = ring_get_head(&reds->agent_state.external_bufs))) {
red_printf("no agent free bufs");
reds_disconnect();
@@ -1115,7 +1105,7 @@ void reds_marshall_migrate_data_item(SpiceMarshaller *m, MainMigrateData *data)
data->version = MAIN_CHANNEL_MIG_DATA_VERSION;
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;
@@ -1350,7 +1340,7 @@ void reds_on_main_receive_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);
@@ -1573,6 +1563,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);
}
@@ -3180,6 +3171,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);
}
@@ -3417,8 +3409,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 3accb60240dbfd9a2a7627aef879f403cd517612
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 c7dcdb6..b1a97e6 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -576,7 +576,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;
}
@@ -595,7 +595,8 @@ void reds_disconnect()
reds->disconnecting = TRUE;
reds->link_id = 0;
- 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;
sif = SPICE_CONTAINEROF(vdagent->base.sif, SpiceCharDeviceInterface, base);
@@ -3416,8 +3417,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 66cf0e28b3e7bcc5db3d1faf1de0437a6d2878d1
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 20184fa..c7dcdb6 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1027,17 +1027,6 @@ void reds_on_main_agent_data(void *message, size_t size)
}
--reds->agent_state.num_client_tokens;
- if (!vdagent) {
- add_token();
- return;
- }
-
- if (!reds->agent_state.client_agent_started) {
- red_printf("SPICE_MSGC_MAIN_AGENT_DATA race");
- add_token();
- return;
- }
-
res = agent_msg_filter_process_data(&reds->agent_state.write_filter,
message, size);
switch (res) {
@@ -1051,6 +1040,17 @@ void reds_on_main_agent_data(void *message, size_t size)
return;
}
+ if (!vdagent) {
+ add_token();
+ return;
+ }
+
+ if (!reds->agent_state.client_agent_started) {
+ red_printf("SPICE_MSGC_MAIN_AGENT_DATA race");
+ add_token();
+ return;
+ }
+
if (!(ring_item = ring_get_head(&reds->agent_state.external_bufs))) {
red_printf("no agent free bufs");
reds_disconnect();
commit 326fdf34f28b397914660f00ee9594316d82e516
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 c55f0d3..20184fa 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -576,6 +576,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;
}
@@ -594,6 +595,7 @@ void reds_disconnect()
reds->disconnecting = TRUE;
reds->link_id = 0;
+ agent_msg_filter_init(&reds->agent_state.write_filter, agent_copypaste);
if (reds->agent_state.connected) {
SpiceCharDeviceInterface *sif;
sif = SPICE_CONTAINEROF(vdagent->base.sif, SpiceCharDeviceInterface, base);
commit d5906664080b73338bd2766565a94330fd3fd5d2
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 ba159be..c55f0d3 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -883,7 +883,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 c652dfa5e635342672a66b9f4c7cbbfb1820341c
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 705316b..ba159be 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -765,12 +765,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 f8e6dc78c7e71a3744be380350a55ad98f75def7
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 288174e..705316b 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -601,7 +601,7 @@ 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();
@@ -703,6 +703,10 @@ static void reds_agent_remove()
SpiceCharDeviceInstance *sin = vdagent;
SpiceCharDeviceInterface *sif;
+ if (!reds->mig_target) {
+ reds_reset_vdp();
+ }
+
if (!reds->agent_state.connected) {
return;
}
@@ -723,7 +727,6 @@ static void reds_agent_remove()
return;
}
- reds_reset_vdp();
main_channel_push_agent_disconnected(reds->main_channel);
}
More information about the Spice-commits
mailing list