On Fri, Oct 19, 2012 at 01:50:11PM +0200, Alon Levy wrote: > New API: spice_server_set_ws_ports > > This adds an optional dependency on libwebsockets. You need to get my > patched 0.0.3 version here: > git://people.freedesktop.org/~alon/libwebsockets > > There is no qemu patches yet, to test change in reds.c the default value > of spice_ws_port to 5959 (for the default of spice-html5). > > For testing there is an online client at > http://spice-space.org/spice-html5/spice.html > > Known issues: > 1. The tester (server/tests/test_display_no_ssl) gets into dropping all > data after a few seconds, I think it's an issue with the implemented > watches, but haven't figured it out. > > 2. libwebsocket's read interface is inverted to what our code expects, > i.e. there is no libwebsocket_read, so there is an additional copy > involved (see RedsWebSocket). This can be fixed. > > 3. Listening on a separate port. Since the headers are different, we > could listen on the same port (first three bytes RED/GET). I don't know > if we want to? > > Todos: > 1. SSL not implemented yet. Needs some thought as to how. > > 2. Serve spice-html5 when accessed as a http server. Nice to have. > --- > configure.ac | 15 ++ > server/Makefile.am | 4 + > server/reds-private.h | 47 +++++- > server/reds.c | 79 ++++++---- > server/reds.h | 17 +++ > server/reds_websockets.c | 311 +++++++++++++++++++++++++++++++++++++++ > server/reds_websockets.h | 9 ++ > server/spice-server.syms | 5 + > server/spice.h | 7 + > server/tests/test_display_base.c | 4 +- > 10 files changed, 466 insertions(+), 32 deletions(-) > create mode 100644 server/reds_websockets.c > create mode 100644 server/reds_websockets.h > > diff --git a/configure.ac b/configure.ac > index dff930d..5561d2c 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -160,6 +160,13 @@ AC_ARG_ENABLE(automated_tests, > AS_IF([test x"$enable_automated_tests" != "xno"], [enable_automated_tests="yes"]) > AM_CONDITIONAL(SUPPORT_AUTOMATED_TESTS, test "x$enable_automated_tests" != "xno") > > +AC_ARG_ENABLE(libwebsockets, > +[ --enable-libwebsockets Enable websockets server support (no need for proxy)],, > +[enable_libwebsockets="yes"]) Would be better if this defaulted to "check" and used libwebsockets if available, and don't use it if it's not > +AS_IF([test x"enable_libwebsockets" != "xyes"], [enable_websockets="no"]) > +if test "x$enable_libwebsockets" = "xyes"; then > + AC_DEFINE([USE_LIBWEBSOCKETS], [1], [Define if supporting websocket connections]) > +fi > > dnl ========================================================================= > dnl Check deps > @@ -237,6 +244,12 @@ if test "x$enable_smartcard" = "xyes"; then > SPICE_REQUIRES+=" libcacard >= 0.1.2" > fi > > +if test "x$enable_libwebsockets" = "xyes"; then > + PKG_CHECK_MODULES(LIBWEBSOCKETS, libwebsockets >= 0.0.3) > + AC_SUBST(LIBWEBSOCKETS_LIBS) > + AC_SUBST(LIBWEBSOCKETS_CFLAGS) > + SPICE_REQUIRES+=" libwebsockets >= 0.0.3" > +fi > > PKG_CHECK_MODULES(PIXMAN, pixman-1 >= 0.17.7) > AC_SUBST(PIXMAN_CFLAGS) > @@ -536,6 +549,8 @@ echo " > SASL support: ${enable_sasl} > > Automated tests: ${enable_automated_tests} > + > + libwebsockets: ${enable_libwebsockets} > " > > if test $os_win32 == "yes" ; then > diff --git a/server/Makefile.am b/server/Makefile.am > index b62d98c..a7d56d4 100644 > --- a/server/Makefile.am > +++ b/server/Makefile.am > @@ -10,6 +10,7 @@ AM_CPPFLAGS = \ > $(SLIRP_CFLAGS) \ > $(SMARTCARD_CFLAGS) \ > $(SSL_CFLAGS) \ > + $(LIBWEBSOCKETS_CFLAGS) \ > $(VISIBILITY_HIDDEN_CFLAGS) \ > $(WARN_CFLAGS) \ > $(NULL) > @@ -38,6 +39,7 @@ libspice_server_la_LIBADD = \ > $(SLIRP_LIBS) \ > $(SSL_LIBS) \ > $(Z_LIBS) \ > + $(LIBWEBSOCKETS_LIBS) \ > $(NULL) > > libspice_server_la_SOURCES = \ > @@ -91,6 +93,8 @@ libspice_server_la_SOURCES = \ > spicevmc.c \ > zlib_encoder.c \ > zlib_encoder.h \ > + reds_websockets.c \ > + reds_websockets.h \ Compilation of this should depend on websockets being available or not > $(NULL) > > if SUPPORT_TUNNEL > diff --git a/server/reds-private.h b/server/reds-private.h > index 3db6565..a5903b3 100644 > --- a/server/reds-private.h > +++ b/server/reds-private.h > @@ -4,6 +4,16 @@ > #include <time.h> > > #include <spice/protocol.h> > +#include <spice/stats.h> > + > +#if USE_LIBWEBSOCKETS > +#include <libwebsockets.h> > +#endif > + > +#include "reds.h" > +#include "char_device.h" > +#include "agent-msg-filter.h" > +#include "main_channel.h" > > #define MIGRATE_TIMEOUT (1000 * 10) /* 10sec */ > #define MM_TIMER_GRANULARITY_MS (1000 / 30) > @@ -34,10 +44,6 @@ typedef struct VDIReadBuf { > uint8_t data[SPICE_AGENT_MAX_DATA_SIZE]; > } VDIReadBuf; > > -static VDIReadBuf *vdi_port_read_buf_get(void); > -static VDIReadBuf *vdi_port_read_buf_ref(VDIReadBuf *buf); > -static void vdi_port_read_buf_unref(VDIReadBuf *buf); > - > enum { > VDI_PORT_READ_STATE_READ_HEADER, > VDI_PORT_READ_STATE_GET_BUFF, > @@ -125,9 +131,19 @@ typedef struct RedsClientMonitorsConfig { > int buffer_pos; > } RedsClientMonitorsConfig; > > +#ifdef USE_LIBWEBSOCKETS > +#define REDS_MAX_WEBSOCKETS 32 > +#endif > + > typedef struct RedsState { > int listen_socket; > int secure_listen_socket; > +#ifdef USE_LIBWEBSOCKETS > + struct libwebsocket_context *ws_context; > + RedsWebSocket ws[REDS_MAX_WEBSOCKETS]; > + int ws_in_service_fd; > + int ws_count; > +#endif > SpiceWatch *listen_watch; > SpiceWatch *secure_listen_watch; > VDIPortState agent_state; > @@ -179,4 +195,27 @@ typedef struct RedsState { > RedsClientMonitorsConfig client_monitors_config; > } RedsState; > > +typedef struct AsyncRead { > + RedsStream *stream; > + void *opaque; > + uint8_t *now; > + uint8_t *end; > + void (*done)(void *opaque); > + void (*error)(void *opaque, int err); > +} AsyncRead; > + > +typedef struct RedLinkInfo { > + RedsStream *stream; > + AsyncRead asyc_read; > + SpiceLinkHeader link_header; > + SpiceLinkMess *link_mess; > + int mess_pos; > + TicketInfo tiTicketing; > + SpiceLinkAuthMechanism auth_mechanism; > + int skip_auth; > +} RedLinkInfo; > + > +RedLinkInfo *spice_server_add_client_create_link(SpiceServer *s, int socket, int skip_auth); > +void reds_handle_new_link(RedLinkInfo *link); > + > #endif > diff --git a/server/reds.c b/server/reds.c > index 98c8706..bd16764 100644 > --- a/server/reds.c > +++ b/server/reds.c > @@ -74,9 +74,16 @@ > #ifdef USE_SMARTCARD > #include "smartcard.h" > #endif > +#if USE_LIBWEBSOCKETS > +#include "reds_websockets.h" > +#endif > > #include "reds-private.h" > > +static VDIReadBuf *vdi_port_read_buf_get(void); > +static VDIReadBuf *vdi_port_read_buf_ref(VDIReadBuf *buf); > +static void vdi_port_read_buf_unref(VDIReadBuf *buf); > + > SpiceCoreInterface *core = NULL; > static SpiceCharDeviceInstance *vdagent = NULL; > static SpiceMigrateInstance *migration_interface = NULL; > @@ -99,6 +106,10 @@ static TicketAuthentication taTicket; > > static int spice_port = -1; > static int spice_secure_port = -1; > +#if USE_LIBWEBSOCKETS > +static int spice_ws_port = -1; > +static int spice_wss_port = -1; > +#endif > static int spice_listen_socket_fd = -1; > static char spice_addr[256]; > static int spice_family = PF_UNSPEC; > @@ -127,26 +138,6 @@ static bool exit_on_disconnect = FALSE; > > static RedsState *reds = NULL; > > -typedef struct AsyncRead { > - RedsStream *stream; > - void *opaque; > - uint8_t *now; > - uint8_t *end; > - void (*done)(void *opaque); > - void (*error)(void *opaque, int err); > -} AsyncRead; > - > -typedef struct RedLinkInfo { > - RedsStream *stream; > - AsyncRead asyc_read; > - SpiceLinkHeader link_header; > - SpiceLinkMess *link_mess; > - int mess_pos; > - TicketInfo tiTicketing; > - SpiceLinkAuthMechanism auth_mechanism; > - int skip_auth; > -} RedLinkInfo; > - > typedef struct RedSSLParameters { > char keyfile_password[256]; > char certs_file[256]; > @@ -2718,7 +2709,7 @@ static void reds_handle_read_header_done(void *opaque) > async_read_handler(0, 0, &link->asyc_read); > } > > -static void reds_handle_new_link(RedLinkInfo *link) > +void reds_handle_new_link(RedLinkInfo *link) > { > AsyncRead *obj = &link->asyc_read; > obj->opaque = link; > @@ -2882,7 +2873,6 @@ static void reds_accept_ssl_connection(int fd, int event, void *data) > } > } > > - > static void reds_accept(int fd, int event, void *data) > { > int socket; > @@ -2896,25 +2886,33 @@ static void reds_accept(int fd, int event, void *data) > close(socket); > } > > - > -SPICE_GNUC_VISIBLE int spice_server_add_client(SpiceServer *s, int socket, int skip_auth) > +RedLinkInfo *spice_server_add_client_create_link(SpiceServer *s, int socket, int skip_auth) > { > RedLinkInfo *link; > - RedsStream *stream; > > spice_assert(reds == s); > if (!(link = reds_init_client_connection(socket))) { > spice_warning("accept failed"); > - return -1; > + return NULL; > } > > link->skip_auth = skip_auth; > + return link; > +} > + > +SPICE_GNUC_VISIBLE int spice_server_add_client(SpiceServer *s, int socket, int skip_auth) > +{ > + RedLinkInfo *link; > + RedsStream *stream; > > + link = spice_server_add_client_create_link(s, socket, skip_auth); > + if (!link) { > + return -1; > + } > stream = link->stream; > stream->read = stream_read_cb; > stream->write = stream_write_cb; > stream->writev = stream_writev_cb; > - > reds_handle_new_link(link); > return 0; > } > @@ -3033,6 +3031,12 @@ static int reds_init_net(void) > return -1; > } > } > + > +#if USE_LIBWEBSOCKETS > + if (spice_ws_port != -1 || spice_wss_port != -1) { > + reds_init_websocket(reds, spice_addr, spice_ws_port, spice_wss_port); > + } > +#endif > return 0; > } > > @@ -3949,6 +3953,27 @@ SPICE_GNUC_VISIBLE int spice_server_set_port(SpiceServer *s, int port) > return 0; > } > > + > +SPICE_GNUC_VISIBLE int spice_server_set_ws_ports(SpiceServer *s, int ws_port, int wss_port) > +{ > +#if USE_LIBWEBSOCKETS > + spice_assert(reds == s); > + if ((ws_port < 1 || ws_port > 0xffff) && (wss_port < 1 || wss_port > 0xffff)) { > + return -1; > + } > + if (ws_port > 0 && ws_port < 0xffff) { > + spice_ws_port = ws_port; > + } > + if (wss_port > 0 && wss_port < 0xffff) { > + spice_wss_port = wss_port; > + } > + return 0; > +#else > + fprintf(stderr, "libwebsockets is unsupported in this spice build\n"); > + return -1; > +#endif > +} > + > SPICE_GNUC_VISIBLE void spice_server_set_addr(SpiceServer *s, const char *addr, int flags) > { > spice_assert(reds == s); > diff --git a/server/reds.h b/server/reds.h > index f8e8d56..0d4f933 100644 > --- a/server/reds.h > +++ b/server/reds.h > @@ -65,6 +65,20 @@ typedef struct RedsSASL { > } RedsSASL; > #endif > > +#ifdef USE_LIBWEBSOCKETS > +typedef struct RedsWebSocket { > + struct libwebsocket_context *context; > + struct libwebsocket *wsi; > + SpiceWatch *watch; > + int fd; > + unsigned events; > + /* buffer of available data to read, always starts at offset 0 to data_avail - 1. */ > + unsigned char *data; > + unsigned data_len; > + unsigned data_avail; > +} RedsWebSocket; > +#endif > + > struct RedsStream { > int socket; > SpiceWatch *watch; > @@ -73,6 +87,9 @@ struct RedsStream { > receive may return data afterward. check the flag before calling receive*/ > int shutdown; > SSL *ssl; > +#ifdef USE_LIBWEBSOCKETS > + RedsWebSocket *ws; > +#endif > > #if HAVE_SASL > RedsSASL sasl; > diff --git a/server/reds_websockets.c b/server/reds_websockets.c > new file mode 100644 > index 0000000..24df2e5 > --- /dev/null > +++ b/server/reds_websockets.c > @@ -0,0 +1,311 @@ > +#include "config.h" > + > +#ifdef USE_LIBWEBSOCKETS > +#include <unistd.h> > +#include <errno.h> > +#include <libwebsockets.h> > + > +#include "spice.h" > +#include "reds.h" > +#include "reds-private.h" > +#include "reds_websockets.h" > + > +static ssize_t stream_write_ws_cb(RedsStream *s, const void *buf, size_t size) > +{ > + /* TODO: better way to handle the requirement of libwebsocket, perhaps > + * we should make a writev version for libwebsocket. Assuming writev doesn't > + * cause a linearlizing copy itself. */ Yep, writev would be better > + ssize_t ret; > + unsigned char *padded_buf = spice_malloc(size + LWS_SEND_BUFFER_PRE_PADDING + > + LWS_SEND_BUFFER_POST_PADDING); > + spice_assert(s && s->ws); > + memcpy(padded_buf + LWS_SEND_BUFFER_PRE_PADDING, buf, size); > + ret = libwebsocket_write(s->ws->wsi, &padded_buf[LWS_SEND_BUFFER_PRE_PADDING], size, > + LWS_WRITE_BINARY); > + free(padded_buf); > + return ret == 0 ? size : -1; /* XXX exact bytes required? if not this is > + good enough, else need to change > + libwebsocket */ > +} > + > +static void reds_websocket_append_data(RedsWebSocket *ws, unsigned char *buf, > + size_t size) > +{ > + if (!ws->data) { > + ws->data = spice_malloc(size); > + ws->data_len = size; > + ws->data_avail = 0; > + } > + if (ws->data_len < size + ws->data_avail) { > + ws->data_len = size + ws->data_avail; > + ws->data = realloc(ws->data, ws->data_len); spice_realloc, or you need to handle realloc failures. > + } > + memcpy(ws->data + ws->data_avail, buf, size); > + ws->data_avail += size; > +} > + > +static ssize_t reds_websocket_read_data(RedsWebSocket *ws, unsigned char *buf, > + size_t size) > +{ > + ssize_t ret; > + > + ret = ws->data_avail > size ? size : ws->data_avail; > + if (ret > 0) { > + memcpy(buf, ws->data, ret); > + } > + if (ret > 0 && ret < ws->data_avail) { > + memmove(ws->data, ws->data + ret, ws->data_avail - ret); > + } > + ws->data_avail -= ret; > + if (ws->data_avail == 0 && ret == size) { > + ws->data = NULL; > + ws->data_len = ws->data_avail = 0; Isn't this leaking ws->data? > + } > + return ret; > +} > + > +static int reds_libwebsocket_service_fd(RedsState *s, struct pollfd *pfd) > +{ > + int ret; > + if (s->ws_in_service_fd) { > + return 0; > + } > + s->ws_in_service_fd = 1; > + ret = libwebsocket_service_fd(s->ws_context, pfd); > + s->ws_in_service_fd = 0; Why do we need this ws_in_service_fd variable? > + if (ret != 0) { > + if (errno == EAGAIN) { > + spice_debug("libwebsocket_servide_fd EAGAIN, pfd->revents = %d", typo: servide Apart from these comments, this looks good (and the code is not invasive anyway). Christophe
Attachment:
pgpP8XgqiYEq2.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel