Re: [Spice-devel] [PATCH spice-gtk 1/2] Add webdav channel
On 02/28/2014 02:16 PM, Marc-André Lureau wrote: From: Marc-André Lureau marcandre.lur...@redhat.com See spice-common for protocol details. phodav, a webdav server library, is imported thanks to a submodule, until this project has a stable API and releases. The webdav channel is reponsible for handling port events and multiplexing the request streams. Extra care has been made to avoid blocking and to enable some fairness between concurrent streams, however this has been particularly tricky and is likely to have some issues left. The webdav server is run in a seperate thread, using libsoup. The client communication is done via a local tcp socket, but protected to only accept local connection and with a pretty strong password. The home directory is exported for the remote to browse, which seems to be a sensible default atm. I didn't understand everything (I'm confused by the soup part), but it works, and looks good to me. ACK series --- .gitmodules | 3 + autogen.sh | 2 +- configure.ac | 4 + doc/reference/spice-gtk-docs.xml | 1 + doc/reference/spice-gtk-sections.txt | 17 + doc/reference/spice-gtk.types| 4 +- gtk/Makefile.am | 9 +- gtk/channel-webdav.c | 733 +++ gtk/channel-webdav.h | 68 gtk/map-file | 1 + gtk/phodav | 1 + gtk/spice-channel.c | 6 + gtk/spice-client.h | 1 + gtk/spice-glib-sym-file | 1 + gtk/spice-session-priv.h | 3 + gtk/spice-session.c | 1 + spice-common | 2 +- 17 files changed, 852 insertions(+), 5 deletions(-) create mode 100644 gtk/channel-webdav.c create mode 100644 gtk/channel-webdav.h create mode 16 gtk/phodav diff --git a/.gitmodules b/.gitmodules index 0c618ee..cfce54a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,6 @@ [submodule spice-common] path = spice-common url = ../spice-common +[submodule gtk/phodav] + path = gtk/phodav + url = git://git.gnome.org/phodav diff --git a/autogen.sh b/autogen.sh index 0c18272..d71be70 100755 --- a/autogen.sh +++ b/autogen.sh @@ -6,6 +6,7 @@ srcdir=`dirname $0` test -z $srcdir srcdir=. git submodule update --init --recursive +(cd $srcdir/gtk/phodav/ intltoolize -f) gtkdocize autoreconf -v --force --install @@ -15,4 +16,3 @@ if [ -z $NOCONFIGURE ]; then echo Running configure with --enable-maintainer-mode --enable-gtk-doc --with-gtk=3.0 --enable-vala ${1+$@} $srcdir/configure --enable-maintainer-mode --enable-gtk-doc --with-gtk=3.0 --enable-vala ${1+$@} fi - diff --git a/configure.ac b/configure.ac index 192d748..d30ec40 100644 --- a/configure.ac +++ b/configure.ac @@ -75,6 +75,8 @@ AC_CONFIG_SUBDIRS([spice-common]) COMMON_CFLAGS='-I ${top_srcdir}/spice-common/ -I ${top_srcdir}/spice-common/spice-protocol/' AC_SUBST(COMMON_CFLAGS) +AC_CONFIG_SUBDIRS([gtk/phodav]) + SPICE_GTK_MAJOR_VERSION=`echo $PACKAGE_VERSION | cut -d. -f1` SPICE_GTK_MINOR_VERSION=`echo $PACKAGE_VERSION | cut -d. -f2` SPICE_GTK_MICRO_VERSION=`echo $PACKAGE_VERSION | cut -d. -f3 | cut -d- -f1` @@ -267,6 +269,8 @@ PKG_CHECK_MODULES(GTHREAD, gthread-2.0 2.0.0) AC_SUBST(GTHREAD_CFLAGS) AC_SUBST(GTHREAD_LIBS) +PKG_CHECK_MODULES(SOUP, libsoup-2.4) + AC_ARG_WITH([audio], AS_HELP_STRING([--with-audio=@:@gstreamer/pulse/auto/no@:@], [Select audio backend @:@default=auto@:@]), [], diff --git a/doc/reference/spice-gtk-docs.xml b/doc/reference/spice-gtk-docs.xml index d2c1a2b..5faea74 100644 --- a/doc/reference/spice-gtk-docs.xml +++ b/doc/reference/spice-gtk-docs.xml @@ -37,6 +37,7 @@ xi:include href=xml/channel-smartcard.xml/ xi:include href=xml/channel-usbredir.xml/ xi:include href=xml/channel-port.xml/ + xi:include href=xml/channel-webdav.xml/ /chapter chapter diff --git a/doc/reference/spice-gtk-sections.txt b/doc/reference/spice-gtk-sections.txt index 9232a23..caaa92c 100644 --- a/doc/reference/spice-gtk-sections.txt +++ b/doc/reference/spice-gtk-sections.txt @@ -461,3 +461,20 @@ spice_uri_get_type SUBSECTION Private SpiceURIPrivate /SECTION + +SECTION +FILEchannel-webdav/FILE +TITLESpiceWebdavChannel/TITLE +SpiceWebdavChannel +SpiceWebdavChannelClass +SUBSECTION Standard +SPICE_IS_WEBDAV_CHANNEL +SPICE_IS_WEBDAV_CHANNEL_CLASS +SPICE_TYPE_WEBDAV_CHANNEL +SPICE_WEBDAV_CHANNEL +SPICE_WEBDAV_CHANNEL_CLASS +SPICE_WEBDAV_CHANNEL_GET_CLASS +spice_webdav_channel_get_type +SUBSECTION Private +SpiceWebdavChannelPrivate +/SECTION diff --git a/doc/reference/spice-gtk.types b/doc/reference/spice-gtk.types index 2f52845..db0374a 100644 ---
Re: [Spice-devel] [PATCH spice-common] RFC: spice.proto: add webdav channel
On 03/14/2014 06:17 PM, Marc-André Lureau wrote: If no further comments, can I move ahead adding that new channel? ACK by me. On Fri, Feb 28, 2014 at 4:39 PM, David Jaša dj...@redhat.com wrote: On Čt, 2014-02-27 at 18:27 +0100, Marc-André Lureau wrote: Hi On Wed, Feb 26, 2014 at 11:00 PM, Jonathon Jongsma jjong...@redhat.com wrote: There's also the option of using a 'temporary' name (e.g. DraftWebDAVChannel or something) until we're convinced that it's the right approach and then rename it to a more official name and advertise it as stable... Imho, we should prefer versioning over unstable APIs when possible. The fact that channel id X will be named DraftWebDAVChannel or WebDAVChannel doesn't change much, but perhaps it makes it clear that it could be removed/replaced later. Tbh, this is all hypothetical, the proposed implementation works well with both Linux and Windows remotes (but more exposure would help finding shortcomings..) IMO fresh builds in virt-preview repo could bring some extra exposure... David ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] Don't truncate large 'now' values in _spice_timer_set
On 03/10/2014 12:58 PM, Christophe Fergeau wrote: From: David Gibson dgib...@redhat.com static void _spice_timer_set(SpiceTimer *timer, uint32_t ms, uint32_t now) The _spice_timer_set() function takes a 32-bit integer for the now value. The now value passed in however, can exceed 2^32 (it's in ms and derived from CLOCK_MONOTONIC, which will wrap around a 32-bit integer in around 46 days). If the now value passed in exceeds 2^32, this will mean timers are inserted into the active list with expiry values before the current time, they will immediately trigger, and (if they don't make themselves inactive) be reinserted still before the current time. This leads to an infinite loop in spice_timer_queue_cb(). https://bugzilla.redhat.com/show_bug.cgi?id=1072700 ACK --- server/spice_timer_queue.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/server/spice_timer_queue.c b/server/spice_timer_queue.c index 8f6e9c8..71de84a 100644 --- a/server/spice_timer_queue.c +++ b/server/spice_timer_queue.c @@ -147,7 +147,7 @@ SpiceTimer *spice_timer_queue_add(SpiceTimerFunc func, void *opaque) return timer; } -static void _spice_timer_set(SpiceTimer *timer, uint32_t ms, uint32_t now) +static void _spice_timer_set(SpiceTimer *timer, uint32_t ms, uint64_t now) { RingItem *next_item; SpiceTimerQueue *queue; @@ -183,7 +183,8 @@ void spice_timer_set(SpiceTimer *timer, uint32_t ms) spice_assert(pthread_equal(timer-queue-thread, pthread_self()) != 0); clock_gettime(CLOCK_MONOTONIC, now); -_spice_timer_set(timer, ms, now.tv_sec * 1000 + (now.tv_nsec / 1000 / 1000)); +_spice_timer_set(timer, ms, + (uint64_t)now.tv_sec * 1000 + (now.tv_nsec / 1000 / 1000)); } void spice_timer_cancel(SpiceTimer *timer) @@ -217,7 +218,7 @@ void spice_timer_remove(SpiceTimer *timer) unsigned int spice_timer_queue_get_timeout_ms(void) { struct timespec now; -int now_ms; +int64_t now_ms; RingItem *head; SpiceTimer *head_timer; SpiceTimerQueue *queue = spice_timer_queue_find_with_lock(); @@ -232,9 +233,9 @@ unsigned int spice_timer_queue_get_timeout_ms(void) head_timer = SPICE_CONTAINEROF(head, SpiceTimer, active_link); clock_gettime(CLOCK_MONOTONIC, now); -now_ms = (now.tv_sec * 1000) - (now.tv_nsec / 1000 / 1000); +now_ms = ((int64_t)now.tv_sec * 1000) - (now.tv_nsec / 1000 / 1000); -return MAX(0, ((int)head_timer-expiry_time - now_ms)); +return MAX(0, ((int64_t)head_timer-expiry_time - now_ms)); } @@ -252,7 +253,7 @@ void spice_timer_queue_cb(void) } clock_gettime(CLOCK_MONOTONIC, now); -now_ms = (now.tv_sec * 1000) + (now.tv_nsec / 1000 / 1000); +now_ms = ((uint64_t)now.tv_sec * 1000) + (now.tv_nsec / 1000 / 1000); while ((head = ring_get_head(queue-active_timers))) { SpiceTimer *timer = SPICE_CONTAINEROF(head, SpiceTimer, active_link); ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] Releasing ownership of Spice components in Fedora
Hi All, It seems I was still the owner of various Spice components in Fedora. I've just released the following components: https://admin.fedoraproject.org/pkgdb/acls/name/spice https://admin.fedoraproject.org/pkgdb/acls/name/spice-vdagent Someone from the team should go to the above urls and pick these up. Regards, Hans ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Mouse mode client vs Mouse mode server
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 03/15/2014 06:33 AM, Lindsay Mathieson wrote: Whats the significance of this? does server mode come into play when there is no qxl spice agent in the guest? client/server mode has the client/server determine the location of the cursor and render it. So client mode has better mouse responsiveness, especially with high latency. A spice agent (vdagent) is required for client mode. You can also use a usb tablet in which case you get the same 'perk' but nothing else that the agent provides (unless you run it - you can run both, although it doesn't make sense). With a tablet you also cannot use multiple monitors (the mouse is mapped wrongly). ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTJr6uAAoJEGSFt2Lm6PXuBrsIALC8xR3Hu9owjveGDmM2dUYH /J4r9XCRKh2xM5VdrSdUgvVRLkjzkDkjorSpJBIR+vv3dXbUMNJmH8PP52t7zGed WZwjP8M4Kj9da0rzuMxFbbjoCz9W/ZJ4ylEGsQH6sDODkVVoWWNq8gIWNjxOaJPJ OnsD0bXXQiJH69Ac31sHuHfJLOURmfO72KCDNNpWPeN3uErxVdv5ecPJn92LUYev CfEPRvaJXMWPof9AblEYbfXxaqMdS1zG4d5837paJ/6Pa/932psklrJYUM6uc39W bbbVHoglrI/hFoL1BzbDAi6ezC3DMW3O4ONpYWIst2PtkSXT2zhTsXF7L0badBU= =p7yv -END PGP SIGNATURE- ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] wmctrl and spice
We have just moved to using SPICE and virt-viewer (as provided by Redhat with the Redhat Enterprise Virtualization server). In the past I have used wmctrl to modify windows, move them between desktops, hide them from the taskbar, etc. but this does not appear to work with version of SPICE that we have available (virt-viewer 0.5.618.el6_5).' The client is running on a Windows 7 desktop - the virtual machine is Redhat Enterprise Linux. Is this expected to work? Thanks! -- Dan -- The information contained in this communication and any attachments is confidential and may be privileged, and is for the sole use of the intended recipient(s). Any unauthorized review, use, disclosure or distribution is prohibited. Unless explicitly stated otherwise in the body of this communication or the attachment thereto (if any), the information is provided on an AS-IS basis without any express or implied warranties or liabilities. To the extent you are relying on this information, you are doing so at your own risk. If you are not the intended recipient, please notify the sender immediately by replying to this message and destroy all copies of this message and any attachments. ASML is neither liable for the proper and complete transmission of the information contained in this communication, nor for any delay in its receipt. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Releasing ownership of Spice components in Fedora
On Mon, Mar 17, 2014 at 10:08:27AM +0100, Hans de Goede wrote: Hi All, It seems I was still the owner of various Spice components in Fedora. I've just released the following components: https://admin.fedoraproject.org/pkgdb/acls/name/spice https://admin.fedoraproject.org/pkgdb/acls/name/spice-vdagent Someone from the team should go to the above urls and pick these up. I took ownership of spice, thanks for the notice! I can do spice-vdagent too if noone volunteers. Christophe pgp5wP9b1mwcu.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] wmctrl and spice
Hey, On Fri, Mar 14, 2014 at 02:22:03PM -0400, Daniel Rawson wrote: We have just moved to using SPICE and virt-viewer (as provided by Redhat with the Redhat Enterprise Virtualization server). In the past I have used wmctrl to modify windows, move them between desktops, hide them from the taskbar, etc. but this does not appear to work with version of SPICE that we have available (virt-viewer 0.5.618.el6_5).' (I guess this question could go to Red Hat support as well if you are using RH versions) The client is running on a Windows 7 desktop - the virtual machine is Redhat Enterprise Linux. When I look for wmctrl on google, I find http://tomas.styblo.name/wmctrl/ which is a linux tool. Is this the tool you are talking about? If so, I assume you are running it in the guest? The SPICE client you use should not make a difference with respect to the guest windows management, is the SPICE client you use the only difference with a setup where wmctrl works? Christophe pgprVoI1GEP0F.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH server] Associate org.spice-space.webdav.0 port to webdav channel
On Sun, Mar 16, 2014 at 07:01:16PM +0200, Alon Levy wrote: On 01/12/2014 07:34 PM, Marc-André Lureau wrote: For example, with qemu, a webdav channel can be created this way: -chardev spiceport,name=org.spice-space.webdav.0,... And redirected to a virtio port: -device virtserialport,...,name=org.spice-space.webdav.0 Ack. Still reviewing the gtk part. I've tested this. (don't forget to add http://lists.freedesktop.org/archives/spice-devel/2014-February/016259.html in there, but I assume the doc build patches you sent mean you looked at that). Christophe pgpAEK87yvZJN.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] migration: Don't assert() if MIGRATE_DATA comes before attaching the agent
On 02/24/2014 07:44 PM, Christophe Fergeau wrote: During seamless migration, after switching host, if a client was connected during the migration, it will have data to send back to the new qemu/spice-server instance. This is handled through MIGRATE_DATA messages. SPICE char devices use such MIGRATE_DATA messages to restore their state. However, the MIGRATE_DATA message can arrive any time after the new qemu instance has started, this can happen before or after the SPICE char devices have been created. In order to handle this, if the migrate data arrives early, it's stored in reds-agent_state.mig_data, and attach_to_red_agent() will restore the agent state as appropriate. Unfortunately this does not work as expected as expected. If attach_to_red_agent() is called before the MIGRATE_DATA message reaches the server, all goes well, but if MIGRATE_DATA reaches the server before attach_to_red_agent() gets called, then some assert() get triggered in spice_char_device_state_restore(): ((null):32507): Spice-ERROR **: char_device.c:937:spice_char_device_state_restore: assertion `dev-num_clients == 1 dev-wait_for_migrate_data' failed Thread 3 (Thread 0x7f406b543700 (LWP 32543)): Thread 2 (Thread 0x7f40697ff700 (LWP 32586)): Thread 1 (Thread 0x7f4079b45a40 (LWP 32507)): What happens is that dev-wait_for_migrate_data is unconditionally cleared when completing handling of the MIGRATE_DATA message, so it's FALSE when spice_char_device_state_restore() is called. Moreover, dev-num_clients is also 0 as this is only increased by spice_char_device_start() which spice_server_char_device_add_interface() calls after attach_to_red_agent()/spice_char_device_state_restore() have been called. This commit changes the logic in spice_server_char_device_add_interface(), and when there is migrate data pending in reds-agent_state.mig_data, we only attempt to restore it after successfully initializing/starting the needed char device. This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1035184 Looks good to me, ack. --- server/reds.c | 26 +++--- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/server/reds.c b/server/reds.c index 1f02553..217c74e 100644 --- a/server/reds.c +++ b/server/reds.c @@ -2856,16 +2856,7 @@ static SpiceCharDeviceState *attach_to_red_agent(SpiceCharDeviceInstance *sin) state-read_filter.discard_all = FALSE; reds-agent_state.plug_generation++; -if (reds-agent_state.mig_data) { -spice_assert(reds-agent_state.plug_generation == 1); -reds_agent_state_restore(reds-agent_state.mig_data); -free(reds-agent_state.mig_data); -reds-agent_state.mig_data = NULL; -} else if (!red_channel_waits_for_migrate_data(reds-main_channel-base)) { -/* we will assoicate the client with the char device, upon reds_on_main_agent_start, - * in response to MSGC_AGENT_START */ -main_channel_push_agent_connected(reds-main_channel); -} else { +if (red_channel_waits_for_migrate_data(reds-main_channel-base) || reds-agent_state.mig_data) { spice_debug(waiting for migration data); if (!spice_char_device_client_exists(reds-agent_state.base, reds_get_client())) { int client_added; @@ -2882,9 +2873,13 @@ static SpiceCharDeviceState *attach_to_red_agent(SpiceCharDeviceInstance *sin) spice_warning(failed to add client to agent); reds_disconnect(); } - } +} else { +/* we will associate the client with the char device, upon reds_on_main_agent_start, + * in response to MSGC_AGENT_START */ +main_channel_push_agent_connected(reds-main_channel); } + return state-base; } @@ -2990,6 +2985,15 @@ static int spice_server_char_device_add_interface(SpiceServer *s, spice_warning(failed to create device state for %s, char_device-subtype); return -1; } + +if (strcmp(char_device-subtype, SUBTYPE_VDAGENT) == 0) { +if (reds-agent_state.mig_data) { + spice_assert(reds-agent_state.plug_generation == 1); + reds_agent_state_restore(reds-agent_state.mig_data); + free(reds-agent_state.mig_data); + reds-agent_state.mig_data = NULL; +} +} return 0; } ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 1/7] Introduce reds_stream_async_read() helper
This will allow to make RedsStream::async_read private --- server/reds_stream.c | 77 +++- server/reds_stream.h | 2 ++ 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/server/reds_stream.c b/server/reds_stream.c index 873b953..57eb1d1 100644 --- a/server/reds_stream.c +++ b/server/reds_stream.c @@ -310,8 +310,6 @@ RedsStream *reds_stream_new(int socket) stream-priv-write = stream_write_cb; stream-priv-writev = stream_writev_cb; -stream-priv-async_read.stream = stream; - return stream; } @@ -396,11 +394,10 @@ void async_read_set_error_handler(AsyncRead *async, static inline void async_read_clear_handlers(AsyncRead *obj) { -if (!obj-stream-watch) { -return; +if (obj-stream-watch) { +reds_stream_remove_watch(obj-stream); } - -reds_stream_remove_watch(obj-stream); +obj-stream = NULL; } void async_read_handler(int fd, int event, void *data) @@ -449,6 +446,23 @@ void async_read_handler(int fd, int event, void *data) } } +void reds_stream_async_read(RedsStream *stream, +uint8_t *data, size_t size, +AsyncReadDone read_done_cb, +void *opaque) +{ +AsyncRead *obj = stream-priv-async_read; + +g_assert(!obj-stream); +obj-stream = stream; +obj-now = data; +obj-end = obj-now + size; +obj-done = read_done_cb; +obj-opaque = opaque; +async_read_handler(0, 0, obj); + +} + #if HAVE_SASL bool reds_stream_write_u8(RedsStream *s, uint8_t n) { @@ -638,7 +652,6 @@ RedsSaslError reds_sasl_handle_auth_step(RedsStream *stream, AsyncReadDone read_ char *clientdata = NULL; RedsSASL *sasl = stream-priv-sasl; uint32_t datalen = sasl-len; -AsyncRead *obj = stream-priv-async_read; /* NB, distinction of NULL vs is *critical* in SASL */ if (datalen) { @@ -683,10 +696,8 @@ RedsSaslError reds_sasl_handle_auth_step(RedsStream *stream, AsyncReadDone read_ if (err == SASL_CONTINUE) { spice_info(%s, Authentication must continue (step)); /* Wait for step length */ -obj-now = (uint8_t *)sasl-len; -obj-end = obj-now + sizeof(uint32_t); -obj-done = read_cb; -async_read_handler(0, 0, stream-priv-async_read); +reds_stream_async_read(stream, (uint8_t *)sasl-len, sizeof(uint32_t), + read_cb, opaque); return REDS_SASL_ERROR_CONTINUE; } else { int ssf; @@ -718,7 +729,6 @@ authreject: RedsSaslError reds_sasl_handle_auth_steplen(RedsStream *stream, AsyncReadDone read_cb, void *opaque) { -AsyncRead *obj = stream-priv-async_read; RedsSASL *sasl = stream-priv-sasl; spice_info(Got steplen %d, sasl-len); @@ -735,11 +745,8 @@ RedsSaslError reds_sasl_handle_auth_steplen(RedsStream *stream, AsyncReadDone re * treatment */ return REDS_SASL_ERROR_OK; } else { -sasl-data = spice_realloc(sasl-data, sasl-len); -obj-now = (uint8_t *)sasl-data; -obj-end = obj-now + sasl-len; -obj-done = read_cb; -async_read_handler(0, 0, obj); +reds_stream_async_read(stream, (uint8_t *)sasl-data, sasl-len, + read_cb, opaque); return REDS_SASL_ERROR_OK; } } @@ -761,7 +768,6 @@ RedsSaslError reds_sasl_handle_auth_steplen(RedsStream *stream, AsyncReadDone re RedsSaslError reds_sasl_handle_auth_start(RedsStream *stream, AsyncReadDone read_cb, void *opaque) { -AsyncRead *obj = stream-priv-async_read; const char *serverout; unsigned int serveroutlen; int err; @@ -813,10 +819,8 @@ RedsSaslError reds_sasl_handle_auth_start(RedsStream *stream, AsyncReadDone read if (err == SASL_CONTINUE) { spice_info(%s, Authentication must continue (start)); /* Wait for step length */ -obj-now = (uint8_t *)sasl-len; -obj-end = obj-now + sizeof(uint32_t); -obj-done = read_cb; -async_read_handler(0, 0, stream-priv-async_read); +reds_stream_async_read(stream, (uint8_t *)sasl-len, sizeof(uint32_t), + read_cb, opaque); return REDS_SASL_ERROR_CONTINUE; } else { int ssf; @@ -847,7 +851,6 @@ authreject: RedsSaslError reds_sasl_handle_auth_startlen(RedsStream *stream, AsyncReadDone read_cb, void *opaque) { -AsyncRead *obj = stream-priv-async_read; RedsSASL *sasl = stream-priv-sasl; spice_info(Got client start len %d, sasl-len); @@ -861,17 +864,14 @@ RedsSaslError reds_sasl_handle_auth_startlen(RedsStream *stream, AsyncReadDone r } sasl-data = spice_realloc(sasl-data, sasl-len); -obj-now = (uint8_t *)sasl-data; -obj-end = obj-now + sasl-len; -obj-done = read_cb; -async_read_handler(0, 0, obj); +reds_stream_async_read(stream, (uint8_t *)sasl-data, sasl-len, +
[Spice-devel] [PATCH 6/7] Call correct SASL helper in reds_handle_auth_sasl_step
sasl_handle_auth_start() was called instead of reds_sasl_handle_auth_step() --- server/reds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/reds.c b/server/reds.c index c1c0efc..fb2a19b 100644 --- a/server/reds.c +++ b/server/reds.c @@ -1901,7 +1901,7 @@ static void reds_handle_auth_sasl_step(void *opaque) RedLinkInfo *link = (RedLinkInfo *)opaque; RedsSaslError status; -status = reds_sasl_handle_auth_start(link-stream, reds_handle_auth_sasl_steplen, link); +status = reds_sasl_handle_auth_step(link-stream, reds_handle_auth_sasl_steplen, link); if (status == REDS_SASL_ERROR_OK) { reds_handle_link(link); } else if (status != REDS_SASL_ERROR_CONTINUE) { -- 1.8.5.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 5/7] Add G_GNUC_UNUSED annotations to async_read_handler args
2 of the arguments are not used, the G_GNUC_UNUSED annotation will make this explicit. --- server/reds_stream.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/reds_stream.c b/server/reds_stream.c index 9041033..8476066 100644 --- a/server/reds_stream.c +++ b/server/reds_stream.c @@ -409,7 +409,9 @@ static inline void async_read_clear_handlers(AsyncRead *obj) obj-stream = NULL; } -static void async_read_handler(int fd, int event, void *data) +static void async_read_handler(G_GNUC_UNUSED int fd, + G_GNUC_UNUSED int event, + void *data) { AsyncRead *obj = (AsyncRead *)data; -- 1.8.5.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 2/7] Add reds_stream_set_async_error_handler() helper
This replaces async_read_set_error_handler() which was unused. This sets a callback to be called when an async operation fails. --- server/reds_stream.c | 7 +++ server/reds_stream.h | 5 ++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/server/reds_stream.c b/server/reds_stream.c index 57eb1d1..cb42561 100644 --- a/server/reds_stream.c +++ b/server/reds_stream.c @@ -385,11 +385,10 @@ int reds_stream_enable_ssl(RedsStream *stream, SSL_CTX *ctx) return reds_stream_ssl_accept(stream); } -void async_read_set_error_handler(AsyncRead *async, - AsyncReadError error_handler, - void *opaque) +void reds_stream_set_async_error_handler(RedsStream *stream, + AsyncReadError error_handler) { -async-error = error_handler; +stream-priv-async_read.error = error_handler; } static inline void async_read_clear_handlers(AsyncRead *obj) diff --git a/server/reds_stream.h b/server/reds_stream.h index 866679a..a5b7a17 100644 --- a/server/reds_stream.h +++ b/server/reds_stream.h @@ -39,9 +39,6 @@ typedef struct AsyncRead { } AsyncRead; void async_read_handler(int fd, int event, void *data); -void async_read_set_error_handler(AsyncRead *async, - AsyncReadError error_handler, - void *opaque); typedef struct RedsStreamPrivate RedsStreamPrivate; @@ -67,6 +64,8 @@ typedef enum { ssize_t reds_stream_read(RedsStream *s, void *buf, size_t nbyte); void reds_stream_async_read(RedsStream *stream, uint8_t *data, size_t size, AsyncReadDone read_done_cb, void *opaque); +void reds_stream_set_async_error_handler(RedsStream *stream, + AsyncReadError error_handler); ssize_t reds_stream_write(RedsStream *s, const void *buf, size_t nbyte); ssize_t reds_stream_writev(RedsStream *s, const struct iovec *iov, int iovcnt); bool reds_stream_write_all(RedsStream *stream, const void *in_buf, size_t n); -- 1.8.5.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 7/7] Add missing buffer (re)allocation to reds_sasl_handle_auth_steplen()
We need to make sure we have a buffer big enough to accomodate the data sent by the coming SASL step. --- server/reds_stream.c | 1 + 1 file changed, 1 insertion(+) diff --git a/server/reds_stream.c b/server/reds_stream.c index 8476066..0b06771 100644 --- a/server/reds_stream.c +++ b/server/reds_stream.c @@ -756,6 +756,7 @@ RedsSaslError reds_sasl_handle_auth_steplen(RedsStream *stream, AsyncReadDone re * treatment */ return REDS_SASL_ERROR_OK; } else { +sasl-data = spice_realloc(sasl-data, sasl-len); reds_stream_async_read(stream, (uint8_t *)sasl-data, sasl-len, read_cb, opaque); return REDS_SASL_ERROR_OK; -- 1.8.5.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 4/7] Make struct AsyncRead/async_read_handler private
All users are now contained in reds_stream.c --- server/reds_stream.c | 12 +++- server/reds_stream.h | 11 --- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/server/reds_stream.c b/server/reds_stream.c index cb42561..9041033 100644 --- a/server/reds_stream.c +++ b/server/reds_stream.c @@ -33,6 +33,16 @@ #include openssl/err.h +struct AsyncRead { +RedsStream *stream; +void *opaque; +uint8_t *now; +uint8_t *end; +AsyncReadDone done; +AsyncReadError error; +}; +typedef struct AsyncRead AsyncRead; + extern SpiceCoreInterface *core; #if HAVE_SASL @@ -399,7 +409,7 @@ static inline void async_read_clear_handlers(AsyncRead *obj) obj-stream = NULL; } -void async_read_handler(int fd, int event, void *data) +static void async_read_handler(int fd, int event, void *data) { AsyncRead *obj = (AsyncRead *)data; diff --git a/server/reds_stream.h b/server/reds_stream.h index a5b7a17..6cb 100644 --- a/server/reds_stream.h +++ b/server/reds_stream.h @@ -29,17 +29,6 @@ typedef void (*AsyncReadDone)(void *opaque); typedef void (*AsyncReadError)(void *opaque, int err); typedef struct RedsStream RedsStream; -typedef struct AsyncRead { -RedsStream *stream; -void *opaque; -uint8_t *now; -uint8_t *end; -AsyncReadDone done; -AsyncReadError error; -} AsyncRead; - -void async_read_handler(int fd, int event, void *data); - typedef struct RedsStreamPrivate RedsStreamPrivate; struct RedsStream { -- 1.8.5.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 0/7] Fix SASL
The RedsStream refactoring broke SASL :( This series fixes that, first by finishing the move of async reads to RedsStream, and then by using this to start fixing SASL auth. 2 more commits are necessary to fix 2 more SASL bugs :( Christophe ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 3/7] Remove RedLinkInfo::async_read
9feed69 moved the async reader code to RedsStream so that it can be used for the SASL authentication code. In particular, it introduced a RedsStream::async_read member which is used by the SASL authentication code for its async operations. However, what was not done is to remove the now redundant RedLinkInfo::async_read field. This causes failures when using SASL authentication as the async read error callback is getting set on the RedLinkInfo::async_read structure, but then the SASL code is trying to use the RedeStream::async_read structure for its async IOs, which do not have the needed error callback set. This commit makes use of the newly introduced reds_stream_async_read() helper in order to make use of RedsStream::async_read. --- server/reds.c | 44 +++- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/server/reds.c b/server/reds.c index 2ef4aad..c1c0efc 100644 --- a/server/reds.c +++ b/server/reds.c @@ -123,7 +123,6 @@ static RedsState *reds = NULL; typedef struct RedLinkInfo { RedsStream *stream; -AsyncRead async_read; SpiceLinkHeader link_header; SpiceLinkMess *link_mess; int mess_pos; @@ -1873,12 +1872,9 @@ end: static void reds_get_spice_ticket(RedLinkInfo *link) { -AsyncRead *obj = link-async_read; - -obj-now = (uint8_t *)link-tiTicketing.encrypted_ticket.encrypted_data; -obj-end = obj-now + link-tiTicketing.rsa_size; -obj-done = reds_handle_ticket; -async_read_handler(0, 0, link-async_read); +reds_stream_async_read(link-stream, + (uint8_t *)link-tiTicketing.encrypted_ticket.encrypted_data, + link-tiTicketing.rsa_size, reds_handle_ticket, link); } #if HAVE_SASL @@ -2041,7 +2037,6 @@ static void reds_handle_read_link_done(void *opaque) { RedLinkInfo *link = (RedLinkInfo *)opaque; SpiceLinkMess *link_mess = link-link_mess; -AsyncRead *obj = link-async_read; uint32_t num_caps = link_mess-num_common_caps + link_mess-num_channel_caps; uint32_t *caps = (uint32_t *)((uint8_t *)link_mess + link_mess-caps_offset); int auth_selection; @@ -2083,10 +2078,11 @@ static void reds_handle_read_link_done(void *opaque) spice_warning(Peer doesn't support AUTH selection); reds_get_spice_ticket(link); } else { -obj-now = (uint8_t *)link-auth_mechanism; -obj-end = obj-now + sizeof(SpiceLinkAuthMechanism); -obj-done = reds_handle_auth_mechanism; -async_read_handler(0, 0, link-async_read); +reds_stream_async_read(link-stream, + (uint8_t *)link-auth_mechanism, + sizeof(SpiceLinkAuthMechanism), + reds_handle_auth_mechanism, + link); } } @@ -2108,7 +2104,6 @@ static void reds_handle_read_header_done(void *opaque) { RedLinkInfo *link = (RedLinkInfo *)opaque; SpiceLinkHeader *header = link-link_header; -AsyncRead *obj = link-async_read; if (header-magic != SPICE_MAGIC) { reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC); @@ -2137,22 +2132,21 @@ static void reds_handle_read_header_done(void *opaque) link-link_mess = spice_malloc(header-size); -obj-now = (uint8_t *)link-link_mess; -obj-end = obj-now + header-size; -obj-done = reds_handle_read_link_done; -async_read_handler(0, 0, link-async_read); +reds_stream_async_read(link-stream, + (uint8_t *)link-link_mess, + header-size, + reds_handle_read_link_done, + link); } static void reds_handle_new_link(RedLinkInfo *link) { -AsyncRead *obj = link-async_read; -obj-opaque = link; -obj-stream = link-stream; -obj-now = (uint8_t *)link-link_header; -obj-end = (uint8_t *)((SpiceLinkHeader *)link-link_header + 1); -obj-done = reds_handle_read_header_done; -obj-error = reds_handle_link_error; -async_read_handler(0, 0, link-async_read); +reds_stream_set_async_error_handler(link-stream, reds_handle_link_error); +reds_stream_async_read(link-stream, + (uint8_t *)link-link_header, + sizeof(SpiceLinkHeader), + reds_handle_read_header_done, + link); } static void reds_handle_ssl_accept(int fd, int event, void *data) -- 1.8.5.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Mouse mode client vs Mouse mode server
On Mon, 17 Mar 2014 11:21:50 AM Alon Levy wrote: client/server mode has the client/server determine the location of the cursor and render it. So client mode has better mouse responsiveness, especially with high latency. A spice agent (vdagent) is required for client mode. Thought that might be the case, thanks. -- Lindsay signature.asc Description: This is a digitally signed message part. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel