Re: [Spice-devel] [PATCH spice-gtk 1/2] Add webdav channel

2014-03-17 Thread Alon Levy
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

2014-03-17 Thread Alon Levy
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

2014-03-17 Thread Alon Levy
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

2014-03-17 Thread Hans de Goede
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

2014-03-17 Thread Alon Levy
-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

2014-03-17 Thread Daniel Rawson

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

2014-03-17 Thread Christophe Fergeau
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

2014-03-17 Thread Christophe Fergeau
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

2014-03-17 Thread Christophe Fergeau
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

2014-03-17 Thread Alon Levy
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

2014-03-17 Thread Christophe Fergeau
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

2014-03-17 Thread Christophe Fergeau
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

2014-03-17 Thread Christophe Fergeau
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

2014-03-17 Thread Christophe Fergeau
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()

2014-03-17 Thread Christophe Fergeau
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

2014-03-17 Thread Christophe Fergeau
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

2014-03-17 Thread Christophe Fergeau
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

2014-03-17 Thread Christophe Fergeau
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

2014-03-17 Thread Lindsay Mathieson
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