[Spice-commits] 8 commits - server/inputs_channel.c server/Makefile.am server/red_dispatcher.c server/red_dispatcher.h server/reds.c server/red_worker.c server/spice.h server/tests spice-common

Marc-André Lureau elmarco at kemper.freedesktop.org
Mon Oct 7 07:33:37 PDT 2013


 server/Makefile.am               |    1 
 server/inputs_channel.c          |   46 ++++++++++++++++++++++------
 server/red_dispatcher.c          |   12 ++++---
 server/red_dispatcher.h          |    3 +
 server/red_worker.c              |   63 +++++++++++++++++++++++----------------
 server/reds.c                    |    2 -
 server/spice.h                   |   51 +++++++++++++++++--------------
 server/tests/test_display_base.c |   13 +++-----
 spice-common                     |    2 -
 9 files changed, 122 insertions(+), 71 deletions(-)

New commits:
commit b18d867b319b3077d12e853897ce30be09924045
Author: Marc-André Lureau <marcandre.lureau at gmail.com>
Date:   Mon Oct 7 13:51:20 2013 +0200

    server: handle red_get_surface_cmd() error explicitely
    
    Don't ignore red_get_surface_cmd() error, and explicitely interrupt and
    free cmd before processing.

diff --git a/server/red_worker.c b/server/red_worker.c
index 8f7a1fc..8763c8e 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -11631,40 +11631,48 @@ void handle_dev_driver_unload(void *opaque, void *payload)
     worker->driver_cap_monitors_config = 0;
 }
 
+static int loadvm_command(RedWorker *worker, QXLCommandExt *ext)
+{
+    RedCursorCmd *cursor_cmd;
+    RedSurfaceCmd *surface_cmd;
+
+    switch (ext->cmd.type) {
+    case QXL_CMD_CURSOR:
+        cursor_cmd = spice_new0(RedCursorCmd, 1);
+        if (red_get_cursor_cmd(&worker->mem_slots, ext->group_id, cursor_cmd, ext->cmd.data)) {
+            free(cursor_cmd);
+            return FALSE;
+        }
+        qxl_process_cursor(worker, cursor_cmd, ext->group_id);
+        break;
+    case QXL_CMD_SURFACE:
+        surface_cmd = spice_new0(RedSurfaceCmd, 1);
+        if (red_get_surface_cmd(&worker->mem_slots, ext->group_id, surface_cmd, ext->cmd.data)) {
+            free(surface_cmd);
+            return FALSE;
+        }
+        red_process_surface(worker, surface_cmd, ext->group_id, TRUE);
+        break;
+    default:
+        spice_warning("unhandled loadvm command type (%d)", ext->cmd.type);
+    }
+
+    return TRUE;
+}
+
 void handle_dev_loadvm_commands(void *opaque, void *payload)
 {
     RedWorkerMessageLoadvmCommands *msg = payload;
     RedWorker *worker = opaque;
     uint32_t i;
-    RedCursorCmd *cursor_cmd;
-    RedSurfaceCmd *surface_cmd;
     uint32_t count = msg->count;
     QXLCommandExt *ext = msg->ext;
 
     spice_info("loadvm_commands");
     for (i = 0 ; i < count ; ++i) {
-        switch (ext[i].cmd.type) {
-        case QXL_CMD_CURSOR:
-            cursor_cmd = spice_new0(RedCursorCmd, 1);
-            if (red_get_cursor_cmd(&worker->mem_slots, ext[i].group_id,
-                                   cursor_cmd, ext[i].cmd.data)) {
-                /* XXX allow failure in loadvm? */
-                spice_warning("failed loadvm command type (%d)",
-                              ext[i].cmd.type);
-                free(cursor_cmd);
-                continue;
-            }
-            qxl_process_cursor(worker, cursor_cmd, ext[i].group_id);
-            break;
-        case QXL_CMD_SURFACE:
-            surface_cmd = spice_new0(RedSurfaceCmd, 1);
-            red_get_surface_cmd(&worker->mem_slots, ext[i].group_id,
-                                surface_cmd, ext[i].cmd.data);
-            red_process_surface(worker, surface_cmd, ext[i].group_id, TRUE);
-            break;
-        default:
-            spice_warning("unhandled loadvm command type (%d)", ext[i].cmd.type);
-            break;
+        if (!loadvm_command(worker, &ext[i])) {
+            /* XXX allow failure in loadvm? */
+            spice_warning("failed loadvm command type (%d)", ext[i].cmd.type);
         }
     }
 }
commit 1f12fa72cc62fe1dcf213a03d9fb446585469c0c
Author: Marc-André Lureau <marcandre.lureau at gmail.com>
Date:   Fri Oct 4 20:59:26 2013 +0200

    server: plug some leaks on error
    
    Plug what looks like memory leaks, that could be potentially be
    triggered by a misbehaving guest.

diff --git a/server/red_worker.c b/server/red_worker.c
index f5a5553..8f7a1fc 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -4949,10 +4949,13 @@ static int red_process_cursor(RedWorker *worker, uint32_t max_pipe_size, int *ri
         case QXL_CMD_CURSOR: {
             RedCursorCmd *cursor = spice_new0(RedCursorCmd, 1);
 
-            if (!red_get_cursor_cmd(&worker->mem_slots, ext_cmd.group_id,
+            if (red_get_cursor_cmd(&worker->mem_slots, ext_cmd.group_id,
                                     cursor, ext_cmd.cmd.data)) {
-                qxl_process_cursor(worker, cursor, ext_cmd.group_id);
+                free(cursor);
+                break;
             }
+
+            qxl_process_cursor(worker, cursor, ext_cmd.group_id);
             break;
         }
         default:
@@ -5058,6 +5061,7 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
 
             if (red_get_surface_cmd(&worker->mem_slots, ext_cmd.group_id,
                                     surface, ext_cmd.cmd.data)) {
+                free(surface);
                 break;
             }
             red_process_surface(worker, surface, ext_cmd.group_id, FALSE);
@@ -11647,6 +11651,7 @@ void handle_dev_loadvm_commands(void *opaque, void *payload)
                 /* XXX allow failure in loadvm? */
                 spice_warning("failed loadvm command type (%d)",
                               ext[i].cmd.type);
+                free(cursor_cmd);
                 continue;
             }
             qxl_process_cursor(worker, cursor_cmd, ext[i].group_id);
commit 3bb7db9c5d2e30e073adf5bd6878b121f72b8aae
Author: Marc-André Lureau <marcandre.lureau at gmail.com>
Date:   Mon Oct 7 11:49:17 2013 +0200

    server: inputs s/relase/release

diff --git a/server/inputs_channel.c b/server/inputs_channel.c
index bda525a..dd8f5ae 100644
--- a/server/inputs_channel.c
+++ b/server/inputs_channel.c
@@ -479,7 +479,7 @@ static int inputs_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, ui
     return TRUE;
 }
 
-static void inputs_relase_keys(void)
+static void inputs_release_keys(void)
 {
     int i;
     SpiceKbdState *st = keyboard->st;
@@ -507,7 +507,7 @@ static void inputs_channel_on_disconnect(RedChannelClient *rcc)
     if (!rcc) {
         return;
     }
-    inputs_relase_keys();
+    inputs_release_keys();
 }
 
 static void inputs_pipe_add_init(RedChannelClient *rcc)
commit 2d28da3c17f115fb1be4e5045e4a7a67fd1158b9
Author: Marc-André Lureau <marcandre.lureau at gmail.com>
Date:   Fri Oct 4 18:48:41 2013 +0200

    server: release all pressed keys on client disconnect
    
    Releasing modifiers keys unconditionally on disconnect leads to
    unexpected guest wakeups. To improve the situation, the server can
    release only the pressed keys, which will prevent the wakeup in most
    cases.
    
    Furthermore, it's not sufficient to release only the modifiers keys.
    Any key should be released on client disconnect to avoid sticky key
    press across connections.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=871240

diff --git a/server/inputs_channel.c b/server/inputs_channel.c
index 931dac1..bda525a 100644
--- a/server/inputs_channel.c
+++ b/server/inputs_channel.c
@@ -27,6 +27,7 @@
 #include <spice/macros.h>
 #include <spice/vd_agent.h>
 #include <spice/protocol.h>
+#include <stdbool.h>
 
 #include "common/marshaller.h"
 #include "common/messages.h"
@@ -53,7 +54,11 @@
     (4096 + (REDS_AGENT_WINDOW_SIZE + REDS_NUM_INTERNAL_AGENT_MESSAGES) * SPICE_AGENT_MAX_DATA_SIZE)
 
 struct SpiceKbdState {
-    int dummy;
+    bool push_ext;
+
+    /* track key press state */
+    bool key[0x7f];
+    bool key_ext[0x7f];
 };
 
 struct SpiceMouseState {
@@ -221,6 +226,16 @@ static void kbd_push_scan(SpiceKbdInstance *sin, uint8_t scan)
         return;
     }
     sif = SPICE_CONTAINEROF(sin->base.sif, SpiceKbdInterface, base);
+
+    /* track XT scan code set 1 key state */
+    if (scan == 0xe0) {
+        sin->st->push_ext = TRUE;
+    } else {
+        bool *state = sin->st->push_ext ? sin->st->key : sin->st->key_ext;
+        sin->st->push_ext = FALSE;
+        state[scan & 0x7f] = !(scan & 0x80);
+    }
+
     sif->push_scan_freg(sin, scan);
 }
 
@@ -466,12 +481,25 @@ static int inputs_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, ui
 
 static void inputs_relase_keys(void)
 {
-    kbd_push_scan(keyboard, 0x2a | 0x80); //LSHIFT
-    kbd_push_scan(keyboard, 0x36 | 0x80); //RSHIFT
-    kbd_push_scan(keyboard, 0xe0); kbd_push_scan(keyboard, 0x1d | 0x80); //RCTRL
-    kbd_push_scan(keyboard, 0x1d | 0x80); //LCTRL
-    kbd_push_scan(keyboard, 0xe0); kbd_push_scan(keyboard, 0x38 | 0x80); //RALT
-    kbd_push_scan(keyboard, 0x38 | 0x80); //LALT
+    int i;
+    SpiceKbdState *st = keyboard->st;
+
+    for (i = 0; i < SPICE_N_ELEMENTS(st->key); i++) {
+        if (!st->key[i])
+            continue;
+
+        st->key[i] = FALSE;
+        kbd_push_scan(keyboard, i | 0x80);
+    }
+
+    for (i = 0; i < SPICE_N_ELEMENTS(st->key_ext); i++) {
+        if (!st->key_ext[i])
+            continue;
+
+        st->key_ext[i] = FALSE;
+        kbd_push_scan(keyboard, 0xe0);
+        kbd_push_scan(keyboard, i | 0x80);
+    }
 }
 
 static void inputs_channel_on_disconnect(RedChannelClient *rcc)
commit fe0941fb025d02a750bdaee296f93e8024375d25
Author: Marc-André Lureau <marcandre.lureau at gmail.com>
Date:   Thu Oct 3 22:52:38 2013 +0200

    server: mark deprecated symbols

diff --git a/server/Makefile.am b/server/Makefile.am
index 815f65e..8cbd87b 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -2,6 +2,7 @@ NULL =
 SUBDIRS = . tests
 
 AM_CPPFLAGS =					\
+	-DSPICE_SERVER_INTERNAL			\
 	-DRED_STATISTICS			\
 	$(CELT051_CFLAGS)			\
 	$(COMMON_CFLAGS)			\
diff --git a/server/spice.h b/server/spice.h
index 6fbb7b2..b645112 100644
--- a/server/spice.h
+++ b/server/spice.h
@@ -22,9 +22,15 @@
 #include <sys/socket.h>
 #include <spice/qxl_dev.h>
 #include <spice/vd_agent.h>
+#include <spice/macros.h>
 
 #define SPICE_SERVER_VERSION 0x000c04 /* release 0.12.4 */
 
+#ifdef SPICE_SERVER_INTERNAL
+#undef SPICE_GNUC_DEPRECATED
+#define SPICE_GNUC_DEPRECATED
+#endif
+
 /* interface base type */
 
 typedef struct SpiceBaseInterface SpiceBaseInterface;
@@ -69,9 +75,10 @@ typedef struct SpiceChannelEventInfo {
     int id;
     int flags;
     /* deprecated, can't hold ipv6 addresses, kept for backward compatibility */
-    struct sockaddr laddr;
-    struct sockaddr paddr;
-    socklen_t llen, plen;
+    struct sockaddr laddr SPICE_GNUC_DEPRECATED;
+    struct sockaddr paddr SPICE_GNUC_DEPRECATED;
+    socklen_t llen SPICE_GNUC_DEPRECATED;
+    socklen_t plen SPICE_GNUC_DEPRECATED;
     /* should be used if (flags & SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT) */
     struct sockaddr_storage laddr_ext;
     struct sockaddr_storage paddr_ext;
@@ -113,32 +120,32 @@ struct QXLWorker {
     uint32_t minor_version;
     uint32_t major_version;
     /* These calls are deprecated. Please use the spice_qxl_* calls instead */
-    void (*wakeup)(QXLWorker *worker);
-    void (*oom)(QXLWorker *worker);
-    void (*start)(QXLWorker *worker);
-    void (*stop)(QXLWorker *worker);
+    void (*wakeup)(QXLWorker *worker) SPICE_GNUC_DEPRECATED;
+    void (*oom)(QXLWorker *worker) SPICE_GNUC_DEPRECATED;
+    void (*start)(QXLWorker *worker) SPICE_GNUC_DEPRECATED;
+    void (*stop)(QXLWorker *worker) SPICE_GNUC_DEPRECATED;
     void (*update_area)(QXLWorker *qxl_worker, uint32_t surface_id,
                        struct QXLRect *area, struct QXLRect *dirty_rects,
-                       uint32_t num_dirty_rects, uint32_t clear_dirty_region);
-    void (*add_memslot)(QXLWorker *worker, QXLDevMemSlot *slot);
-    void (*del_memslot)(QXLWorker *worker, uint32_t slot_group_id, uint32_t slot_id);
-    void (*reset_memslots)(QXLWorker *worker);
-    void (*destroy_surfaces)(QXLWorker *worker);
-    void (*destroy_primary_surface)(QXLWorker *worker, uint32_t surface_id);
+                       uint32_t num_dirty_rects, uint32_t clear_dirty_region) SPICE_GNUC_DEPRECATED;
+    void (*add_memslot)(QXLWorker *worker, QXLDevMemSlot *slot) SPICE_GNUC_DEPRECATED;
+    void (*del_memslot)(QXLWorker *worker, uint32_t slot_group_id, uint32_t slot_id) SPICE_GNUC_DEPRECATED;
+    void (*reset_memslots)(QXLWorker *worker) SPICE_GNUC_DEPRECATED;
+    void (*destroy_surfaces)(QXLWorker *worker) SPICE_GNUC_DEPRECATED;
+    void (*destroy_primary_surface)(QXLWorker *worker, uint32_t surface_id) SPICE_GNUC_DEPRECATED;
     void (*create_primary_surface)(QXLWorker *worker, uint32_t surface_id,
-                                   QXLDevSurfaceCreate *surface);
-    void (*reset_image_cache)(QXLWorker *worker);
-    void (*reset_cursor)(QXLWorker *worker);
-    void (*destroy_surface_wait)(QXLWorker *worker, uint32_t surface_id);
-    void (*loadvm_commands)(QXLWorker *worker, struct QXLCommandExt *ext, uint32_t count);
+                                   QXLDevSurfaceCreate *surface) SPICE_GNUC_DEPRECATED;
+    void (*reset_image_cache)(QXLWorker *worker) SPICE_GNUC_DEPRECATED;
+    void (*reset_cursor)(QXLWorker *worker) SPICE_GNUC_DEPRECATED;
+    void (*destroy_surface_wait)(QXLWorker *worker, uint32_t surface_id) SPICE_GNUC_DEPRECATED;
+    void (*loadvm_commands)(QXLWorker *worker, struct QXLCommandExt *ext, uint32_t count) SPICE_GNUC_DEPRECATED;
 };
 
 void spice_qxl_wakeup(QXLInstance *instance);
 void spice_qxl_oom(QXLInstance *instance);
-void spice_qxl_start(QXLInstance *instance); /* deprecated since 0.11.2
-                                                spice_server_vm_start replaces it */
-void spice_qxl_stop(QXLInstance *instance);  /* deprecated since 0.11.2
-                                                spice_server_vm_stop replaces it */
+/* deprecated since 0.11.2, spice_server_vm_start replaces it */
+void spice_qxl_start(QXLInstance *instance) SPICE_GNUC_DEPRECATED;
+/* deprecated since 0.11.2 spice_server_vm_stop replaces it */
+void spice_qxl_stop(QXLInstance *instance) SPICE_GNUC_DEPRECATED;
 void spice_qxl_update_area(QXLInstance *instance, uint32_t surface_id,
                    struct QXLRect *area, struct QXLRect *dirty_rects,
                    uint32_t num_dirty_rects, uint32_t clear_dirty_region);
commit e93b2bb188d43dfe61c1463dc419350e53c8aaca
Author: Marc-André Lureau <marcandre.lureau at gmail.com>
Date:   Thu Oct 3 22:56:19 2013 +0200

    server/tests: avoid using deprecated symbols

diff --git a/server/tests/test_display_base.c b/server/tests/test_display_base.c
index 20c0e47..ccdd2f2 100644
--- a/server/tests/test_display_base.c
+++ b/server/tests/test_display_base.c
@@ -358,7 +358,6 @@ static SimpleSurfaceCmd *destroy_surface(int surface_id)
 static void create_primary_surface(Test *test, uint32_t width,
                                    uint32_t height)
 {
-    QXLWorker *qxl_worker = test->qxl_worker;
     QXLDevSurfaceCreate surface = { 0, };
 
     ASSERT(height <= MAX_HEIGHT);
@@ -380,7 +379,7 @@ static void create_primary_surface(Test *test, uint32_t width,
     test->width = width;
     test->height = height;
 
-    qxl_worker->create_primary_surface(qxl_worker, 0, &surface);
+    spice_qxl_create_primary_surface(&test->qxl_instance, 0, &surface);
 }
 
 QXLDevMemSlot slot = {
@@ -407,9 +406,9 @@ static void attache_worker(QXLInstance *qin, QXLWorker *_qxl_worker)
     }
     printf("%s\n", __func__);
     test->qxl_worker = _qxl_worker;
-    test->qxl_worker->add_memslot(test->qxl_worker, &slot);
+    spice_qxl_add_memslot(&test->qxl_instance, &slot);
     create_primary_surface(test, DEFAULT_WIDTH, DEFAULT_HEIGHT);
-    test->qxl_worker->start(test->qxl_worker);
+    spice_server_vm_start(test->server);
 }
 
 static void set_compression_level(QXLInstance *qin, int level)
@@ -511,7 +510,7 @@ static void produce_command(Test *test)
                 .bottom = (test->target_surface == 0 ? test->primary_height : test->height)
             };
             if (rect.right > 0 && rect.bottom > 0) {
-                qxl_worker->update_area(qxl_worker, test->target_surface, &rect, NULL, 0, 1);
+                spice_qxl_update_area(&test->qxl_instance, test->target_surface, &rect, NULL, 0, 1);
             }
             break;
         }
@@ -584,7 +583,7 @@ static void produce_command(Test *test)
         }
 
         case DESTROY_PRIMARY:
-            qxl_worker->destroy_primary_surface(qxl_worker, 0);
+            spice_qxl_destroy_primary_surface(&test->qxl_instance, 0);
             break;
 
         case CREATE_PRIMARY:
@@ -614,7 +613,7 @@ static void do_wakeup(void *opaque)
     }
 
     test->core->timer_start(test->wakeup_timer, test->wakeup_ms);
-    test->qxl_worker->wakeup(test->qxl_worker);
+    spice_qxl_wakeup(&test->qxl_instance);
 }
 
 static void release_resource(QXLInstance *qin, struct QXLReleaseInfoExt release_info)
commit 1d18b7e98ab268c755933061d77ccc7a981815e2
Author: Marc-André Lureau <marcandre.lureau at gmail.com>
Date:   Thu Oct 3 22:54:30 2013 +0200

    server: set dispatcher before calling attache_worker
    
    This allows to call spice_qxl_add_memslot during attache_worker(), like
    done in the tests.

diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index 03a4c4a..fd6c2e1 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -1072,7 +1072,7 @@ static RedChannel *red_dispatcher_cursor_channel_create(RedDispatcher *dispatche
     return cursor_channel;
 }
 
-RedDispatcher *red_dispatcher_init(QXLInstance *qxl)
+void red_dispatcher_init(QXLInstance *qxl)
 {
     RedDispatcher *red_dispatcher;
     RedWorkerMessage message;
@@ -1085,6 +1085,8 @@ RedDispatcher *red_dispatcher_init(QXLInstance *qxl)
     sigset_t curr_sig_mask;
     ClientCbs client_cbs = { NULL, };
 
+    spice_return_if_fail(qxl->st->dispatcher == NULL);
+
     quic_init();
     sw_canvas_init();
 #ifdef USE_OPENGL
@@ -1175,12 +1177,12 @@ RedDispatcher *red_dispatcher_init(QXLInstance *qxl)
         reds_register_channel(cursor_channel);
     }
 
-    qxl->st->qif->attache_worker(qxl, &red_dispatcher->base);
-    qxl->st->qif->set_compression_level(qxl, calc_compression_level());
-
+    qxl->st->dispatcher = red_dispatcher;
     red_dispatcher->next = dispatchers;
     dispatchers = red_dispatcher;
-    return red_dispatcher;
+
+    qxl->st->qif->attache_worker(qxl, &red_dispatcher->base);
+    qxl->st->qif->set_compression_level(qxl, calc_compression_level());
 }
 
 struct Dispatcher *red_dispatcher_get_dispatcher(RedDispatcher *red_dispatcher)
diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
index 7d23b11..907b7c7 100644
--- a/server/red_dispatcher.h
+++ b/server/red_dispatcher.h
@@ -21,9 +21,10 @@
 #include "red_channel.h"
 
 struct RedChannelClient;
+struct RedDispatcher;
 typedef struct AsyncCommand AsyncCommand;
 
-struct RedDispatcher *red_dispatcher_init(QXLInstance *qxl);
+void red_dispatcher_init(QXLInstance *qxl);
 
 void red_dispatcher_set_mm_time(uint32_t);
 void red_dispatcher_on_ic_change(void);
diff --git a/server/reds.c b/server/reds.c
index 0f81a32..1456b75 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3771,7 +3771,7 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
         qxl = SPICE_CONTAINEROF(sin, QXLInstance, base);
         qxl->st = spice_new0(QXLState, 1);
         qxl->st->qif = SPICE_CONTAINEROF(interface, QXLInterface, base);
-        qxl->st->dispatcher = red_dispatcher_init(qxl);
+        red_dispatcher_init(qxl);
 
     } else if (strcmp(interface->type, SPICE_INTERFACE_TABLET) == 0) {
         spice_info("SPICE_INTERFACE_TABLET");
commit 961fa773654b51dc8631e1113371b1c3feb21aaf
Author: Marc-André Lureau <marcandre.lureau at gmail.com>
Date:   Mon Oct 7 16:32:06 2013 +0200

    Update spice-common

diff --git a/spice-common b/spice-common
index e443c9f..7e8ba10 160000
--- a/spice-common
+++ b/spice-common
@@ -1 +1 @@
-Subproject commit e443c9f6039407633d38a0eba03c344272ac8559
+Subproject commit 7e8ba10779a3fb11d587e8a59fe389acd2412dd0


More information about the Spice-commits mailing list