[Spice-commits] 5 commits - client/x11 server/reds.c
Hans de Goede
jwrdegoede at kemper.freedesktop.org
Sat Oct 16 06:45:23 PDT 2010
client/x11/platform.cpp | 126 ++++++++++++++++++++++++++----------------------
server/reds.c | 57 ++++++++-------------
2 files changed, 92 insertions(+), 91 deletions(-)
New commits:
commit 21f586762f96f9b0c3237b2b7ea21fbc9022435c
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Oct 15 09:36:52 2010 +0200
server: remove useless agent send_tokens
We are keeping track of tokens for sending agent data to the client, but
the client send an initial value of ~0, and never gives us new send tokens
so this is all rather useless -> remove it.
Note that it is kept in the migration data struct for compatibility reasons.
diff --git a/server/reds.c b/server/reds.c
index 5160b01..7ab4925 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -203,7 +203,6 @@ typedef struct VDIPortState {
VDIChunkHeader vdi_chunk_header;
int client_agent_started;
- uint32_t send_tokens;
} VDIPortState;
typedef struct InputsState {
@@ -710,7 +709,6 @@ static void reds_reset_vdp()
state->current_read_buf = NULL;
}
state->client_agent_started = FALSE;
- state->send_tokens = 0;
}
static void reds_reset_outgoing()
@@ -1288,13 +1286,6 @@ static int read_from_vdi_port(void)
break;
}
- if (state->vdi_chunk_header.port == VDP_CLIENT_PORT) {
- if (!state->send_tokens) {
- quit_loop = 1;
- break;
- }
- --state->send_tokens;
- }
ring_remove(item);
state->current_read_buf = (VDIReadBuf *)item;
state->recive_pos = state->current_read_buf->data;
@@ -1414,7 +1405,7 @@ static void main_channel_push_migrate_data_item()
data->agent_connected = !!state->connected;
data->client_agent_started = state->client_agent_started;
data->num_client_tokens = state->num_client_tokens;
- data->send_tokens = state->send_tokens;
+ data->send_tokens = ~0;
data->read_state = state->read_state;
data->vdi_chunk_header = state->vdi_chunk_header;
@@ -1633,8 +1624,6 @@ static void main_channel_recive_migrate_data(MainMigrateData *data, uint8_t *end
ASSERT(state->num_client_tokens + data->write_queue_size <= REDS_AGENT_WINDOW_SIZE +
REDS_NUM_INTERNAL_AGENT_MESSAGES);
state->num_tokens = REDS_AGENT_WINDOW_SIZE - state->num_client_tokens;
- state->send_tokens = data->send_tokens;
-
if (!data->agent_connected) {
if (state->connected) {
@@ -1669,19 +1658,13 @@ static void main_channel_recive_migrate_data(MainMigrateData *data, uint8_t *end
static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, void *message)
{
switch (type) {
- case SPICE_MSGC_MAIN_AGENT_START: {
- SpiceMsgcMainAgentTokens *agent_start;
-
+ case SPICE_MSGC_MAIN_AGENT_START:
red_printf("agent start");
if (!reds->peer) {
return;
}
- agent_start = (SpiceMsgcMainAgentTokens *)message;
reds->agent_state.client_agent_started = TRUE;
- reds->agent_state.send_tokens = agent_start->num_tokens; // TODO: sanitize? coming from guest!
- while (read_from_vdi_port());
break;
- }
case SPICE_MSGC_MAIN_AGENT_DATA: {
RingItem *ring_item;
VDAgentExtBuf *buf;
@@ -1725,19 +1708,8 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
write_to_vdi_port();
break;
}
- case SPICE_MSGC_MAIN_AGENT_TOKEN: {
- SpiceMsgcMainAgentTokens *token;
-
- if (!reds->agent_state.client_agent_started) {
- red_printf("SPICE_MSGC_MAIN_AGENT_TOKEN race");
- break;
- }
-
- token = (SpiceMsgcMainAgentTokens *)message;
- reds->agent_state.send_tokens += token->num_tokens;
- while (read_from_vdi_port());
+ case SPICE_MSGC_MAIN_AGENT_TOKEN:
break;
- }
case SPICE_MSGC_MAIN_ATTACH_CHANNELS:
reds_send_channels();
break;
@@ -2048,7 +2020,6 @@ static void reds_handle_main_link(RedLinkInfo *link)
reds_send_link_result(link, SPICE_LINK_ERR_OK);
while((connection_id = rand()) == 0);
reds->agent_state.num_tokens = 0;
- reds->agent_state.send_tokens = 0;
memcpy(&(reds->taTicket), &taTicket, sizeof(reds->taTicket));
reds->mig_target = FALSE;
} else {
@@ -2106,6 +2077,8 @@ static void reds_handle_main_link(RedLinkInfo *link)
reds_push_pipe_item(item);
reds_start_net_test();
+ /* Now that we have a client, forward any pending agent data */
+ while (read_from_vdi_port());
}
}
commit 0b2336cd9c556cec98457e16e09e6c9855d81e82
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Oct 15 09:16:49 2010 +0200
Call read_from_vdi_port() from vdi_read_buf_release()
read_from_vdi_port(), called from vdagent_char_device_wakeup() may
fail to consume all data because no buffers are available in the
read_bufs ring. When this happens we would fail to ever read more data
from the agent on the guest as the port is throttled and stays throttled
until we've consumed all data from the current buffer.
This patch re-enables the call to read_from_vdi_port() from
vdi_read_buf_release(), so that we will try the read again when space
becomes available in the read_bufs ring.
Together with another nasty hack in the linux guest virtio_console
driver, where it waits for a write to be acked by the host before
continuing with the next one, this can lead to a linux guest
getting stuck / hang (until the write is read by the spice-server
which never happens becaus of the above issues).
Note that even with this patch, the guest will still gets stuck due to
a bug in watch_update_mask in spice-core in qemu, which causes writing
to the client to never resume once it blocked. A patch for this has been
submitted to qemu.
diff --git a/server/reds.c b/server/reds.c
index 8e63062..5160b01 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1202,7 +1202,12 @@ void vdi_read_buf_release(uint8_t *data, void *opaque)
VDIReadBuf *buf = (VDIReadBuf *)opaque;
ring_add(&reds->agent_state.read_bufs, &buf->link);
- //read_from_vdi_port(); // XXX WTF? - ask arnon. should we be calling this here?? (causes recursion of read_from_vdi_port..)
+ /* read_from_vdi_port() may have never completed because the read_bufs
+ ring was empty. So we call it again so it can complete its work if
+ necessary. Note since we can be called from read_from_vdi_port ourselves
+ this can cause recursion, read_from_vdi_port() contains code protecting
+ it against this. */
+ while (read_from_vdi_port());
}
static void dispatch_vdi_port_data(int port, VDIReadBuf *buf)
@@ -1235,7 +1240,12 @@ static void dispatch_vdi_port_data(int port, VDIReadBuf *buf)
it returns 0 ensures that all data has been consumed. */
static int read_from_vdi_port(void)
{
- // FIXME: UGLY HACK. Result of spice-vmc vmc_read triggering flush of throttled data, and recalling this.
+ /* There are 2 scenarios where we can get called recursively:
+ 1) spice-vmc vmc_read triggering flush of throttled data, recalling us
+ 2) the buf we push to the client may be send immediately without
+ blocking, in which case its free function will recall us
+ This messes up the state machine, so ignore recursive calls.
+ This is why we always must be called in a loop. */
static int inside_call = 0;
int quit_loop = 0;
VDIPortState *state = &reds->agent_state;
commit a52324525d5707365c4b6758e5d5b08f21b0ac31
Author: Hans de Goede <hdegoede at redhat.com>
Date: Fri Oct 15 09:08:10 2010 +0200
server: always call read_from_vdi_port() in a while loop
read_from_vdi_port() MUST always be called in a while loop until it returns 0.
This is needed because it can cause new data available events and its
recursion protection causes those to get lost. Calling it until it returns 0
ensures that all data has been consumed.
Example scenario where we can fail to read the available data otherwise:
- server/reds.c: vdagent_char_device_wakeup get called
by hw/spice-vmc.c because data has arrived from the guest,
- hw/spice-vmc.c: vmc_read get calls
- If the vmc_read call depletes the current buffer it calls
virtio_serial_throttle_port(&svc->port, false)
- This causes vmc_have_data to get called, which if in the
mean time another buffer has arrived causes
vdagent_char_device_wakeup to gets called again
(so recursively)
- vdagent_char_device_wakeup is protected against recursive
calling and ignores the second call (a nasty hack)
- if no other data arrives, the arrived data will not get read
diff --git a/server/reds.c b/server/reds.c
index a88ca95..8e63062 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1229,6 +1229,10 @@ static void dispatch_vdi_port_data(int port, VDIReadBuf *buf)
}
}
+/* Note this function MUST always be called in a while loop until it
+ returns 0. This is needed because it can cause new data available events
+ and its recursion protection causes those to get lost. Calling it until
+ it returns 0 ensures that all data has been consumed. */
static int read_from_vdi_port(void)
{
// FIXME: UGLY HACK. Result of spice-vmc vmc_read triggering flush of throttled data, and recalling this.
@@ -1665,7 +1669,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
agent_start = (SpiceMsgcMainAgentTokens *)message;
reds->agent_state.client_agent_started = TRUE;
reds->agent_state.send_tokens = agent_start->num_tokens; // TODO: sanitize? coming from guest!
- read_from_vdi_port();
+ while (read_from_vdi_port());
break;
}
case SPICE_MSGC_MAIN_AGENT_DATA: {
@@ -1721,7 +1725,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
token = (SpiceMsgcMainAgentTokens *)message;
reds->agent_state.send_tokens += token->num_tokens;
- read_from_vdi_port();
+ while (read_from_vdi_port());
break;
}
case SPICE_MSGC_MAIN_ATTACH_CHANNELS:
commit d37adccfa7e4586a68e7da8a225de0a50c6eeff5
Author: Hans de Goede <hdegoede at redhat.com>
Date: Wed Oct 13 08:07:36 2010 +0200
Don't crash when a client disconnects while there were pending writes
diff --git a/server/reds.c b/server/reds.c
index fcdda79..a88ca95 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1894,7 +1894,7 @@ static void reds_main_event(int fd, int event, void *data)
RedsOutgoingData *outgoing = &reds->outgoing;
if (reds_send_data()) {
reds_push();
- if (!outgoing->item) {
+ if (!outgoing->item && reds->peer) {
core->watch_update_mask(reds->peer->watch,
SPICE_WATCH_EVENT_READ);
}
commit f78ad47407689d1a19be0decfbff82c62bd4b190
Author: Hans de Goede <hdegoede at redhat.com>
Date: Tue Oct 12 15:53:35 2010 +0200
spicec-x11: add support for image copy and paste
diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
index 9751a30..0642922 100644
--- a/client/x11/platform.cpp
+++ b/client/x11/platform.cpp
@@ -107,13 +107,24 @@ static unsigned int caps_lock_mask = 0;
static unsigned int num_lock_mask = 0;
//FIXME: nicify
-static const char * const utf8_atom_names[] = {
- "UTF8_STRING",
- "text/plain;charset=UTF-8",
- "text/plain;charset=utf-8",
+struct clipboard_format_info {
+ uint32_t type;
+ const char *atom_names[16];
+ Atom atoms[16];
+ int atom_count;
};
-#define utf8_atom_count (sizeof(utf8_atom_names)/sizeof(utf8_atom_names[0]))
+static struct clipboard_format_info clipboard_formats[] = {
+ { VD_AGENT_CLIPBOARD_UTF8_TEXT, { "UTF8_STRING",
+ "text/plain;charset=UTF-8", "text/plain;charset=utf-8", NULL }, },
+ { VD_AGENT_CLIPBOARD_IMAGE_PNG, { "image/png", NULL }, },
+ { VD_AGENT_CLIPBOARD_IMAGE_BMP, { "image/bmp", "image/x-bmp",
+ "image/x-MS-bmp", "image/x-win-bitmap", NULL }, },
+ { VD_AGENT_CLIPBOARD_IMAGE_TIFF, { "image/tiff", NULL }, },
+ { VD_AGENT_CLIPBOARD_IMAGE_JPG, { "image/jpeg", NULL }, },
+};
+
+#define clipboard_format_count (sizeof(clipboard_formats)/sizeof(clipboard_formats[0]))
struct selection_request {
XEvent event;
@@ -128,15 +139,11 @@ static int32_t clipboard_data_space = 0;
static Atom clipboard_request_target = None;
static selection_request *next_selection_request = NULL;
static uint32_t clipboard_type_count = 0;
-/* TODO Add support for more types here */
-/* Warning the size of these 2 needs to be increased each time we add
- support for a new type!! */
-static uint32_t clipboard_agent_types[1];
-static Atom clipboard_x11_targets[1];
+static uint32_t clipboard_agent_types[256];
+static Atom clipboard_x11_targets[256];
static Mutex clipboard_lock;
static Atom clipboard_prop;
static Atom incr_atom;
-static Atom utf8_atoms[utf8_atom_count];
static Atom targets_atom;
static Atom multiple_atom;
static Bool handle_x_error = false;
@@ -188,16 +195,18 @@ static const char *atom_name(Atom atom)
}
static uint32_t get_clipboard_type(Atom target) {
- int i;
+ int i, j;
if (target == None)
return VD_AGENT_CLIPBOARD_NONE;
- for (i = 0; i < utf8_atom_count; i++)
- if (utf8_atoms[i] == target)
- return VD_AGENT_CLIPBOARD_UTF8_TEXT;
-
- /* TODO Add support for more types here */
+ for (i = 0; i < clipboard_format_count; i++) {
+ for (j = 0; j < clipboard_formats[i].atom_count; i++) {
+ if (clipboard_formats[i].atoms[j] == target) {
+ return clipboard_formats[i].type;
+ }
+ }
+ }
LOG_WARN("unexpected selection type %s", atom_name(target));
return VD_AGENT_CLIPBOARD_NONE;
@@ -871,7 +880,7 @@ private:
static void intern_clipboard_atoms()
{
- int i;
+ int i, j;
static bool interned = false;
if (interned) return;
@@ -880,8 +889,14 @@ static void intern_clipboard_atoms()
incr_atom = XInternAtom(x_display, "INCR", False);
multiple_atom = XInternAtom(x_display, "MULTIPLE", False);
targets_atom = XInternAtom(x_display, "TARGETS", False);
- for(i = 0; i < utf8_atom_count; i++)
- utf8_atoms[i] = XInternAtom(x_display, utf8_atom_names[i], False);
+ for(i = 0; i < clipboard_format_count; i++) {
+ for(j = 0; clipboard_formats[i].atom_names[j]; j++) {
+ clipboard_formats[i].atoms[j] =
+ XInternAtom(x_display, clipboard_formats[i].atom_names[j],
+ False);
+ }
+ clipboard_formats[i].atom_count = j;
+ }
XUnlockDisplay(x_display);
@@ -2402,23 +2417,25 @@ static void print_targets(const char *action, Atom *atoms, int c)
static void send_targets(XEvent& request_event)
{
- /* TODO Add support for more types here */
- /* Warning the size of this needs to be increased each time we add support
- for a new type, or the atom count of an existing type changes */
- Atom targets[4] = { targets_atom, };
- int i, j, target_count = 1;
+ Atom targets[256] = { targets_atom, };
+ int i, j, k, target_count = 1;
for (i = 0; i < clipboard_type_count; i++) {
- switch (clipboard_agent_types[i]) {
- case VD_AGENT_CLIPBOARD_UTF8_TEXT:
- for (j = 0; j < utf8_atom_count; j++) {
- targets[target_count] = utf8_atoms[j];
- target_count++;
+ for (j = 0; j < clipboard_format_count; j++) {
+ if (clipboard_formats[j].type != clipboard_agent_types[i]) {
+ continue;
+ }
+ for (k = 0; k < clipboard_formats[j].atom_count; k++) {
+ targets[target_count] = clipboard_formats[j].atoms[k];
+ target_count++;
+ if (target_count == sizeof(targets)/sizeof(Atom)) {
+ LOG_WARN("sendtargets: too many targets");
+ goto exit_loop;
}
- break;
- /* TODO Add support for more types here */
+ }
}
}
+exit_loop:
Window requestor_win = request_event.xselectionrequest.requestor;
Atom prop = request_event.xselectionrequest.property;
@@ -2576,9 +2593,9 @@ static Atom atom_lists_overlap(Atom *atoms1, Atom *atoms2, int l1, int l2)
static void handle_targets_notify(XEvent& event, bool incr)
{
- int len;
+ int i, len;
Lock lock(clipboard_lock);
- Atom *atoms = NULL;
+ Atom atom, *atoms = NULL;
if (!expected_targets_notifies) {
LOG_WARN("unexpected selection notify TARGETS");
@@ -2602,16 +2619,22 @@ static void handle_targets_notify(XEvent& event, bool incr)
print_targets("received", atoms, len);
clipboard_type_count = 0;
- Atom atom = atom_lists_overlap(utf8_atoms, atoms, utf8_atom_count, len);
- if (atom) {
- clipboard_agent_types[clipboard_type_count] =
- VD_AGENT_CLIPBOARD_UTF8_TEXT;
- clipboard_x11_targets[clipboard_type_count] = atom;
- clipboard_type_count++;
+ for (i = 0; i < clipboard_format_count; i++) {
+ atom = atom_lists_overlap(clipboard_formats[i].atoms, atoms,
+ clipboard_formats[i].atom_count, len);
+ if (atom) {
+ clipboard_agent_types[clipboard_type_count] =
+ clipboard_formats[i].type;
+ clipboard_x11_targets[clipboard_type_count] = atom;
+ clipboard_type_count++;
+ if (clipboard_type_count ==
+ sizeof(clipboard_agent_types)/sizeof(uint32_t)) {
+ LOG_WARN("handle_targets_notify: too many matching types");
+ break;
+ }
+ }
}
- /* TODO Add support for more types here */
-
if (clipboard_type_count)
clipboard_listener->on_clipboard_grab(clipboard_agent_types,
clipboard_type_count);
@@ -3459,23 +3482,14 @@ LocalCursor* Platform::create_default_cursor()
bool Platform::on_clipboard_grab(uint32_t *types, uint32_t type_count)
{
Lock lock(clipboard_lock);
- int i;
-
- clipboard_type_count = 0;
- for (i = 0; i < type_count; i++) {
- /* TODO Add support for more types here */
- /* Check if we support the type */
- if (types[i] != VD_AGENT_CLIPBOARD_UTF8_TEXT)
- continue;
- clipboard_agent_types[clipboard_type_count] = types[i];
- clipboard_type_count++;
+ if (type_count > sizeof(clipboard_agent_types)/sizeof(uint32_t)) {
+ LOG_WARN("on_clipboard_grab: too many types");
+ type_count = sizeof(clipboard_agent_types)/sizeof(uint32_t);
}
- if (!clipboard_type_count) {
- LOG_INFO("No supported clipboard types in agent grab");
- return false;
- }
+ memcpy(clipboard_agent_types, types, type_count * sizeof(uint32_t));
+ clipboard_type_count = type_count;
XSetSelectionOwner(x_display, clipboard_prop, platform_win, CurrentTime);
XFlush(x_display);
More information about the Spice-commits
mailing list