[Spice-commits] 10 commits - NEWS common/lines.c configure.ac server/Makefile.am server/char_device.h server/inputs_channel.c server/jpeg_encoder.c server/red_channel.c server/red_dispatcher.c server/reds.c server/smartcard.c server/spicevmc.c server/usbredir.c

Hans de Goede jwrdegoede at kemper.freedesktop.org
Thu Aug 25 06:22:06 PDT 2011


 NEWS                    |   10 +
 common/lines.c          |    3 
 configure.ac            |    4 
 server/Makefile.am      |    2 
 server/char_device.h    |    6 -
 server/inputs_channel.c |    6 -
 server/jpeg_encoder.c   |    4 
 server/red_channel.c    |    1 
 server/red_dispatcher.c |    2 
 server/reds.c           |    6 -
 server/smartcard.c      |    6 -
 server/spicevmc.c       |  275 +++++++++++++++++++++++++++++++++++++++++++++++
 server/usbredir.c       |  278 ------------------------------------------------
 13 files changed, 307 insertions(+), 296 deletions(-)

New commits:
commit 75dacb8d62b30cd08a3e6286d6c36e77e3e21254
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Aug 25 14:19:27 2011 +0200

    Release 0.9.1

diff --git a/NEWS b/NEWS
index a2ca72c..91a6f0e 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,14 @@
+Major changes in 0.9.1:
+=======================
+* !Development Release!
+* Multi-client support, disabled by default (experimental!) set the
+  environment variable PICE_DEBUG_ALLOW_MC before starting qemu to enable
+* Add support for adding generic spicevmc chardev passthrough channels
+* Add USB redirection channel (using generic spicevmc chardev passthrough)
+* Various bugfixes / cleanups
+
 Major changes in 0.9.0:
+=======================
 * !Development Release!
 * volume synchronization between client and guest (client->guest only)
 * turbo-jpeg used to avoid expensive color conversion in mjpeg encoder.
diff --git a/configure.ac b/configure.ac
index 486e323..2afc559 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2,7 +2,7 @@ AC_PREREQ([2.57])
 
 m4_define([SPICE_MAJOR], 0)
 m4_define([SPICE_MINOR], 9)
-m4_define([SPICE_MICRO], 0)
+m4_define([SPICE_MICRO], 1)
 
 AC_INIT(spice, [SPICE_MAJOR.SPICE_MINOR.SPICE_MICRO], [], spice)
 
@@ -132,7 +132,7 @@ AM_CONDITIONAL(SUPPORT_CLIENT, test "x$enable_client" = "xyes")
 dnl =========================================================================
 dnl Check deps
 
-PKG_CHECK_MODULES(PROTOCOL, spice-protocol >= 0.8.1)
+PKG_CHECK_MODULES(PROTOCOL, spice-protocol >= 0.9.0)
 AC_SUBST(PROTOCOL_CFLAGS)
 
 AC_CHECK_LIBM
commit 731c44d75234b5a35d267d67418db233dcdec0c6
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Aug 25 14:52:42 2011 +0200

    fix more inverted memset parameters
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/server/inputs_channel.c b/server/inputs_channel.c
index 653910e..a6dec83 100644
--- a/server/inputs_channel.c
+++ b/server/inputs_channel.c
@@ -523,12 +523,12 @@ static void key_modifiers_sender(void *opaque)
 void inputs_init(void)
 {
     ChannelCbs channel_cbs;
-    ClientCbs client_cbs = {0,};
+    ClientCbs client_cbs;
 
     ASSERT(!g_inputs_channel);
 
-    memset(&channel_cbs, sizeof(channel_cbs), 0);
-    memset(&client_cbs, sizeof(client_cbs), 0);
+    memset(&channel_cbs, 0, sizeof(channel_cbs));
+    memset(&client_cbs, 0, sizeof(client_cbs));
 
     channel_cbs.config_socket = inputs_channel_config_socket;
     channel_cbs.on_disconnect = inputs_channel_on_disconnect;
diff --git a/server/smartcard.c b/server/smartcard.c
index 2ff1310..056caa6 100644
--- a/server/smartcard.c
+++ b/server/smartcard.c
@@ -506,12 +506,12 @@ SmartCardChannel *g_smartcard_channel;
 static void smartcard_init(void)
 {
     ChannelCbs channel_cbs;
-    ClientCbs client_cbs = {0,};
+    ClientCbs client_cbs;
 
     ASSERT(!g_smartcard_channel);
 
-    memset(&channel_cbs, sizeof(channel_cbs), 0);
-    memset(&client_cbs, sizeof(client_cbs), 0);
+    memset(&channel_cbs, 0, sizeof(channel_cbs));
+    memset(&client_cbs, 0, sizeof(client_cbs));
 
     channel_cbs.config_socket = smartcard_channel_client_config_socket;
     channel_cbs.on_disconnect = smartcard_channel_on_disconnect;
commit 8c9404374a6066cf5f5567b93e2e5e9debd2c722
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Aug 25 14:35:33 2011 +0200

    fix leak in do_jpeg_encode
    
    Issue found by the Coverity scanner.
    
    HDG: Fixup don't free RGB24_line if it was not allocated by do_jpeg_encode
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/server/jpeg_encoder.c b/server/jpeg_encoder.c
index 85ef54e..a308437 100644
--- a/server/jpeg_encoder.c
+++ b/server/jpeg_encoder.c
@@ -194,6 +194,10 @@ static void do_jpeg_encode(JpegEncoder *jpeg, uint8_t *lines, unsigned int num_l
         row_pointer[0] = RGB24_line;
         jpeg_write_scanlines(&jpeg->cinfo, row_pointer, 1);
     }
+
+    if (jpeg->cur_image.type != JPEG_IMAGE_TYPE_RGB24) {
+        free(RGB24_line);
+    }
 }
 
 int jpeg_encode(JpegEncoderContext *jpeg, int quality, JpegEncoderImageType type,
commit a09052b83cf4003b5eb5d486e498e8bc70e8262f
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Aug 25 14:35:01 2011 +0200

    fix memory leak in error path
    
    Issue found by the Coverity scanner

diff --git a/common/lines.c b/common/lines.c
index 8731fb7..e2349e8 100644
--- a/common/lines.c
+++ b/common/lines.c
@@ -1512,7 +1512,7 @@ miZeroLine (GCPtr pGC, int mode,        /* Origin or Previous */
     pspanInit = (DDXPointRec *)xalloc (list_len * sizeof (DDXPointRec));
     pwidthInit = (int *)xalloc (list_len * sizeof (int));
     if (!pspanInit || !pwidthInit)
-        return;
+        goto out;
 
     Nspans = 0;
     new_span = TRUE;
@@ -1686,6 +1686,7 @@ miZeroLine (GCPtr pGC, int mode,        /* Origin or Previous */
     if (Nspans > 0)
         (*pGC->ops->FillSpans) (pGC, Nspans, pspanInit, pwidthInit, FALSE, TRUE);
 
+out:
     xfree (pwidthInit);
     xfree (pspanInit);
 }
commit 2abd83c203ed7363b8c762dd4affa9835ca6dfca
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Aug 25 14:34:04 2011 +0200

    fix inverted memset parameters
    
    Issue found by the Coverity scanner.

diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index c00dc58..721c79c 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -426,7 +426,7 @@ static void red_dispatcher_create_primary_surface_complete(RedDispatcher *dispat
     dispatcher->primary_active = TRUE;
 
     update_client_mouse_allowed();
-    memset(&dispatcher->surface_create, sizeof(QXLDevSurfaceCreate), 0);
+    memset(&dispatcher->surface_create, 0, sizeof(QXLDevSurfaceCreate));
 }
 
 static void
commit c030382abbd14c3ae09db030719e52efa8600f15
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Aug 25 12:16:06 2011 +0200

    Rename usbredir channel code to spicevmc
    
    While discussing various things with Alon in Vancouver, it came up that
    having a channel which simply passes through data coming out of a qemu
    chardev frontend unmodified, like the usbredir channel does, can be used
    for a lot of other cases too. To facilitate this the usbredir channel code
    will be turned into a generic spicevmc channel, which is just a passthrough
    to the client, from the spicevmc chardev.
    
    This patch renames usbredir.c to spicevmc.c and changes the prefix of all
    functions / structs to match. This should make clear that the code is not
    usbredir specific.
    
    Some examples of why having a generic spicevmc pass through is good:
    1) We could add a monitor channel, allowing access to the qemu monitor from
    the spice client, since the monitor is a chardev frontend we could re-use
    the generic spicevmc channel server code, so all that is needed to add this
    (server side) would be reserving a new channel id for this.
    
    2) We could allow users to come up with new channels of their own, without
    requiring qemu or server modification. The idea is to allow doing something
    like this on the qemu startup cmdline:
    -chardev spicevmc,name=generic,channelid=128
    
    To ensure these new "generic" channels cannot conflict with newly added
    official types, they must start at the SPICE_CHANNEL_USER_DEFINED_START value
    (128).
    
    These new user defined channels could then either be used with a special
    modified client, with client plugins (if we add support for those), or
    by exporting them on the client side for use by an external ap, see below.
    
    3) We could also add support to the client to make user-defined channels
    end in a unix socket / pipe, allowing handling of the data by an external app,
    we could for example have a new spice client cmdline argument like this:
    --custom-channel unixsocket=/tmp/mysocket,channelid=128
    
    This would allow for something like:
    $random app on guest -> virtio-serial -> spicevmc chardev ->
     -> spicevmc channel -> unix socket -> $random app on client
    
    4) On hind sight this could also have been used for the smartcard stuff,
    with a 1 channel / reader model, rather then the current multiplexing code
    where we've our own multiplexing protocol wrapper over the libcacard
    smartcard protocol.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/server/Makefile.am b/server/Makefile.am
index b9be242..a7cdd84 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -95,7 +95,7 @@ libspice_server_la_SOURCES =			\
 	spice-experimental.h			\
 	spice.h					\
 	stat.h					\
-	usbredir.c				\
+	spicevmc.c				\
 	zlib_encoder.c				\
 	zlib_encoder.h				\
 	$(NULL)
diff --git a/server/char_device.h b/server/char_device.h
index 4b55869..bdb32ae 100644
--- a/server/char_device.h
+++ b/server/char_device.h
@@ -7,8 +7,8 @@ struct SpiceCharDeviceState {
     void (*wakeup)(SpiceCharDeviceInstance *sin);
 };
 
-int usbredir_device_connect(SpiceCharDeviceInstance *char_device);
-void usbredir_device_disconnect(SpiceCharDeviceInstance *char_device);
+void spicevmc_device_connect(SpiceCharDeviceInstance *sin,
+                             uint8_t channel_type);
+void spicevmc_device_disconnect(SpiceCharDeviceInstance *char_device);
 
 #endif // __CHAR_DEVICE_H__
-
diff --git a/server/reds.c b/server/reds.c
index d140afd..c58586a 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3283,9 +3283,7 @@ static int spice_server_char_device_add_interface(SpiceServer *s,
     }
 #endif
     else if (strcmp(char_device->subtype, SUBTYPE_USBREDIR) == 0) {
-        if (usbredir_device_connect(char_device) == -1) {
-            return -1;
-        }
+        spicevmc_device_connect(char_device, SPICE_CHANNEL_USBREDIR);
     }
     return 0;
 }
@@ -3307,7 +3305,7 @@ static void spice_server_char_device_remove_interface(SpiceBaseInstance *sin)
     }
 #endif
     else if (strcmp(char_device->subtype, SUBTYPE_USBREDIR) == 0) {
-        usbredir_device_disconnect(char_device);
+        spicevmc_device_disconnect(char_device);
     }
 }
 
diff --git a/server/spicevmc.c b/server/spicevmc.c
new file mode 100644
index 0000000..9ccc0d1
--- /dev/null
+++ b/server/spicevmc.c
@@ -0,0 +1,275 @@
+/* spice-server spicevmc passthrough channel code
+
+   Copyright (C) 2011 Red Hat, Inc.
+
+   Red Hat Authors:
+   Hans de Goede <hdegoede at redhat.com>
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <assert.h>
+
+#include "server/char_device.h"
+#include "server/red_channel.h"
+#include "server/reds.h"
+
+/* 64K should be enough for all but the largest writes + 32 bytes hdr */
+#define BUF_SIZE (64 * 1024 + 32)
+
+typedef struct SpiceVmcPipeItem {
+    PipeItem base;
+    /* writes which don't fit this will get split, this is not a problem */
+    uint8_t buf[BUF_SIZE];
+    uint32_t buf_used;
+} SpiceVmcPipeItem;
+
+typedef struct SpiceVmcState {
+    RedChannel channel; /* Must be the first item */
+    RedChannelClient *rcc;
+    SpiceCharDeviceState chardev_st;
+    SpiceCharDeviceInstance *chardev_sin;
+    SpiceVmcPipeItem *pipe_item;
+    uint8_t *rcv_buf;
+    uint32_t rcv_buf_size;
+    int rcv_buf_in_use;
+} SpiceVmcState;
+
+static void spicevmc_chardev_wakeup(SpiceCharDeviceInstance *sin)
+{
+    SpiceVmcState *state;
+    SpiceCharDeviceInterface *sif;
+    int n;
+
+    state = SPICE_CONTAINEROF(sin->st, SpiceVmcState, chardev_st);
+    sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
+
+    if (!state->rcc) {
+        return;
+    }
+
+    do {
+        if (!state->pipe_item) {
+            state->pipe_item = spice_malloc(sizeof(SpiceVmcPipeItem));
+            red_channel_pipe_item_init(&state->channel,
+                                       &state->pipe_item->base, 0);
+        }
+
+        n = sif->read(sin, state->pipe_item->buf,
+                      sizeof(state->pipe_item->buf));
+        if (n > 0) {
+            state->pipe_item->buf_used = n;
+            red_channel_client_pipe_add_push(state->rcc,
+                                             &state->pipe_item->base);
+            state->pipe_item = NULL;
+        }
+    } while (n > 0);
+}
+
+static int spicevmc_red_channel_client_config_socket(RedChannelClient *rcc)
+{
+    return TRUE;
+}
+
+static void spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
+{
+    SpiceVmcState *state;
+    SpiceCharDeviceInstance *sin;
+    SpiceCharDeviceInterface *sif;
+
+    if (!rcc) {
+        return;
+    }
+
+    state = SPICE_CONTAINEROF(rcc->channel, SpiceVmcState, channel);
+    sin = state->chardev_sin;
+    sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
+
+    red_channel_client_destroy(rcc);
+    state->rcc = NULL;
+    if (sif->state) {
+        sif->state(sin, 0);
+    }
+}
+
+static int spicevmc_red_channel_client_handle_message(RedChannelClient *rcc,
+    SpiceDataHeader *header, uint8_t *msg)
+{
+    SpiceVmcState *state;
+    SpiceCharDeviceInstance *sin;
+    SpiceCharDeviceInterface *sif;
+
+    state = SPICE_CONTAINEROF(rcc->channel, SpiceVmcState, channel);
+    sin = state->chardev_sin;
+    sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
+
+    if (header->type != SPICE_MSGC_SPICEVMC_DATA) {
+        return red_channel_client_handle_message(rcc, header->size,
+                                                 header->type, msg);
+    }
+
+    /*
+     * qemu spicevmc will consume everything we give it, no need for
+     * flow control checks (or to use a pipe).
+     */
+    sif->write(sin, msg, header->size);
+
+    return TRUE;
+}
+
+static uint8_t *spicevmc_red_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
+    SpiceDataHeader *msg_header)
+{
+    SpiceVmcState *state;
+
+    state = SPICE_CONTAINEROF(rcc->channel, SpiceVmcState, channel);
+
+    assert(!state->rcv_buf_in_use);
+
+    if (msg_header->size > state->rcv_buf_size) {
+        state->rcv_buf = spice_realloc(state->rcv_buf, msg_header->size);
+        state->rcv_buf_size = msg_header->size;
+    }
+
+    state->rcv_buf_in_use = 1;
+
+    return state->rcv_buf;
+}
+
+static void spicevmc_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
+    SpiceDataHeader *msg_header, uint8_t *msg)
+{
+    SpiceVmcState *state;
+
+    state = SPICE_CONTAINEROF(rcc->channel, SpiceVmcState, channel);
+
+    /* NOOP, we re-use the buffer every time and only free it on destruction */
+    state->rcv_buf_in_use = 0;
+}
+
+static void spicevmc_red_channel_hold_pipe_item(RedChannelClient *rcc,
+    PipeItem *item)
+{
+    /* NOOP */
+}
+
+static void spicevmc_red_channel_send_item(RedChannelClient *rcc,
+    PipeItem *item)
+{
+    SpiceVmcPipeItem *i = SPICE_CONTAINEROF(item, SpiceVmcPipeItem, base);
+    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
+
+    red_channel_client_init_send_data(rcc, SPICE_MSG_SPICEVMC_DATA, item);
+    spice_marshaller_add_ref(m, i->buf, i->buf_used);
+    red_channel_client_begin_send_message(rcc);
+}
+
+static void spicevmc_red_channel_release_pipe_item(RedChannelClient *rcc,
+    PipeItem *item, int item_pushed)
+{
+    free(item);
+}
+
+static void spicevmc_connect(RedChannel *channel, RedClient *client,
+    RedsStream *stream, int migration, int num_common_caps,
+    uint32_t *common_caps, int num_caps, uint32_t *caps)
+{
+    RedChannelClient *rcc;
+    SpiceVmcState *state;
+    SpiceCharDeviceInstance *sin;
+    SpiceCharDeviceInterface *sif;
+
+    state = SPICE_CONTAINEROF(channel, SpiceVmcState, channel);
+    sin = state->chardev_sin;
+    sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
+
+    if (state->rcc) {
+        WARN("channel client %d:%d (%p) already connected, refusing second connection\n",
+             channel->type, channel->id, state->rcc);
+        // TODO: notify client in advance about the in use channel using
+        // SPICE_MSG_MAIN_CHANNEL_IN_USE (for example)
+        reds_stream_free(stream);
+        return;
+    }
+
+    rcc = red_channel_client_create(sizeof(RedChannelClient), channel, client, stream);
+    if (!rcc) {
+        return;
+    }
+    state->rcc = rcc;
+    red_channel_client_ack_zero_messages_window(rcc);
+
+    if (sif->state) {
+        sif->state(sin, 1);
+    }
+}
+
+static void spicevmc_migrate(RedChannelClient *rcc)
+{
+    /* NOOP */
+}
+
+void spicevmc_device_connect(SpiceCharDeviceInstance *sin,
+    uint8_t channel_type)
+{
+    static uint8_t id[256] = { 0, };
+    SpiceVmcState *state;
+    ChannelCbs channel_cbs = {0,};
+    ClientCbs client_cbs = {0,};
+
+    channel_cbs.config_socket = spicevmc_red_channel_client_config_socket;
+    channel_cbs.on_disconnect = spicevmc_red_channel_client_on_disconnect;
+    channel_cbs.send_item = spicevmc_red_channel_send_item;
+    channel_cbs.hold_item = spicevmc_red_channel_hold_pipe_item;
+    channel_cbs.release_item = spicevmc_red_channel_release_pipe_item;
+    channel_cbs.alloc_recv_buf = spicevmc_red_channel_alloc_msg_rcv_buf;
+    channel_cbs.release_recv_buf = spicevmc_red_channel_release_msg_rcv_buf;
+
+    state = (SpiceVmcState*)red_channel_create(sizeof(SpiceVmcState),
+                                   core, channel_type, id[channel_type]++,
+                                   FALSE /* migration - TODO? */,
+                                   FALSE /* handle_acks */,
+                                   spicevmc_red_channel_client_handle_message,
+                                   &channel_cbs);
+    red_channel_init_outgoing_messages_window(&state->channel);
+    state->chardev_st.wakeup = spicevmc_chardev_wakeup;
+    state->chardev_sin = sin;
+    state->rcv_buf = spice_malloc(BUF_SIZE);
+    state->rcv_buf_size = BUF_SIZE;
+
+    client_cbs.connect = spicevmc_connect;
+    client_cbs.migrate = spicevmc_migrate;
+    red_channel_register_client_cbs(&state->channel, &client_cbs);
+
+    sin->st = &state->chardev_st;
+
+    reds_register_channel(&state->channel);
+}
+
+/* Must be called from RedClient handling thread. */
+void spicevmc_device_disconnect(SpiceCharDeviceInstance *sin)
+{
+    SpiceVmcState *state;
+
+    state = SPICE_CONTAINEROF(sin->st, SpiceVmcState, chardev_st);
+
+    reds_unregister_channel(&state->channel);
+
+    free(state->pipe_item);
+    free(state->rcv_buf);
+    red_channel_destroy(&state->channel);
+}
diff --git a/server/usbredir.c b/server/usbredir.c
deleted file mode 100644
index 7860389..0000000
--- a/server/usbredir.c
+++ /dev/null
@@ -1,276 +0,0 @@
-/* spice-server usbredir code
-
-   Copyright (C) 2011 Red Hat, Inc.
-
-   Red Hat Authors:
-   Hans de Goede <hdegoede at redhat.com>
-
-   This library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   This library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with this library; if not, see <http://www.gnu.org/licenses/>.
-*/
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <assert.h>
-
-#include "server/char_device.h"
-#include "server/red_channel.h"
-#include "server/reds.h"
-
-/* 64K should be enough for all but the largest bulk xfers + 32 bytes hdr */
-#define BUF_SIZE (64 * 1024 + 32)
-
-typedef struct UsbRedirPipeItem {
-    PipeItem base;
-    /* packets which don't fit this will get split, this is not a problem */
-    uint8_t buf[BUF_SIZE];
-    uint32_t buf_used;
-} UsbRedirPipeItem;
-
-typedef struct UsbRedirState {
-    RedChannel channel; /* Must be the first item */
-    RedChannelClient *rcc;
-    SpiceCharDeviceState chardev_st;
-    SpiceCharDeviceInstance *chardev_sin;
-    UsbRedirPipeItem *pipe_item;
-    uint8_t *rcv_buf;
-    uint32_t rcv_buf_size;
-    int rcv_buf_in_use;
-} UsbRedirState;
-
-static void usbredir_chardev_wakeup(SpiceCharDeviceInstance *sin)
-{
-    UsbRedirState *state;
-    SpiceCharDeviceInterface *sif;
-    int n;
-
-    state = SPICE_CONTAINEROF(sin->st, UsbRedirState, chardev_st);
-    sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
-
-    if (!state->rcc) {
-        return;
-    }
-
-    do {
-        if (!state->pipe_item) {
-            state->pipe_item = spice_malloc(sizeof(UsbRedirPipeItem));
-            red_channel_pipe_item_init(&state->channel,
-                                       &state->pipe_item->base, 0);
-        }
-
-        n = sif->read(sin, state->pipe_item->buf,
-                      sizeof(state->pipe_item->buf));
-        if (n > 0) {
-            state->pipe_item->buf_used = n;
-            red_channel_client_pipe_add_push(state->rcc,
-                                             &state->pipe_item->base);
-            state->pipe_item = NULL;
-        }
-    } while (n > 0);
-}
-
-static int usbredir_red_channel_client_config_socket(RedChannelClient *rcc)
-{
-    return TRUE;
-}
-
-static void usbredir_red_channel_client_on_disconnect(RedChannelClient *rcc)
-{
-    UsbRedirState *state;
-    SpiceCharDeviceInstance *sin;
-    SpiceCharDeviceInterface *sif;
-
-    if (!rcc) {
-        return;
-    }
-
-    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirState, channel);
-    sin = state->chardev_sin;
-    sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
-
-    red_channel_client_destroy(rcc);
-    state->rcc = NULL;
-    if (sif->state) {
-        sif->state(sin, 0);
-    }
-}
-
-static int usbredir_red_channel_client_handle_message(RedChannelClient *rcc,
-    SpiceDataHeader *header, uint8_t *msg)
-{
-    UsbRedirState *state;
-    SpiceCharDeviceInstance *sin;
-    SpiceCharDeviceInterface *sif;
-
-    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirState, channel);
-    sin = state->chardev_sin;
-    sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
-
-    if (header->type != SPICE_MSGC_USBREDIR_DATA) {
-        return red_channel_client_handle_message(rcc, header->size,
-                                                 header->type, msg);
-    }
-
-    /*
-     * qemu usbredir will consume everything we give it, no need for
-     * flow control checks (or to use a pipe).
-     */
-    sif->write(sin, msg, header->size);
-
-    return TRUE;
-}
-
-static uint8_t *usbredir_red_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
-    SpiceDataHeader *msg_header)
-{
-    UsbRedirState *state;
-
-    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirState, channel);
-
-    assert(!state->rcv_buf_in_use);
-
-    if (msg_header->size > state->rcv_buf_size) {
-        state->rcv_buf = spice_realloc(state->rcv_buf, msg_header->size);
-        state->rcv_buf_size = msg_header->size;
-    }
-
-    state->rcv_buf_in_use = 1;
-
-    return state->rcv_buf;
-}
-
-static void usbredir_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
-    SpiceDataHeader *msg_header, uint8_t *msg)
-{
-    UsbRedirState *state;
-
-    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirState, channel);
-
-    /* NOOP, we re-use the buffer every time and only free it on destruction */
-    state->rcv_buf_in_use = 0;
-}
-
-static void usbredir_red_channel_hold_pipe_item(RedChannelClient *rcc,
-    PipeItem *item)
-{
-    /* NOOP */
-}
-
-static void usbredir_red_channel_send_item(RedChannelClient *rcc,
-    PipeItem *item)
-{
-    UsbRedirPipeItem *i = SPICE_CONTAINEROF(item, UsbRedirPipeItem, base);
-    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
-
-    red_channel_client_init_send_data(rcc, SPICE_MSG_USBREDIR_DATA, item);
-    spice_marshaller_add_ref(m, i->buf, i->buf_used);
-    red_channel_client_begin_send_message(rcc);
-}
-
-static void usbredir_red_channel_release_pipe_item(RedChannelClient *rcc,
-    PipeItem *item, int item_pushed)
-{
-    free(item);
-}
-
-static void usbredir_connect(RedChannel *channel, RedClient *client,
-    RedsStream *stream, int migration, int num_common_caps,
-    uint32_t *common_caps, int num_caps, uint32_t *caps)
-{
-    RedChannelClient *rcc;
-    UsbRedirState *state;
-    SpiceCharDeviceInstance *sin;
-    SpiceCharDeviceInterface *sif;
-
-    state = SPICE_CONTAINEROF(channel, UsbRedirState, channel);
-    sin = state->chardev_sin;
-    sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
-
-    if (state->rcc) {
-        WARN("channel client %d:%d (%p) already connected, refusing second connection\n",
-             channel->type, channel->id, state->rcc);
-        // TODO: notify client in advance about the in use channel using
-        // SPICE_MSG_MAIN_CHANNEL_IN_USE (for example)
-        reds_stream_free(stream);
-        return;
-    }
-
-    rcc = red_channel_client_create(sizeof(RedChannelClient), channel, client, stream);
-    if (!rcc) {
-        return;
-    }
-    state->rcc = rcc;
-    red_channel_client_ack_zero_messages_window(rcc);
-
-    if (sif->state) {
-        sif->state(sin, 1);
-    }
-}
-
-static void usbredir_migrate(RedChannelClient *rcc)
-{
-    /* NOOP */
-}
-
-int usbredir_device_connect(SpiceCharDeviceInstance *sin)
-{
-    static int id = 0;
-    UsbRedirState *state;
-    ChannelCbs channel_cbs = {0,};
-    ClientCbs client_cbs = {0,};
-
-    channel_cbs.config_socket = usbredir_red_channel_client_config_socket;
-    channel_cbs.on_disconnect = usbredir_red_channel_client_on_disconnect;
-    channel_cbs.send_item = usbredir_red_channel_send_item;
-    channel_cbs.hold_item = usbredir_red_channel_hold_pipe_item;
-    channel_cbs.release_item = usbredir_red_channel_release_pipe_item;
-    channel_cbs.alloc_recv_buf = usbredir_red_channel_alloc_msg_rcv_buf;
-    channel_cbs.release_recv_buf = usbredir_red_channel_release_msg_rcv_buf;
-
-    state = (UsbRedirState*)red_channel_create(sizeof(UsbRedirState),
-                                   core, SPICE_CHANNEL_USBREDIR, id++,
-                                   FALSE /* migration - TODO? */,
-                                   FALSE /* handle_acks */,
-                                   usbredir_red_channel_client_handle_message,
-                                   &channel_cbs);
-    red_channel_init_outgoing_messages_window(&state->channel);
-    state->chardev_st.wakeup = usbredir_chardev_wakeup;
-    state->chardev_sin = sin;
-    state->rcv_buf = spice_malloc(BUF_SIZE);
-    state->rcv_buf_size = BUF_SIZE;
-
-    client_cbs.connect = usbredir_connect;
-    client_cbs.migrate = usbredir_migrate;
-    red_channel_register_client_cbs(&state->channel, &client_cbs);
-
-    sin->st = &state->chardev_st;
-
-    reds_register_channel(&state->channel);
-
-    return 0;
-}
-
-/* Must be called from RedClient handling thread. */
-void usbredir_device_disconnect(SpiceCharDeviceInstance *sin)
-{
-    UsbRedirState *state;
-
-    state = SPICE_CONTAINEROF(sin->st, UsbRedirState, chardev_st);
-
-    reds_unregister_channel(&state->channel);
-
-    free(state->pipe_item);
-    free(state->rcv_buf);
-    red_channel_destroy(&state->channel);
-}
commit 073cf20ac57b9e035f247f180a109f29333dacb8
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Aug 25 10:41:51 2011 +0200

    usbredir: Merge UsbRedirState and UsbRedirChannel
    
    Now that the Channel struct is gone and the RedChannel has the same lifetime
    as the chardev interface there is no need to have these 2 separate.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/server/usbredir.c b/server/usbredir.c
index eb9b91f..7860389 100644
--- a/server/usbredir.c
+++ b/server/usbredir.c
@@ -39,7 +39,7 @@ typedef struct UsbRedirPipeItem {
 } UsbRedirPipeItem;
 
 typedef struct UsbRedirState {
-    RedChannel *channel;
+    RedChannel channel; /* Must be the first item */
     RedChannelClient *rcc;
     SpiceCharDeviceState chardev_st;
     SpiceCharDeviceInstance *chardev_sin;
@@ -49,11 +49,6 @@ typedef struct UsbRedirState {
     int rcv_buf_in_use;
 } UsbRedirState;
 
-typedef struct UsbRedirChannel {
-    RedChannel base;
-    UsbRedirState *state;
-} UsbRedirChannel;
-
 static void usbredir_chardev_wakeup(SpiceCharDeviceInstance *sin)
 {
     UsbRedirState *state;
@@ -70,7 +65,7 @@ static void usbredir_chardev_wakeup(SpiceCharDeviceInstance *sin)
     do {
         if (!state->pipe_item) {
             state->pipe_item = spice_malloc(sizeof(UsbRedirPipeItem));
-            red_channel_pipe_item_init(state->rcc->channel,
+            red_channel_pipe_item_init(&state->channel,
                                        &state->pipe_item->base, 0);
         }
 
@@ -100,7 +95,7 @@ static void usbredir_red_channel_client_on_disconnect(RedChannelClient *rcc)
         return;
     }
 
-    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirChannel, base)->state;
+    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirState, channel);
     sin = state->chardev_sin;
     sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
 
@@ -118,7 +113,7 @@ static int usbredir_red_channel_client_handle_message(RedChannelClient *rcc,
     SpiceCharDeviceInstance *sin;
     SpiceCharDeviceInterface *sif;
 
-    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirChannel, base)->state;
+    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirState, channel);
     sin = state->chardev_sin;
     sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
 
@@ -141,7 +136,7 @@ static uint8_t *usbredir_red_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
 {
     UsbRedirState *state;
 
-    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirChannel, base)->state;
+    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirState, channel);
 
     assert(!state->rcv_buf_in_use);
 
@@ -160,7 +155,7 @@ static void usbredir_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
 {
     UsbRedirState *state;
 
-    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirChannel, base)->state;
+    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirState, channel);
 
     /* NOOP, we re-use the buffer every time and only free it on destruction */
     state->rcv_buf_in_use = 0;
@@ -195,12 +190,10 @@ static void usbredir_connect(RedChannel *channel, RedClient *client,
 {
     RedChannelClient *rcc;
     UsbRedirState *state;
-    UsbRedirChannel *redir_chan;
     SpiceCharDeviceInstance *sin;
     SpiceCharDeviceInterface *sif;
 
-    redir_chan = SPICE_CONTAINEROF(channel, UsbRedirChannel, base);
-    state = redir_chan->state;
+    state = SPICE_CONTAINEROF(channel, UsbRedirState, channel);
     sin = state->chardev_sin;
     sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
 
@@ -234,7 +227,6 @@ int usbredir_device_connect(SpiceCharDeviceInstance *sin)
 {
     static int id = 0;
     UsbRedirState *state;
-    UsbRedirChannel *redir_chan;
     ChannelCbs channel_cbs = {0,};
     ClientCbs client_cbs = {0,};
 
@@ -246,28 +238,25 @@ int usbredir_device_connect(SpiceCharDeviceInstance *sin)
     channel_cbs.alloc_recv_buf = usbredir_red_channel_alloc_msg_rcv_buf;
     channel_cbs.release_recv_buf = usbredir_red_channel_release_msg_rcv_buf;
 
-    redir_chan = (UsbRedirChannel*)red_channel_create(sizeof(UsbRedirChannel),
-                                             core, SPICE_CHANNEL_USBREDIR, id++,
-                                             FALSE /* migration - TODO?*/,
-                                             FALSE /* handle_acks */,
-                                             usbredir_red_channel_client_handle_message,
-                                             &channel_cbs);
-    red_channel_init_outgoing_messages_window(&redir_chan->base);
-    state = spice_new0(UsbRedirState, 1);
-    state->channel = &redir_chan->base;
+    state = (UsbRedirState*)red_channel_create(sizeof(UsbRedirState),
+                                   core, SPICE_CHANNEL_USBREDIR, id++,
+                                   FALSE /* migration - TODO? */,
+                                   FALSE /* handle_acks */,
+                                   usbredir_red_channel_client_handle_message,
+                                   &channel_cbs);
+    red_channel_init_outgoing_messages_window(&state->channel);
     state->chardev_st.wakeup = usbredir_chardev_wakeup;
     state->chardev_sin = sin;
     state->rcv_buf = spice_malloc(BUF_SIZE);
     state->rcv_buf_size = BUF_SIZE;
-    redir_chan->state = state;
 
     client_cbs.connect = usbredir_connect;
     client_cbs.migrate = usbredir_migrate;
-    red_channel_register_client_cbs(&redir_chan->base, &client_cbs);
+    red_channel_register_client_cbs(&state->channel, &client_cbs);
 
     sin->st = &state->chardev_st;
 
-    reds_register_channel(&redir_chan->base);
+    reds_register_channel(&state->channel);
 
     return 0;
 }
@@ -279,13 +268,9 @@ void usbredir_device_disconnect(SpiceCharDeviceInstance *sin)
 
     state = SPICE_CONTAINEROF(sin->st, UsbRedirState, chardev_st);
 
-    reds_unregister_channel(state->channel);
-
-    red_channel_client_destroy(state->rcc);
-    red_channel_disconnect(state->channel);
+    reds_unregister_channel(&state->channel);
 
     free(state->pipe_item);
     free(state->rcv_buf);
-    free(state->channel);
-    free(state);
+    red_channel_destroy(&state->channel);
 }
commit 22a6eef60dbeb49b07461107c55fb0cbc7ae73c0
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Wed Aug 24 16:34:45 2011 +0200

    usbredir: Fix crash caused by MC changes
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/server/usbredir.c b/server/usbredir.c
index 11c4058..eb9b91f 100644
--- a/server/usbredir.c
+++ b/server/usbredir.c
@@ -259,6 +259,7 @@ int usbredir_device_connect(SpiceCharDeviceInstance *sin)
     state->chardev_sin = sin;
     state->rcv_buf = spice_malloc(BUF_SIZE);
     state->rcv_buf_size = BUF_SIZE;
+    redir_chan->state = state;
 
     client_cbs.connect = usbredir_connect;
     client_cbs.migrate = usbredir_migrate;
commit e1570ce519711be803ee5e8736187d8f7da9789c
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Wed Aug 24 16:25:38 2011 +0200

    usbredir: Ensure that our msg_rcv_buf is not used re-entrantly
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/server/usbredir.c b/server/usbredir.c
index 8daab7d..11c4058 100644
--- a/server/usbredir.c
+++ b/server/usbredir.c
@@ -22,6 +22,8 @@
 #include <config.h>
 #endif
 
+#include <assert.h>
+
 #include "server/char_device.h"
 #include "server/red_channel.h"
 #include "server/reds.h"
@@ -44,6 +46,7 @@ typedef struct UsbRedirState {
     UsbRedirPipeItem *pipe_item;
     uint8_t *rcv_buf;
     uint32_t rcv_buf_size;
+    int rcv_buf_in_use;
 } UsbRedirState;
 
 typedef struct UsbRedirChannel {
@@ -140,18 +143,27 @@ static uint8_t *usbredir_red_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
 
     state = SPICE_CONTAINEROF(rcc->channel, UsbRedirChannel, base)->state;
 
+    assert(!state->rcv_buf_in_use);
+
     if (msg_header->size > state->rcv_buf_size) {
         state->rcv_buf = spice_realloc(state->rcv_buf, msg_header->size);
         state->rcv_buf_size = msg_header->size;
     }
 
+    state->rcv_buf_in_use = 1;
+
     return state->rcv_buf;
 }
 
 static void usbredir_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
     SpiceDataHeader *msg_header, uint8_t *msg)
 {
+    UsbRedirState *state;
+
+    state = SPICE_CONTAINEROF(rcc->channel, UsbRedirChannel, base)->state;
+
     /* NOOP, we re-use the buffer every time and only free it on destruction */
+    state->rcv_buf_in_use = 0;
 }
 
 static void usbredir_red_channel_hold_pipe_item(RedChannelClient *rcc,
commit e9d6e86fc806b1f43a8785e00527add211cfb786
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Wed Aug 24 16:14:46 2011 +0200

    red_channel: Fix msg buf memleak on parser error
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/server/red_channel.c b/server/red_channel.c
index 87320b4..8993cc3 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -130,6 +130,7 @@ static void red_peer_handle_incoming(RedsStream *stream, IncomingHandler *handle
                 SPICE_VERSION_MINOR, &parsed_size, &parsed_free);
             if (parsed == NULL) {
                 red_printf("failed to parse message type %d", handler->header.type);
+                handler->cb->release_msg_buf(handler->opaque, &handler->header, handler->msg);
                 handler->cb->on_error(handler->opaque);
                 return;
             }


More information about the Spice-commits mailing list