> 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 I just wanted to avoid regressions for now - let it only be available if explicitly asked for. > > > +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 The files have an ifdef that makes them null, but I can also do it here, probably nicer, so ok. > > > > $(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. > Thanks, will fix. > > + } > > + 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? It is, will fix. > > > + } > > + 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? > To avoid recursively calling libwebsocket_service_fd, which we don't need and I don't want to find out if it is supported the hard way. > > + if (ret != 0) { > > + if (errno == EAGAIN) { > > + spice_debug("libwebsocket_servide_fd EAGAIN, > > pfd->revents = %d", > > typo: servide Thanks, wlil fix. > > Apart from these comments, this looks good (and the code is not > invasive > anyway). > > Christophe > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel