Re: [Spice-devel] [PATCH spice-protocol] stream-device: Force proper packing of protocol structures

> On 7 Mar 2018, at 17:11, Frediano Ziglio wrote: > >> >> Interesting and creative follow up on our discussion :-) >> >>> On 7 Mar 2018, at 12:08, Frediano Ziglio wrote: >>> >>> Assuming the packing of structure can be a portability issue. >>>

Re: [Spice-devel] [PATCH usbredirserver] usbredirserver : enable TCP keepalive

On 03/08/2018 03:30 AM, Uri Lublin wrote: On 03/01/2018 11:08 AM, zhenwei.pi wrote: In some bad cases, for example, host OS crashes without sending any FIN to usbredirserver, and usbredirserver will keep idle connection for a long time. We can also set the kernel arguments, it means that other

Re: [Spice-devel] [PATCH 12/22] Add exception handling classes

> On 7 Mar 2018, at 12:52, Frediano Ziglio wrote: > >>> On 2 Mar 2018, at 18:13, Christophe Fergeau wrote: >>> >>> On Fri, Mar 02, 2018 at 02:00:23PM +0100, Christophe de Dinechin wrote: > On 2 Mar 2018, at 09:03, Frediano Ziglio

[Spice-devel] [spice-server v2] reds: Close sockets when failing to watch them

Currently if we fail to set up the watch waiting for accept() to be called on the socket, we still keep the network socket(s) open even if we are not going to be able to use it. This commit makes sure it's closed a set to -1 when such a failure occurs rather than having a half initialized

Re: [Spice-devel] [spice-server v2] reds: Close sockets when failing to watch them

On Thu, Mar 08, 2018 at 05:50:29AM -0500, Frediano Ziglio wrote: > > > > Currently if we fail to set up the watch waiting for accept() to be > > called on the socket, we still keep the network socket(s) open even if we > > are not going to be able to use it. This commit makes sure it's closed a >

[Spice-devel] [spice-server v3] reds: Close sockets when failing to watch them

Currently if we fail to set up the watch waiting for accept() to be called on the socket, we still keep the network socket(s) open even if we are not going to be able to use it. This commit makes sure it's closed a set to -1 when such a failure occurs rather than having a half initialized

Re: [Spice-devel] [spice-server v3] reds: Close sockets when failing to watch them

On 03/08/2018 03:23 PM, Christophe Fergeau wrote: Currently if we fail to set up the watch waiting for accept() to be called on the socket, we still keep the network socket(s) open even if we are not going to be able to use it. This commit makes sure it's closed a set to -1 when such a failure

Re: [Spice-devel] [PATCH 06/22] Get rid of C-style memset initializations, use C++ style aggregates

> On 7 Mar 2018, at 11:50, Frediano Ziglio wrote: > >>> On 6 Mar 2018, at 17:15, Christophe Fergeau wrote: >>> >>> On Thu, Mar 01, 2018 at 09:01:37PM +0100, Christophe de Dinechin wrote: > On 28 Feb 2018, at 17:36, Christophe Fergeau

Re: [Spice-devel] [PATCH 06/22] Get rid of C-style memset initializations, use C++ style aggregates

> > > On 7 Mar 2018, at 11:50, Frediano Ziglio wrote: > > > >>> On 6 Mar 2018, at 17:15, Christophe Fergeau wrote: > >>> > >>> On Thu, Mar 01, 2018 at 09:01:37PM +0100, Christophe de Dinechin wrote: > > On 28 Feb 2018, at 17:36, Christophe Fergeau

Re: [Spice-devel] [PATCH 06/22] Get rid of C-style memset initializations, use C++ style aggregates

> > On Thu, Mar 08, 2018 at 05:39:48AM -0500, Frediano Ziglio wrote: > > > > There are however still some issues: > > > > - the syntax is using C++20 while we state we use C++11 syntax, this > > > > is basically using C compatibility extensions. I just tried and for > > > > instance this code

Re: [Spice-devel] [spice-server v2] reds: Close sockets when failing to watch them

> > Currently if we fail to set up the watch waiting for accept() to be > called on the socket, we still keep the network socket(s) open even if we > are not going to be able to use it. This commit makes sure it's closed a > set to -1 when such a failure occurs rather than having a half >

Re: [Spice-devel] [PATCH spice-server v2 2/2] stream-device: Create channels before first non-main channel connection

Hey, I assume the client is not going to show an unwanted window or something like that? Looks good to me, Acked-by: Christophe Fergeau though maybe people more familiar with the streaming channel will want to take a look too. Christophe On Wed, Mar 07, 2018 at 08:26:16AM

Re: [Spice-devel] [PATCH spice-server v2 2/2] stream-device: Create channels before first non-main channel connection

> Hey, > > I assume the client is not going to show an unwanted window or something > like that? > No, the client window are shown when there's a surface. > Looks good to me, Acked-by: Christophe Fergeau > though maybe people more familiar with the streaming channel will

Re: [Spice-devel] [PATCH 06/22] Get rid of C-style memset initializations, use C++ style aggregates

On Thu, Mar 08, 2018 at 05:39:48AM -0500, Frediano Ziglio wrote: > > > There are however still some issues: > > > - the syntax is using C++20 while we state we use C++11 syntax, this > > > is basically using C compatibility extensions. I just tried and for > > > instance this code is not

[Spice-devel] [PATCH spice-server] dcc: Remove unused channel parameter

Signed-off-by: Frediano Ziglio --- server/dcc.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/server/dcc.c b/server/dcc.c index d457989b..15b65978 100644 --- a/server/dcc.c +++ b/server/dcc.c @@ -676,8 +676,7 @@ void

Re: [Spice-devel] [PATCH spice-server v2 1/2] stream-device: Separate declaration in a separate header

On Wed, Mar 07, 2018 at 08:26:15AM +, Frediano Ziglio wrote: > Move public declaration (stream_device_connect) from char-device.h > to a new stream-device.h. > Add type declaration for StreamDevice. > This allows to use the type outside the implementation file and "outside *of* the

Re: [Spice-devel] [PATCH spice-server v2 1/2] stream-device: Separate declaration in a separate header

> > On Wed, Mar 07, 2018 at 08:26:15AM +, Frediano Ziglio wrote: > > Move public declaration (stream_device_connect) from char-device.h > > to a new stream-device.h. > > Add type declaration for StreamDevice. > > This allows to use the type outside the implementation file and > > "outside

Re: [Spice-devel] [PATCH 06/22] Get rid of C-style memset initializations, use C++ style aggregates

> On 8 Mar 2018, at 11:39, Frediano Ziglio wrote: > >>> >>> On 7 Mar 2018, at 11:50, Frediano Ziglio wrote: >>> > On 6 Mar 2018, at 17:15, Christophe Fergeau wrote: > > On Thu, Mar 01, 2018 at 09:01:37PM +0100,

Re: [Spice-devel] [PATCH usbredirserver] usbredirserver : enable TCP keepalive

> > On 03/08/2018 03:30 AM, Uri Lublin wrote: > > > On 03/01/2018 11:08 AM, zhenwei.pi wrote: > >> In some bad cases, for example, host OS crashes without > >> sending any FIN to usbredirserver, and usbredirserver > >> will keep idle connection for a long time. > >> We can also set the kernel

Re: [Spice-devel] [PATCH 12/22] Add exception handling classes

On Thu, Mar 08, 2018 at 10:56:55AM +0100, Christophe de Dinechin wrote: > > In many cases you want to add information to the exception and > > is quite common to catch one exception, add another informations > > and throw a more detailed exception. > > It is useful, indeed. And when you need to

[Spice-devel] [PATCH spice-server] ci: Fix compiling of some functions

Address sanitizer use a larger stack than normal compiled programs making the build fails. This causes this error on GitLab CI system: stream-device.c: In function 'stream_device_partial_read': stream-device.c:182:1: error: the frame size of 32992 bytes is larger than 20460 bytes

Re: [Spice-devel] [PATCH 09/22] Move read, write, handle and locking into the 'Stream' class

On Thu, Mar 01, 2018 at 08:52:50PM +0100, Christophe de Dinechin wrote: > > > > On 1 Mar 2018, at 11:59, Frediano Ziglio wrote: > > > >> > >> From: Christophe de Dinechin > >> > >> The 'Stream' class is designed to abstract file I/O. In a subsequent

Re: [Spice-devel] [PATCH 06/22] Get rid of C-style memset initializations, use C++ style aggregates

> On 8 Mar 2018, at 12:42, Christophe Fergeau wrote: > > On Thu, Mar 08, 2018 at 05:39:48AM -0500, Frediano Ziglio wrote: There are however still some issues: - the syntax is using C++20 while we state we use C++11 syntax, this is basically using C

Re: [Spice-devel] [PATCH spice-streaming-agent 2/2] Do not redefine "msg" field

> > msg.msg was redefining msg.StreamMsgNotifyError::msg. > This cause some confusion. > > Signed-off-by: Frediano Ziglio > --- > src/spice-streaming-agent.cpp | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/spice-streaming-agent.cpp

[Spice-devel] [PATCH spice-streaming-agent 2/2] Do not redefine "msg" field

msg.msg was redefining msg.StreamMsgNotifyError::msg. This cause some confusion. Signed-off-by: Frediano Ziglio --- src/spice-streaming-agent.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/spice-streaming-agent.cpp

[Spice-devel] [PATCH spice-streaming-agent 1/2] Add name to an unnamed structure

Unnamed structure combined with inheritance is considered a bit hard to read, add a name to make more readable. Signed-off-by: Frediano Ziglio --- src/spice-streaming-agent.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spice-streaming-agent.cpp

Re: [Spice-devel] [PATCH] usbredirserver : enable TCP keepalive

> > In some bad cases, for example, host OS crashes without > sending any FIN to usbredirserver, and usbredirserver > will keep idle connection for a long time. > > We can also set the kernel arguments, it means that other > processes may be affected. > > Setting a sensible timeout(like 10

Re: [Spice-devel] [PATCH] Use scancode instead of keycode names

Hi, On Thu, Mar 8, 2018 at 4:27 PM, Daniel P. Berrangé wrote: > On Thu, Mar 08, 2018 at 04:19:03PM +0100, Olivier Fourdan wrote: > > When running on Xwayland, the keycode mapping property is not available, > > which causes unknown keycode mapping errors and the keyboard

Re: [Spice-devel] [PATCH] Use scancode instead of keycode names

On Thu, Mar 08, 2018 at 05:22:01PM +0100, Olivier Fourdan wrote: > Hi, > > On Thu, Mar 8, 2018 at 4:27 PM, Daniel P. Berrangé > wrote: > > > On Thu, Mar 08, 2018 at 04:19:03PM +0100, Olivier Fourdan wrote: > > > When running on Xwayland, the keycode mapping property is not

Re: [Spice-devel] [spice-streaming-agent PATCH] handle_stream_error: add comment for inheriting struct

> On Wed, 2018-03-07 at 13:56 +0100, Christophe de Dinechin wrote: > > > On 7 Mar 2018, at 07:40, Frediano Ziglio wrote: > > > > > > > > > > > Introduced in 548577dc8adae1a558 > > > > > > > > Signed-off-by: Uri Lublin > > > > --- > > > >

[Spice-devel] [PATCH] Use scancode instead of keycode names

When running on Xwayland, the keycode mapping property is not available, which causes unknown keycode mapping errors and the keyboard doesn't work. Use a known scancode (“XK_Page_Up”) to check against the AT scancode and use “xfree86” if it matches or assume “evdev” for anything else.

Re: [Spice-devel] [PATCH] Use scancode instead of keycode names

On Thu, Mar 08, 2018 at 04:19:03PM +0100, Olivier Fourdan wrote: > When running on Xwayland, the keycode mapping property is not available, > which causes unknown keycode mapping errors and the keyboard doesn't > work. > > Use a known scancode (“XK_Page_Up”) to check against the AT scancode and >

Re: [Spice-devel] [PATCH 06/22] Get rid of C-style memset initializations, use C++ style aggregates

> > > On 8 Mar 2018, at 12:42, Christophe Fergeau wrote: > > > > On Thu, Mar 08, 2018 at 05:39:48AM -0500, Frediano Ziglio wrote: > There are however still some issues: > - the syntax is using C++20 while we state we use C++11 syntax, this > is basically

Re: [Spice-devel] [spice-streaming-agent PATCH] handle_stream_error: add comment for inheriting struct

On Wed, 2018-03-07 at 13:56 +0100, Christophe de Dinechin wrote: > > On 7 Mar 2018, at 07:40, Frediano Ziglio wrote: > > > > > > > > Introduced in 548577dc8adae1a558 > > > > > > Signed-off-by: Uri Lublin > > > --- > > > src/spice-streaming-agent.cpp | 5