[Spice-devel] [PATCH v2] Switch over to using keycodemapdb submodule

2017-02-15 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange --- Changed in v2: - Newer submodule hash to pull in python 3 portability fixes .gitmodules | 3 + configure.ac | 11 -- src/Makefile.am | 30 ++-- src/keycodemapdb | 1 + src/keymap-gen.pl | 214 --- src/keymaps.csv

Re: [Spice-devel] bool or gboolean

2017-02-15 Thread Jonathon Jongsma
On Wed, 2017-02-15 at 18:01 +0100, Christophe Fergeau wrote: > On Wed, Feb 15, 2017 at 11:39:32AM -0500, Frediano Ziglio wrote: > > Hi, > >   question was raised recently on the ML and IRC. > > > > Some time ago we decided to use gboolean but some of us would like > > to discuss again. > > So...

Re: [Spice-devel] [PATCH spice-server 2/2] sound: Use memcpy instead of manually copy volume array

2017-02-15 Thread Frediano Ziglio
> > On Wed, 2017-02-15 at 12:07 -0500, Frediano Ziglio wrote: > > > > > > On Thu, Feb 02, 2017 at 04:29:37PM +, Frediano Ziglio wrote: > > > > Signed-off-by: Frediano Ziglio > > > > --- > > > >  server/sound.c | 6 ++ > > > >  1 file changed, 2 insertions(+), 4 deletions(-) > > > > > > >

Re: [Spice-devel] [PATCH spice-server 2/2] sound: Use memcpy instead of manually copy volume array

2017-02-15 Thread Pavel Grunt
On Wed, 2017-02-15 at 12:07 -0500, Frediano Ziglio wrote: > > > > On Thu, Feb 02, 2017 at 04:29:37PM +, Frediano Ziglio wrote: > > > Signed-off-by: Frediano Ziglio > > > --- > > >  server/sound.c | 6 ++ > > >  1 file changed, 2 insertions(+), 4 deletions(-) > > > > > > diff --git a/serve

Re: [Spice-devel] [PATCH spice-server 2/2] sound: Use memcpy instead of manually copy volume array

2017-02-15 Thread Frediano Ziglio
> > On Thu, Feb 02, 2017 at 04:29:37PM +, Frediano Ziglio wrote: > > Signed-off-by: Frediano Ziglio > > --- > > server/sound.c | 6 ++ > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/server/sound.c b/server/sound.c > > index 7c36174..b692646 100644 > > --- a/ser

Re: [Spice-devel] bool or gboolean

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 11:39:32AM -0500, Frediano Ziglio wrote: > Hi, > question was raised recently on the ML and IRC. > > Some time ago we decided to use gboolean but some of us would like > to discuss again. So... As said a few times, I would have preferred to wait for a bit to have that di

Re: [Spice-devel] bool or gboolean

2017-02-15 Thread Daniel P. Berrange
On Wed, Feb 15, 2017 at 11:39:32AM -0500, Frediano Ziglio wrote: > Hi, > question was raised recently on the ML and IRC. > > Some time ago we decided to use gboolean but some of us would like > to discuss again. > > As any style changes there's no right or wrong, mainly personal > opinions but

Re: [Spice-devel] [PATCH spice-server 1/3] Use class name in comment

2017-02-15 Thread Frediano Ziglio
Yes, changed all, sending a v2 > > On Wed, Feb 15, 2017 at 11:46:21AM -0500, Frediano Ziglio wrote: > > > > > > ACK, maybe rcc: in the short log > > > > > > > I'd use red-channel-client: as consistent with the second... > > or change both to rcc! > > I meant to suggest to change the second on

Re: [Spice-devel] [PATCH spice-server 2/3] red-channel-client: Remove unused vfuncs

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 11:44:53AM -0500, Frediano Ziglio wrote: > > > > On Wed, Feb 15, 2017 at 12:49:06PM +, Frediano Ziglio wrote: > > > RedChannelClient is responsible for talking to the client so it knows > > > how if is connected or not. > > > > s/how// > > > > I'd mention explicitly t

Re: [Spice-devel] [PATCH spice-server 1/3] Use class name in comment

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 11:46:21AM -0500, Frediano Ziglio wrote: > > > > ACK, maybe rcc: in the short log > > > > I'd use red-channel-client: as consistent with the second... > or change both to rcc! I meant to suggest to change the second one to rcc but forgot. Only reason for red-channel-clie

Re: [Spice-devel] [PATCH spice-server 2/2] sound: Use memcpy instead of manually copy volume array

2017-02-15 Thread Christophe Fergeau
On Thu, Feb 02, 2017 at 04:29:37PM +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > server/sound.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/server/sound.c b/server/sound.c > index 7c36174..b692646 100644 > --- a/server/sound.c > +++ b/s

Re: [Spice-devel] [PATCH spice-server 1/3] Use class name in comment

2017-02-15 Thread Frediano Ziglio
> > ACK, maybe rcc: in the short log > I'd use red-channel-client: as consistent with the second... or change both to rcc! Not strong opinions, on the logs there are both versions Frediano > On Wed, Feb 15, 2017 at 12:49:05PM +, Frediano Ziglio wrote: > > red_channel seems a variable name

Re: [Spice-devel] [PATCH spice-server 2/3] red-channel-client: Remove unused vfuncs

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 12:49:06PM +, Frediano Ziglio wrote: > RedChannelClient is responsible for talking to the client so it knows > how if is connected or not. s/how// I'd mention explicitly that it's RedChannelClient::is_connected and RedChannelClient::disconnect which are dropped (would

Re: [Spice-devel] [PATCH spice-server 2/3] red-channel-client: Remove unused vfuncs

2017-02-15 Thread Frediano Ziglio
> > On Wed, Feb 15, 2017 at 12:49:06PM +, Frediano Ziglio wrote: > > RedChannelClient is responsible for talking to the client so it knows > > how if is connected or not. > > s/how// > > I'd mention explicitly that it's RedChannelClient::is_connected and > RedChannelClient::disconnect which

Re: [Spice-devel] [PATCH spice-server 3/3] Move stuff only related to RedChannelClient into red-channel-client.c

2017-02-15 Thread Christophe Fergeau
I'd change the log to something like: "rcc: Make some functions/macros private Some constants/macros/function prototypes are defined in red-channel-client.h while they are only used by red-channel-client.c. This commit moves them to the .c file to make them private" Acked-by: Christophe Fergea

[Spice-devel] bool or gboolean

2017-02-15 Thread Frediano Ziglio
Hi, question was raised recently on the ML and IRC. Some time ago we decided to use gboolean but some of us would like to discuss again. As any style changes there's no right or wrong, mainly personal opinions but I think consistency is quite important. Some consideration (feel free to add/rem

Re: [Spice-devel] [PATCH spice-server 1/3] Use class name in comment

2017-02-15 Thread Christophe Fergeau
ACK, maybe rcc: in the short log On Wed, Feb 15, 2017 at 12:49:05PM +, Frediano Ziglio wrote: > red_channel seems a variable name. > > Signed-off-by: Frediano Ziglio > --- > server/red-channel-client.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/server/red-chan

Re: [Spice-devel] [spice-server v4 5/5] Change some spice_printerr() to spice_debug()

2017-02-15 Thread Frediano Ziglio
> > > > On 15 Feb 2017, at 12:45, Frediano Ziglio wrote: > > > >> @@ -134,7 +134,7 @@ static void red_qxl_display_migrate(RedChannelClient > >> *rcc) > >> } > >> g_object_get(channel, "channel-type", &type, "id", &id, NULL); > >> dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT

Re: [Spice-devel] [PATCH 1/2] authentication: Handle failed SASL authentication separately

2017-02-15 Thread Snir Sheriber
Hi, Most of them will never show up, and if they will, it always starts with "Unable to authenticate" so i thought it's good enough , but no problem, i can show the user only the relevant ones and log the others (or to ignore them :] ) On 02/15/2017 05:00 PM, Christophe Fergeau wrote: On W

Re: [Spice-devel] [spice-server v4 5/5] Change some spice_printerr() to spice_debug()

2017-02-15 Thread Jonathon Jongsma
On Wed, 2017-02-15 at 16:34 +0100, Christophe de Dinechin wrote: > > On 15 Feb 2017, at 12:45, Frediano Ziglio > > wrote: > > > > > @@ -134,7 +134,7 @@ static void > > > red_qxl_display_migrate(RedChannelClient > > > *rcc) > > > } > > > g_object_get(channel, "channel-type", &type, "id", &

Re: [Spice-devel] [PATCH] Switch over to using keycodemapdb submodule

2017-02-15 Thread Daniel P. Berrange
On Wed, Feb 15, 2017 at 04:44:22PM +0100, Pavel Grunt wrote: > nice, although the python script looks pretty heavy. Different users of the keymap database have different output formats. We don't want ever user re-implementing the same logic, as that's just as bad what we have today. So the tool is

Re: [Spice-devel] [spice-server v2 14/14] rcc: Consistently name RedChannelClient 'rcc'

2017-02-15 Thread Jonathon Jongsma
On Wed, 2017-02-15 at 08:51 +0100, Christophe Fergeau wrote: > On Tue, Feb 14, 2017 at 12:14:56PM -0600, Jonathon Jongsma wrote: > > To be honest, I could go either way on this one. I like the > > consistency, but I don't like the fact that it makes things like > > "git > > blame" less useful. I'll

Re: [Spice-devel] [PATCH] Switch over to using keycodemapdb submodule

2017-02-15 Thread Pavel Grunt
Hi, nice, although the python script looks pretty heavy. Anyway, in spice-gtk we support python3, imo it cannot go in without the python3 support. Thanks, Pavel On Wed, 2017-02-15 at 15:17 +, Daniel P. Berrange wrote: > Signed-off-by: Daniel P. Berrange > --- >  .gitmodules   |   3 +

Re: [Spice-devel] [spice-server v4 5/5] Change some spice_printerr() to spice_debug()

2017-02-15 Thread Christophe de Dinechin
> On 15 Feb 2017, at 12:45, Frediano Ziglio wrote: > >> @@ -134,7 +134,7 @@ static void red_qxl_display_migrate(RedChannelClient >> *rcc) >> } >> g_object_get(channel, "channel-type", &type, "id", &id, NULL); >> dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel), >> "

[Spice-devel] [PATCH] Switch over to using keycodemapdb submodule

2017-02-15 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange --- .gitmodules | 3 + configure.ac | 11 -- src/Makefile.am | 30 ++-- src/keycodemapdb | 1 + src/keymap-gen.pl | 214 --- src/keymaps.csv | 511 -- 6 files changed, 1

Re: [Spice-devel] [PATCH spice-server 3/4] reds-stream: Introduce reds_stream_set_no_delay() helper

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 03:23:36PM +0100, Pavel Grunt wrote: > On Wed, 2017-02-15 at 15:16 +0100, Christophe Fergeau wrote: > > On Wed, Feb 15, 2017 at 09:14:10AM -0500, Frediano Ziglio wrote: > > > > > > > > On Wed, Feb 15, 2017 at 06:39:13AM -0500, Frediano Ziglio wrote: > > > > > > > > > > Pat

Re: [Spice-devel] [PATCH 1/2] authentication: Handle failed SASL authentication separately

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 02:56:36PM +0200, Snir Sheriber wrote: > Hi, > yes, the idea is to present errors which are generated on the sasl server > side, in the err window on the user/sasl-client side (only errors- without > sasl_ok, continue , interact) by sending the error number to the client and

Re: [Spice-devel] [PATCH spice-server 3/4] reds-stream: Introduce reds_stream_set_no_delay() helper

2017-02-15 Thread Pavel Grunt
On Wed, 2017-02-15 at 15:16 +0100, Christophe Fergeau wrote: > On Wed, Feb 15, 2017 at 09:14:10AM -0500, Frediano Ziglio wrote: > > > > > > On Wed, Feb 15, 2017 at 06:39:13AM -0500, Frediano Ziglio wrote: > > > > > > > > Patch is good however the use of bool is inconsistent. > > > > Although I _m

Re: [Spice-devel] [PATCH spice-server 3/4] reds-stream: Introduce reds_stream_set_no_delay() helper

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 09:14:10AM -0500, Frediano Ziglio wrote: > > > > On Wed, Feb 15, 2017 at 06:39:13AM -0500, Frediano Ziglio wrote: > > > > > > Patch is good however the use of bool is inconsistent. > > > Although I _much_ prefer bool/true/false we decided for > > > gboolean/TRUE/FALSE. > >

Re: [Spice-devel] [PATCH spice-server 3/4] reds-stream: Introduce reds_stream_set_no_delay() helper

2017-02-15 Thread Frediano Ziglio
> > On Wed, Feb 15, 2017 at 06:39:13AM -0500, Frediano Ziglio wrote: > > > > Patch is good however the use of bool is inconsistent. > > Although I _much_ prefer bool/true/false we decided for > > gboolean/TRUE/FALSE. > > reds-stream.[ch] is already exclusively using bool instead of gboolean, > s

Re: [Spice-devel] [PATCH spice-server 3/4] reds-stream: Introduce reds_stream_set_no_delay() helper

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 06:39:13AM -0500, Frediano Ziglio wrote: > > Patch is good however the use of bool is inconsistent. > Although I _much_ prefer bool/true/false we decided for gboolean/TRUE/FALSE. reds-stream.[ch] is already exclusively using bool instead of gboolean, so I'd prefer to stick

Re: [Spice-devel] [PATCH spice-server] sound: Avoid unused IOV_MAX definition

2017-02-15 Thread Frediano Ziglio
> > Hi, > > On Wed, 2017-02-15 at 12:37 +, Frediano Ziglio wrote: > > The usage was removed with commit > > 7ea1f2c133a8e57523078ba3112cec6a1d2f353e > > ("sound: Use RedChannelClient to receive/send data"). > > > > Signed-off-by: Frediano Ziglio > > --- > >  server/sound.c | 4 > >  1 f

Re: [Spice-devel] [PATCH spice-server] sound: Avoid unused IOV_MAX definition

2017-02-15 Thread Pavel Grunt
Hi, On Wed, 2017-02-15 at 12:37 +, Frediano Ziglio wrote: > The usage was removed with commit > 7ea1f2c133a8e57523078ba3112cec6a1d2f353e > ("sound: Use RedChannelClient to receive/send data"). > > Signed-off-by: Frediano Ziglio > --- >  server/sound.c | 4 >  1 file changed, 4 deletions(

Re: [Spice-devel] [PATCH 1/2] authentication: Handle failed SASL authentication separately

2017-02-15 Thread Snir Sheriber
Hi, On 02/14/2017 06:42 PM, Christophe Fergeau wrote: On Mon, Feb 13, 2017 at 03:49:44PM +0200, Snir Sheriber wrote: Remove handling with failures in the SASL authentication process to separate function and display the error message as reported by the SASL client (could also display SASL serve

[Spice-devel] [PATCH spice-server 1/3] Use class name in comment

2017-02-15 Thread Frediano Ziglio
red_channel seems a variable name. Signed-off-by: Frediano Ziglio --- server/red-channel-client.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/red-channel-client.h b/server/red-channel-client.h index 4f1d481..75d6cc3 100644 --- a/server/red-channel-client.h +++ b/se

[Spice-devel] [PATCH spice-server 3/3] Move stuff only related to RedChannelClient into red-channel-client.c

2017-02-15 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio --- server/red-channel-client.c | 25 - server/red-channel-client.h | 23 --- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/server/red-channel-client.c b/server/red-channel-client.c index 16e5446..a0e213

[Spice-devel] [PATCH spice-server 2/3] red-channel-client: Remove unused vfuncs

2017-02-15 Thread Frediano Ziglio
RedChannelClient is responsible for talking to the client so it knows how if is connected or not. About disconnect you think that we loose a bit of flexibility however on disconnect the channel is triggered so additional needed process can be handled in destructor. These vfuncs where used by DummyC

[Spice-devel] [PATCH spice-server] sound: Avoid unused IOV_MAX definition

2017-02-15 Thread Frediano Ziglio
The usage was removed with commit 7ea1f2c133a8e57523078ba3112cec6a1d2f353e ("sound: Use RedChannelClient to receive/send data"). Signed-off-by: Frediano Ziglio --- server/sound.c | 4 1 file changed, 4 deletions(-) diff --git a/server/sound.c b/server/sound.c index 530a7ec..afe9c96 100644

Re: [Spice-devel] [PATCH spice-server 2/4] Do not set TCP_NODELAY flag twice

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 06:48:32AM -0500, Frediano Ziglio wrote: > > > > On Wed, Feb 15, 2017 at 11:31:59AM +, Frediano Ziglio wrote: > > > TCP_NODELAY flag is set by default for all connection inside > > > reds.c so there's no need to set again for the single > > > client channel. > > > > Wo

Re: [Spice-devel] [PATCH spice-server 2/2] sound: Use memcpy instead of manually copy volume array

2017-02-15 Thread Frediano Ziglio
Ping > > Signed-off-by: Frediano Ziglio > --- > server/sound.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/server/sound.c b/server/sound.c > index 7c36174..b692646 100644 > --- a/server/sound.c > +++ b/server/sound.c > @@ -384,7 +384,6 @@ static int > snd_pla

[Spice-devel] [PATCH v2 2/6] qxl-wddm-dod: Option to provide frequency data to the OS

2017-02-15 Thread Yuri Benditovich
Concentrate filling of signal info in single procedure. Fill signal info with specific or default frequency data according to the global flag of VSync support. Note that the state of this flobal flag must be defined only on driver startup (based on OS version or any other information available at D

[Spice-devel] [PATCH v2 6/6] qxl-wddm-dod: Preparation for VSync activation

2017-02-15 Thread Yuri Benditovich
Prepare all the procedures needed for VSync feature but do not turn it on yet. Manual uncomment still required to enable it. Advanced users may enable VSync feature and report functional problem with it. The driver is expected to pass all the formal tests under HLK 1607 with device rev.3 and rev.4

[Spice-devel] [PATCH v2 0/6] Preparation to pass HLK tests

2017-02-15 Thread Yuri Benditovich
These patches address failures in Windows Hardware Lab Tests: Requirement for EDID data Requirement for refresh rate of 75/60 Hz Now the driver contains data in EDID format and reports it to the system (requires manual uncomment) In order to report refresh rate the driver reports support for VSy

[Spice-devel] [PATCH v2 1/6] qxl-wddm-dod: Return EDID data to the OS

2017-02-15 Thread Yuri Benditovich
Solves failure of HLK "Test for EDID requirements" EDID contains capabilities and manufacturer data of the emulated display device. Parsed EDID data presented in the source file. Signed-off-by: Yuri Benditovich --- qxldod/QxlDod.cpp | 106 -- 1

[Spice-devel] [PATCH v2 4/6] qxl-wddm-dod: Simplify interrupt handling for rev4 device

2017-02-15 Thread Yuri Benditovich
Do not clear interrupt mask upon interrupt. Instead clear pending interrupt status and write QXL_IO_UPDATE_IRQ register (this drops interrupt level). There are 3 advantages: 1. We do not need to wake the host to enable interrupt mask in DPC procedure (1 wake per interrupt instead of 2) 2. The drive

[Spice-devel] [PATCH v2 3/6] qxl-wddm-dod: Fix video modes enumeration

2017-02-15 Thread Yuri Benditovich
When the video mode is changed and then the driver disabled and enabled, it did not enumerate available video modes with lower resolution than current one. All modes starting from 1024x768 should be available regardless what is current resolution on driver startup Signed-off-by: Yuri Benditovich

[Spice-devel] [PATCH v2 5/6] qxl-wddm-dod: Timer-based VSync interrupt indication

2017-02-15 Thread Yuri Benditovich
In case the driver supports VSync control feature, it maintains timer for VSync interrupt indication. In further commits this timer will be started upon class driver request. The interrupt notification and related DPC do not access device registers. Signed-off-by: Yuri Benditovich --- qxldod/Qxl

Re: [Spice-devel] [PATCH spice-server 2/4] Do not set TCP_NODELAY flag twice

2017-02-15 Thread Frediano Ziglio
> > On Wed, Feb 15, 2017 at 11:31:59AM +, Frediano Ziglio wrote: > > TCP_NODELAY flag is set by default for all connection inside > > reds.c so there's no need to set again for the single > > client channel. > > Would have been nice to keep "Do not set not blocking flag twice" > together with

Re: [Spice-devel] [spice-server v2] gstreamer: Add #ifdef around zero_copy()

2017-02-15 Thread Pavel Grunt
On Wed, 2017-02-15 at 12:29 +0100, Christophe Fergeau wrote: > Since c3d237 "gstreamer: Avoid memory copy if strides are different" > is > only needed when zero copy is disabled. This moves the function > definition to an already existing #ifdef block. Ack, Pavel > --- > > Changes since v1: > - d

Re: [Spice-devel] [PATCH spice-server 1/4] Clear "msg" pointers after releasing

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 11:31:58AM +, Frediano Ziglio wrote: > Avoid possible dangling pointers. Acked-by: Christophe Fergeau > > Signed-off-by: Frediano Ziglio > --- > server/red-channel-client.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/server/red-channel-client.c b/s

Re: [Spice-devel] [spice-server v4 5/5] Change some spice_printerr() to spice_debug()

2017-02-15 Thread Frediano Ziglio
> > Some debug messages were using spice_printerr(), which should be limited > to actual errors. > > Signed-off-by: Christophe Fergeau > --- > server/inputs-channel.c | 4 ++-- > server/main-channel-client.c | 28 ++-- > server/main-channel.c| 10 +-

Re: [Spice-devel] [PATCH spice-server 2/4] Do not set TCP_NODELAY flag twice

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 11:31:59AM +, Frediano Ziglio wrote: > TCP_NODELAY flag is set by default for all connection inside > reds.c so there's no need to set again for the single > client channel. Would have been nice to keep "Do not set not blocking flag twice" together with this patch, and

Re: [Spice-devel] [PATCH spice-server 4/4] Make RedChannel::config_socket() optional

2017-02-15 Thread Christophe Fergeau
Acked-by: Christophe Fergeau Christophe On Wed, Feb 15, 2017 at 11:32:01AM +, Frediano Ziglio wrote: > Most channels don't need to do specific settings for the client socket > so avoid the need to do this setting making easier to setup the client > channnel. > > Some improvements and commi

Re: [Spice-devel] [PATCH spice-server 3/4] reds-stream: Introduce reds_stream_set_no_delay() helper

2017-02-15 Thread Frediano Ziglio
> > From: Christophe Fergeau > > The code to enable/disable on a TCP socket is duplicated in multiple > places in the code base, this commit replaces this duplicated code with > a helper in RedsStream. > --- > server/common-graphics-channel.c | 13 ++--- > server/red-channel-client.c

[Spice-devel] [spice-server v4 5/5] Change some spice_printerr() to spice_debug()

2017-02-15 Thread Christophe Fergeau
Some debug messages were using spice_printerr(), which should be limited to actual errors. Signed-off-by: Christophe Fergeau --- server/inputs-channel.c | 4 ++-- server/main-channel-client.c | 28 ++-- server/main-channel.c| 10 +- server/red-channe

[Spice-devel] [PATCH spice-server 2/4] Do not set TCP_NODELAY flag twice

2017-02-15 Thread Frediano Ziglio
TCP_NODELAY flag is set by default for all connection inside reds.c so there's no need to set again for the single client channel. Note that there are still some call to setsockopt to set this option but in this case the flag can reset the flag. Signed-off-by: Frediano Ziglio --- server/inputs-

[Spice-devel] [PATCH spice-server 3/4] reds-stream: Introduce reds_stream_set_no_delay() helper

2017-02-15 Thread Frediano Ziglio
From: Christophe Fergeau The code to enable/disable on a TCP socket is duplicated in multiple places in the code base, this commit replaces this duplicated code with a helper in RedsStream. --- server/common-graphics-channel.c | 13 ++--- server/red-channel-client.c | 17 ++-

[Spice-devel] [PATCH spice-server 1/4] Clear "msg" pointers after releasing

2017-02-15 Thread Frediano Ziglio
Avoid possible dangling pointers. Signed-off-by: Frediano Ziglio --- server/red-channel-client.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/red-channel-client.c b/server/red-channel-client.c index 9ab22e4..32db186 100644 --- a/server/red-channel-client.c +++ b/server/red-channe

[Spice-devel] [PATCH spice-server 4/4] Make RedChannel::config_socket() optional

2017-02-15 Thread Frediano Ziglio
Most channels don't need to do specific settings for the client socket so avoid the need to do this setting making easier to setup the client channnel. Some improvements and commit subject suggested by Christophe Fergeau. Signed-off-by: Frediano Ziglio --- server/inputs-channel.c | 6 -- se

[Spice-devel] [spice-server v2] gstreamer: Add #ifdef around zero_copy()

2017-02-15 Thread Christophe Fergeau
Since c3d237 "gstreamer: Avoid memory copy if strides are different" is only needed when zero copy is disabled. This moves the function definition to an already existing #ifdef block. --- Changes since v1: - do not add redundant #ifdef block server/gstreamer-encoder.c | 61 ++

[Spice-devel] [spice-server v4 2/5] reds: Close sockets when using spice_server_destroy()

2017-02-15 Thread Christophe Fergeau
Currently, the network sockets opened by reds_init_net() are not closed on destruction, in other words they are leaked. Signed-off-by: Christophe Fergeau Acked-by: Pavel Grunt --- server/reds.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/server/reds.c b/

[Spice-devel] [spice-server v4 4/5] test-vdagent: Make test case more useful

2017-02-15 Thread Christophe Fergeau
This switches the test to using the GTest API, and add several tests related to https://bugzilla.redhat.com/show_bug.cgi?id=1411194 This uses some API not available in glib 2.28, so this checks we have a new enough glib before building this test, and disables warnings when using too new glib API w

Re: [Spice-devel] [PATCH spice-server 4/4] Provide and reuse default implementation for config_socket

2017-02-15 Thread Frediano Ziglio
> > On Wed, Feb 15, 2017 at 06:02:00AM -0500, Frediano Ziglio wrote: > > > > > > On Tue, Feb 14, 2017 at 03:55:25PM +0100, Christophe Fergeau wrote: > > > > I would not provide any default implementation, and just change > > > > red_channel_config_socket to > > > > > > > > > > Thinking more abo

[Spice-devel] [spice-server v4 1/5] test: Add test_destroy()

2017-02-15 Thread Christophe Fergeau
This allows to chain several test cases by using test_new()/test_destroy(). Signed-off-by: Christophe Fergeau Acked-by: Pavel Grunt --- server/tests/test-display-base.c | 7 +++ server/tests/test-display-base.h | 1 + 2 files changed, 8 insertions(+) diff --git a/server/tests/test-display-

[Spice-devel] [spice-server v4 0/5] Better test-vdagent test case

2017-02-15 Thread Christophe Fergeau
Only changes since v3 is the removal of redundant initialization in patch 2/5, and an additional patch changing some spice_printerr() messages to spice_debug(). Christophe ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.fre

[Spice-devel] [spice-server v4 3/5] Use spice_debug rather than spice_info in library code

2017-02-15 Thread Christophe Fergeau
Using spice_info() gets in the way of tests using g_test_expect_message() as all the messages emitted using a non-debug log level must be listed as expected, otherwise we get a critical about an expected message not having been logged. Signed-off-by: Christophe Fergeau --- server/cursor-channel.

Re: [Spice-devel] [PATCH spice-server 4/4] Provide and reuse default implementation for config_socket

2017-02-15 Thread Christophe Fergeau
On Wed, Feb 15, 2017 at 06:02:00AM -0500, Frediano Ziglio wrote: > > > > On Tue, Feb 14, 2017 at 03:55:25PM +0100, Christophe Fergeau wrote: > > > I would not provide any default implementation, and just change > > > red_channel_config_socket to > > > > > > > Thinking more about this, I actually

Re: [Spice-devel] [PATCH spice-server 4/4] Provide and reuse default implementation for config_socket

2017-02-15 Thread Frediano Ziglio
> > On Tue, Feb 14, 2017 at 03:55:25PM +0100, Christophe Fergeau wrote: > > I would not provide any default implementation, and just change > > red_channel_config_socket to > > > > Thinking more about this, I actually would provide a default > implementation, and move the setsockopt/fnctl code f

Re: [Spice-devel] [vdagent-linux v8 4/4] vdagentd: Do endian swapping.

2017-02-15 Thread Victor Toso
On Wed, Feb 15, 2017 at 10:48:07AM +0100, Victor Toso wrote: > From: Michal Suchanek > > This allows running big endian and little endian guest side by side using > cut & paste between them. > > There is a general design idea that swapping should come as close to > virtio_read/virtio_write as poss

Re: [Spice-devel] [vdagent-linux v8 3/4] Adjust size checks

2017-02-15 Thread Victor Toso
On Wed, Feb 15, 2017 at 10:48:06AM +0100, Victor Toso wrote: > From: Christophe Fergeau 'vdagentd:' prefix here too? Just a small note, this was also wrong before "vdagentd: early return on bad message size" patch. Acked-by: Victor Toso > > --- > src/vdagentd/vdagentd.c | 4 ++-- > 1 file ch

Re: [Spice-devel] [vdagent-linux v8 2/4] Add missing size checks

2017-02-15 Thread Victor Toso
On Wed, Feb 15, 2017 at 10:48:05AM +0100, Victor Toso wrote: > From: Christophe Fergeau Maybe include vdagentd: prefix in the commit log? Acked-by: Victor Toso > > --- > src/vdagentd/vdagentd.c | 14 ++ > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/src/vdagent

Re: [Spice-devel] [vdagent-linux v8 1/4] vdagentd: early return on bad message size

2017-02-15 Thread Victor Toso
Hi, On Wed, Feb 15, 2017 at 10:48:04AM +0100, Victor Toso wrote: > From: Victor Toso > > The payload size for each message should be the size of the expected > struct or bigger when it contain arrays of no fixed size. > > This patch creates the vdagent_message_min_size[] static array with > the e

Re: [Spice-devel] [PATCH spice-server 4/4] Provide and reuse default implementation for config_socket

2017-02-15 Thread Christophe Fergeau
On Tue, Feb 14, 2017 at 03:55:25PM +0100, Christophe Fergeau wrote: > I would not provide any default implementation, and just change > red_channel_config_socket to > Thinking more about this, I actually would provide a default implementation, and move the setsockopt/fnctl code from reds_init_cli

[Spice-devel] [vdagent-linux v8 4/4] vdagentd: Do endian swapping.

2017-02-15 Thread Victor Toso
From: Michal Suchanek This allows running big endian and little endian guest side by side using cut & paste between them. There is a general design idea that swapping should come as close to virtio_read/virtio_write as possible. In particular, the protocol between vdagent and vdagentd is guest-s

[Spice-devel] [vdagent-linux v8 0/4]

2017-02-15 Thread Victor Toso
From: Victor Toso Hi there! This patches were also pending for some time and the patches were distributed in two mail threads and for that reason I'm puting them here in (another) thread :) This first three patches were attached to Christophe's email [0], the last patch is the v7 from Michal [1

[Spice-devel] [vdagent-linux v8 2/4] Add missing size checks

2017-02-15 Thread Victor Toso
From: Christophe Fergeau --- src/vdagentd/vdagentd.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c index ea8482b..8d43700 100644 --- a/src/vdagentd/vdagentd.c +++ b/src/vdagentd/vdagentd.c @@ -391,6 +391,8 @@

[Spice-devel] [vdagent-linux v8 1/4] vdagentd: early return on bad message size

2017-02-15 Thread Victor Toso
From: Victor Toso The payload size for each message should be the size of the expected struct or bigger when it contain arrays of no fixed size. This patch creates the vdagent_message_min_size[] static array with the expected size for each message so we can check on virtio_port_read_complete().

[Spice-devel] [vdagent-linux v8 3/4] Adjust size checks

2017-02-15 Thread Victor Toso
From: Christophe Fergeau --- src/vdagentd/vdagentd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c index 8d43700..2329489 100644 --- a/src/vdagentd/vdagentd.c +++ b/src/vdagentd/vdagentd.c @@ -395,8 +395,6 @@ static gboo

Re: [Spice-devel] [spice-server 2/2] gstreamer: Add #ifdef around zero_copy()

2017-02-15 Thread Pavel Grunt
Hi, On Wed, 2017-02-15 at 09:39 +0100, Christophe Fergeau wrote: > Since c3d237 "gstreamer: Avoid memory copy if strides are different" > is > only needed when zero copy is disabled. > --- >  server/gstreamer-encoder.c | 2 ++ >  1 file changed, 2 insertions(+) > > diff --git a/server/gstreamer-en

Re: [Spice-devel] [spice-server 1/2] gstreamer: Remove unused function

2017-02-15 Thread Pavel Grunt
On Wed, 2017-02-15 at 09:39 +0100, Christophe Fergeau wrote: > rate_control_is_active() is static and not used in gstreamer- > encoder.c Yeah, not needed since 97fcad82eb7d1f3bbd8d1163801b5a3db92944c2 Acked-by: Pavel Grunt > --- >  server/gstreamer-encoder.c | 5 - >  1 file changed, 5 delet

[Spice-devel] [spice-server 2/2] gstreamer: Add #ifdef around zero_copy()

2017-02-15 Thread Christophe Fergeau
Since c3d237 "gstreamer: Avoid memory copy if strides are different" is only needed when zero copy is disabled. --- server/gstreamer-encoder.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c index dd2926d..bb41425 100644 --- a/server/gs

[Spice-devel] [spice-server 1/2] gstreamer: Remove unused function

2017-02-15 Thread Christophe Fergeau
rate_control_is_active() is static and not used in gstreamer-encoder.c --- server/gstreamer-encoder.c | 5 - 1 file changed, 5 deletions(-) diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c index 5c60a64..dd2926d 100644 --- a/server/gstreamer-encoder.c +++ b/server/gstreame

Re: [Spice-devel] [spice-server v2 14/14] rcc: Consistently name RedChannelClient 'rcc'

2017-02-15 Thread Frediano Ziglio
> > On Tue, Feb 14, 2017 at 12:14:56PM -0600, Jonathon Jongsma wrote: > > To be honest, I could go either way on this one. I like the > > consistency, but I don't like the fact that it makes things like "git > > blame" less useful. I'll leave the decision to others ;) > > I've pushed the series w

Re: [Spice-devel] [PATCH spice-server] smartcard: Remove a "rename" function

2017-02-15 Thread Christophe Fergeau
Acked-by: Christophe Fergeau On Fri, Feb 10, 2017 at 02:30:57PM +, Frediano Ziglio wrote: > smartcard_channel_client_pipe_add_push was just calling > red_channel_client_pipe_add_push without any cast or other > changes. > > Signed-off-by: Frediano Ziglio > --- > server/smartcard.c | 15 +++