Re: [Spice-devel] [PATCH spice-server 3/3] common-graphics-channel: Use manual flushing on stream to decrease packet fragmentation

ent_watch_update_mask(rcc, SPICE_WATCH_EVENT_READ); > +/* channel has no pending data to send so now we can flush data in > + * order to avoid data stall into buffers in case of manual > + * flushing > + */ > +red_stream_flush(rcc->

Re: [Spice-devel] [PATCH spice-server v2] ci: Workaround bug in Valgrind detecting memcpy instead of memmove

Acked-by: Christophe Fergeau <cferg...@redhat.com> On Tue, Apr 17, 2018 at 11:19:16AM +0100, Frediano Ziglio wrote: > Due to a bug in current packaged Valgrind in the CI (1:3.13.0-13.fc27) > check-valgrind is failing with: > > ==17986== Source and destination overlap in m

Re: [Spice-devel] [spice-server 05/10] qxl: Rename 'qxl' to 'qxl_cmd' in red_get_cursor_cmd()

hese if you prefer. > > > > Christophe > > > > I would personally keep red and qxl and add a qxl_instance. Fwiw, here is what renaming everything would look like. Quite some churn indeed :( From 22a4eb242a7b5da0c37a5ebf9aaf8156be423785 Mon Sep 17 00:00:00 2001 From: Chri

Re: [Spice-devel] [PATCH spice-server] ci: Workaround bug in Valgrind detecting memcpy instead of memmove

On Mon, Apr 16, 2018 at 02:07:57PM -0400, Frediano Ziglio wrote: > > > > On Mon, Apr 16, 2018 at 05:02:33PM +0100, Frediano Ziglio wrote: > > > Due to a bug in current packaged Valgrind check-valgrind is failing > > > with: > > > > > > ==17986== Source and destination overlap in

Re: [Spice-devel] [PATCH spice-server] ci: Workaround bug in Valgrind detecting memcpy instead of memmove

On Mon, Apr 16, 2018 at 05:02:33PM +0100, Frediano Ziglio wrote: > Due to a bug in current packaged Valgrind check-valgrind is failing > with: > > ==17986== Source and destination overlap in memcpy_chk(0x72c060, 0x72c068, 33) > ==17986==at 0x4C344F0: __memcpy_chk (vg_replace_strmem.c:1581) >

[Spice-devel] [spice-server] Slight simplification of red_channel_client_push() logic

Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-channel-client.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/red-channel-client.c b/server/red-channel-client.c index f154c5c63..d46c299c2 100644 --- a/server/red-channel-client.c

Re: [Spice-devel] [spice-server 04/10] qxl: Fix guest resources release in red_put_drawable()

On Mon, Apr 16, 2018 at 07:25:19AM -0400, Frediano Ziglio wrote: > > > > At the moment, we'll unconditionally release the guest QXL resources in > > red_put_drawable() even if red_get_drawable() failed and did not > > initialize drawable->release_info_ext properly. > > This commit checks the

Re: [Spice-devel] [spice-server 05/10] qxl: Rename 'qxl' to 'qxl_cmd' in red_get_cursor_cmd()

On Mon, Apr 16, 2018 at 06:58:18AM -0400, Frediano Ziglio wrote: > Don't like this, lot of structures in this file use "qxl", for coherence > I would change all or nothing but changing all would mean a lot of changes > with not much value Imo 'red' and 'qxl' are not very good names, I'd prefer to

[Spice-devel] [spice-server 08/10] qxl: Release QXL resource in red_put_message

--- server/red-parse-qxl.c | 6 -- server/red-parse-qxl.h | 3 ++- server/red-worker.c| 3 +-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index 5408cfa5a..d7ae53804 100644 --- a/server/red-parse-qxl.c +++

[Spice-devel] [spice-server 10/10] qxl: Release QXL resources in red_put_update_cmd

--- server/red-parse-qxl.c | 10 +++--- server/red-parse-qxl.h | 3 ++- server/red-worker.c| 3 +-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index d4a7decd3..313cd1bfb 100644 --- a/server/red-parse-qxl.c +++

[Spice-devel] [spice-server 03/10] qxl: Make red_{get, put}_drawable static

Rather than needing to call red_drawable_new() and then initialize it with red_get_drawable(), we can improve slightly red_drawable new so that red_drawable_{new,ref,unref} is all which is used by code out of red-parse-qxl.c. --- server/red-parse-qxl.c | 17 -

[Spice-devel] [spice-server 06/10] qxl: Add red_cursor_cmd_new and red_cursor_cmd_free helpers

Currently, the cursor channel is allocating RedCursorCmd instances itself, and then calling into red-parse-qxl.h to initialize it, and doing something similar when releasing the data. This commit moves this common code to red-parse-qxl.[ch] --- server/cursor-channel.c | 6 +-

[Spice-devel] [spice-server 00/10] qxl: Move more guest resource release to red-parse-qxl

Hey, Currently, after parsing a QXL command through red-parse-qxl, the code which got the command has to tell red-parse-qxl when it no longer needs the command, but also to remember to release the command QXL resources itself. This series moves this 'release resource' logic to red-parse-qxl.

[Spice-devel] [spice-server 04/10] qxl: Fix guest resources release in red_put_drawable()

At the moment, we'll unconditionally release the guest QXL resources in red_put_drawable() even if red_get_drawable() failed and did not initialize drawable->release_info_ext properly. This commit checks the QXLReleaseInfo in release_info_ext is non-0 before attempting to release it. ---

[Spice-devel] [spice-server 09/10] qxl: Rename 'qxl' to 'qxl_cmd' in red_get_update_cmd

This is in preparation for next commit. --- server/red-parse-qxl.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index d7ae53804..d4a7decd3 100644 --- a/server/red-parse-qxl.c +++ b/server/red-parse-qxl.c @@

[Spice-devel] [spice-server 05/10] qxl: Rename 'qxl' to 'qxl_cmd' in red_get_cursor_cmd()

This is in preparation for next commit --- server/red-parse-qxl.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index 80746ecbb..c08d18160 100644 --- a/server/red-parse-qxl.c +++ b/server/red-parse-qxl.c

[Spice-devel] [spice-server 01/10] qxl: Remove red_put_blend()

SpiceBlend is a typedef to SpiceCopy, and red_put_blend() and red_put_copy() are identical, so we can add a #define red_put_blend red_put_copy similar to the one we already have for red_get_blend. --- server/red-parse-qxl.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git

[Spice-devel] [spice-server 07/10] qxl: Rename 'qxl' to 'qxl_cmd' in red_get_message

This is in preparation for next commit. --- server/red-parse-qxl.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index eb80ba81b..5408cfa5a 100644 --- a/server/red-parse-qxl.c +++ b/server/red-parse-qxl.c @@

[Spice-devel] [spice-server 02/10] qxl: Move red_drawable_unref/red_drawable_new

RedDrawable really is a RedDrawCmd which is parsed by red-parse-qxl.h This commit moves them close to the other functions creating/unref'ing QXL commands parsed by red-parse-qxl.h --- server/red-parse-qxl.c | 21 + server/red-parse-qxl.h | 2 ++ server/red-worker.c| 20

Re: [Spice-devel] [PATCH spice-gtk 3/4] gio-coroutine: Remove only assigned self field

Alternatively, this could come first in the series, with the nice side-effect of fixing that GSource inheritance issue that you mentioned ;) With the order that you chose, this allows to make the fix more obvious, and to better document why it was not a problem (by luck). On Fri, Apr 13, 2018 at

Re: [Spice-devel] [PATCH spice-gtk 0/4] Miscellaneous small patches

For the series Acked-by: Christophe Fergeau <cferg...@redhat.com> but see my comment on 1/4 On Fri, Apr 13, 2018 at 03:49:15PM +0100, Frediano Ziglio wrote: > Frediano Ziglio (4): > gio-coroutine: Fix C inheritance > gio-coroutine: Make waitFuncs static > gio-coro

Re: [Spice-devel] [PATCH spice-gtk 1/4] gio-coroutine: Fix C inheritance

pice-gtk set GConditiionWaitSource::self. So this ends up all working even though this is wrong. Acked-by: Christophe Fergeau <cferg...@redhat.com> but I'd add more details to the log Christophe > > Signed-off-by: Frediano Ziglio <fzig...@redhat.com> > --- > src/gio-coroutine.c | 2 +-

Re: [Spice-devel] [spice-server 2/2] cursor: Rename cursor_marshall to red_marshall_cursor_command

On Thu, Apr 12, 2018 at 11:09:39AM +0200, Christophe Fergeau wrote: > On Wed, Apr 11, 2018 at 01:16:57PM -0400, Frediano Ziglio wrote: > > > > > > The name is more consistent with red_marshall_cursor_init. > > > --- > > > server/cursor-channel.c | 8 -

Re: [Spice-devel] [PATCH spice-server 3/3] common-graphics-channel: use manual flushing on stream to decrease packet fragmentation

On Tue, Jan 16, 2018 at 02:18:15PM +, Frediano Ziglio wrote: > In order to use new TCP_CORK feature disable auto flush. 'the new TCP_CORK feature, disable auto flush' Might be worth explaining in the commit log why you disable auto_flush only for RedCommonGraphicsChannel. > > Signed-off-by:

Re: [Spice-devel] [PATCH spice-server 2/3] stream: implements flush using TCP_CORK

am, const void > *in_buf, size_t n) > > bool red_stream_set_auto_flush(RedStream *s, bool enable) > { > -return enable; > +if (s->priv->use_cork == !enable) { Might be slightly more readable if 'enable' is renamed to 'auto_flush' Acked-by: Christophe Fergeau <cferg

Re: [Spice-devel] [PATCH spice-server 1/3] stream: implement interface for manual flush

written. This is the default. > + * If not set you should call red_stream_flush to force 'If not set, you should ...' > + * data to be sent. Failing to call red_stream_flush on a > + * manual flush stream could lead to latency. > + * Disabling auto flush can fail while enabling cannot. &

Re: [Spice-devel] [spice-server 1/2] cursor: Delay release of QXL guest cursor resources

> > > This version of the fix is based on a suggestion from Frediano Ziglio. > > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1540919 > > > > > > > > Signed-off-by: Christophe Fergeau <cferg...@redhat.com> > > > > --- > &

Re: [Spice-devel] [spice-server 2/2] cursor: Rename cursor_marshall to red_marshall_cursor_command

On Wed, Apr 11, 2018 at 01:16:57PM -0400, Frediano Ziglio wrote: > > > > The name is more consistent with red_marshall_cursor_init. > > --- > > server/cursor-channel.c | 8 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > Why command (so red_marshall_cursor) ? > The item is

Re: [Spice-devel] [spice-server 1/2] cursor: Delay release of QXL guest cursor resources

the pre-1c6e7cf7 behaviour to avoid a regression > > while QEMU is being fixed. > > > > This version of the fix is based on a suggestion from Frediano Ziglio. > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1540919 > > > > Signed-off-by: Christophe F

[Spice-devel] [spice-server 1/2] cursor: Delay release of QXL guest cursor resources

io. https://bugzilla.redhat.com/show_bug.cgi?id=1540919 Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 3 +++ server/red-parse-qxl.h | 1 + server/red-worker.c| 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/server/red-parse-qx

[Spice-devel] [spice-server 2/2] cursor: Rename cursor_marshall to red_marshall_cursor_command

The name is more consistent with red_marshall_cursor_init. --- server/cursor-channel.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/cursor-channel.c b/server/cursor-channel.c index 522261e3f..a549c5fe0 100644 --- a/server/cursor-channel.c +++

Re: [Spice-devel] [spice-server] cursor: Delay release of QXL guest cursor resources

hristophe > > > > > > > When do you plan to remove this patch from spice-server? > > > > > On Thu, Apr 05, 2018 at 10:36:27AM +0200, Christophe Fergeau wrote: > > > > There's an implicit API/ABI contract between QEMU and SPICE that SPICE > > > &

Re: [Spice-devel] [spice-gtk v1 2/2] channel-usbredir: Fix crash on channel-up

On Tue, Apr 10, 2018 at 02:17:45PM +0200, Victor Toso wrote: > Hi, > > On Tue, Apr 10, 2018 at 01:37:05PM +0200, Victor Toso wrote: > > Hi, > > > > On Tue, Apr 10, 2018 at 12:41:16PM +0200, Christophe Fergeau wrote: > > > On Fri, Apr 06, 2018 at 09:59:44AM +0

Re: [Spice-devel] [PATCH spice-server] red-parse-qxl: Remove unused device_data field

x 4a576ca0..ce7d8b05 100644 > --- a/server/red-parse-qxl.h > +++ b/server/red-parse-qxl.h > @@ -113,7 +113,6 @@ typedef struct RedCursorCmd { > } trail; > SpicePoint16 position; > } u; > -uint8_t *device_data; > } RedCursorCmd; Acked-by: Ch

Re: [Spice-devel] [spice-server] cursor: Delay release of QXL guest cursor resources

at once or not, spice-server will have the same behaviour as in the 0.12 branch. Christophe On Thu, Apr 05, 2018 at 10:36:27AM +0200, Christophe Fergeau wrote: > There's an implicit API/ABI contract between QEMU and SPICE that SPICE > will keep the guest QXL resources alive as long as QE

Re: [Spice-devel] [spice-gtk v1 2/2] channel-usbredir: Fix crash on channel-up

On Fri, Apr 06, 2018 at 09:59:44AM +0200, Victor Toso wrote: > From: Victor Toso > > By adding a guard to not handle channel-up on SpiceUsbredirChannel in > case struct usbredirhost wasn't initialized yet. Same guard is in > place for the generic usbredir_handle_msg()

Re: [Spice-devel] [PATCH spice-streaming-agent] ci: Add make package

On Mon, Apr 09, 2018 at 10:53:34AM +0300, Snir Sheriber wrote: > make is no longer installed by default in fedora:latest Sure, Acked-by: Christophe Fergeau <cferg...@redhat.com> though make/gcc-c++ should also come from 'dnf builddep spice-streaming-agent' if this is no longer

Re: [Spice-devel] [PATCH spice-streaming-agent] Use pkg-config to find jpeg library if available

not found])) +]) AC_SUBST(JPEG_LIBS) dnl === Acked-by: Christophe Fergeau <cferg...@redhat.com> On Fri, Apr 06, 2018 at 07:41:22AM +0100, Frediano Ziglio wrote: > Newer libraries provide pkg-config module for libjpeg

Re: [Spice-devel] [PATCH spice-streaming-agent] Revert "build: Use pkgconfig to detect libjpeg"

Ah I thought this was done already, but yeah, Acked-by: Christophe Fergeau <cferg...@redhat.com> On Thu, Apr 05, 2018 at 06:03:15PM +0100, Frediano Ziglio wrote: > This reverts commit 5240f212ed364d5139f30810b14884f8e2c03535. > RHEL 7 does not provide a pkg-config module for libjpeg.

Re: [Spice-devel] [PATCH 1/2] Ensure that plugins cannot bypass version check

On Fri, Mar 23, 2018 at 01:05:23PM +0100, Christophe de Dinechin wrote: > 3. Major.minor numbering scheme > > The major.minor numbering scheme initially selected makes it harder > to fixes cases where an incompatibility was detected after release. I'm still of the opinion that we should break

Re: [Spice-devel] [spice-server] cursor: Consistently use g_memdup() for cursor data

On Thu, Apr 05, 2018 at 10:55:13AM +0200, Christophe Fergeau wrote: > On Thu, Apr 05, 2018 at 04:43:17AM -0400, Frediano Ziglio wrote: > > > > > > Currently, red-parse-qxl.c open codes g_memdup() when it needs to > > > > "open codes" ? > &g

Re: [Spice-devel] [spice-server] cursor: Consistently use g_memdup() for cursor data

On Thu, Apr 05, 2018 at 04:43:17AM -0400, Frediano Ziglio wrote: > > > > Currently, red-parse-qxl.c open codes g_memdup() when it needs to > > "open codes" ? > well, currently uses g_malloc+memcpy which can be rewritten with > g_memdup. Yes, doing this can be seen as reimplementing g_memdup,

[Spice-devel] [spice-server] cursor: Delay release of QXL guest cursor resources

proper fix would be in QEMU so that spice-server does not need to have that kind of knowledge regarding QEMU internal implementation, this commit reverts to the pre-1c6e7cf7 behaviour to avoid a regression while QEMU is being fixed. https://bugzilla.redhat.com/show_bug.cgi?id=1540919 Signed-off-by:

[Spice-devel] [spice-server] cursor: Consistently use g_memdup() for cursor data

Currently, red-parse-qxl.c open codes g_memdup() when it needs to duplicate the cursor data, while red-stream-device.c does this using spice_memdup(). This commit makes use of g_memdup() in both cases so that this memory is consistently allocated through glib. Signed-off-by: Christophe Fergeau

Re: [Spice-devel] [PATCH 1/2] Ensure that plugins cannot bypass version check

On Thu, Mar 29, 2018 at 10:36:06AM +0200, Christophe de Dinechin wrote: > > > > On 29 Mar 2018, at 10:05, Christophe Fergeau <cferg...@redhat.com> wrote: > > > > On Wed, Mar 28, 2018 at 05:54:21PM +0200, Christophe de Dinechin wrote: > >>> With t

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

On Wed, Mar 28, 2018 at 05:35:35PM +0200, Christophe de Dinechin wrote: > > On 28 Mar 2018, at 17:04, Christophe Fergeau <cferg...@redhat.com> wrote: > > The part I'm missing is how you extract this limited number of full > > strings that we'll need to translate into

Re: [Spice-devel] [PATCH 1/2] Ensure that plugins cannot bypass version check

On Wed, Mar 28, 2018 at 05:54:21PM +0200, Christophe de Dinechin wrote: > > With that said, I find the current "ODR" portion of the commit log > > misleading, it's easy to get the impression that after this change, we > > won't have any ODR problem anymore (this is what I initially thought!). > >

Re: [Spice-devel] [PATCH 0/2] Make plugin version checking more robust

On Wed, Mar 28, 2018 at 06:06:19PM +0200, Christophe de Dinechin wrote: > > If my task is to "move version check to the agent", do I _have_ to change > > the semantics of the version check? No. > > Of course you have to. There is no “PluginVersionIsCompatible” > anymore, etc, so the version

Re: [Spice-devel] [PATCH 0/2] Make plugin version checking more robust

On Wed, Mar 28, 2018 at 10:47:27AM +0200, Christophe de Dinechin wrote: > > What matters in my opinion is that we decide to break it, once we've > > made that decision, the number of commits which are going to make ABI > > breaking changes is less important. > > Introducing “subtle” changes such

Re: [Spice-devel] [PATCH 1/2] Ensure that plugins cannot bypass version check

On Wed, Mar 28, 2018 at 11:10:36AM +0200, Christophe de Dinechin wrote: > > > > On 26 Mar 2018, at 19:06, Christophe Fergeau <cferg...@redhat.com> wrote: > > > > On Fri, Mar 23, 2018 at 01:05:23PM +0100, Christophe de Dinechin wrote: > >> 2. ODR-

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

On Wed, Mar 28, 2018 at 02:44:56PM +0200, Christophe de Dinechin wrote: > In my proposal, that message is built in format_message with something like: > > int written = snprintf(buffer, size, "%s writing %s for file ‘%s'", > what(), operation, filename.c_str()); > return

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

> Do you have something against requiring string constants in English for our > error messages? > > Is it so difficult to understand that if I have: > > WriteError(“can’t write”, “header”, filename) > WriteError(“can’t write”, “boson”, filename) > > I can localize all the

Re: [Spice-devel] [PATCH spice-server 1/2] Add support for building with meson

On Tue, Mar 27, 2018 at 09:57:35AM -0300, Eduardo Lima (Etrunko) wrote: > On 27/03/18 09:51, Christophe Fergeau wrote: > > On Tue, Mar 27, 2018 at 08:53:06AM -0300, Eduardo Lima (Etrunko) wrote: > >> No, I think it should work, but this .wrap file is useful the first tim

Re: [Spice-devel] [PATCH spice-server 1/2] Add support for building with meson

On Tue, Mar 27, 2018 at 08:53:06AM -0300, Eduardo Lima (Etrunko) wrote: > No, I think it should work, but this .wrap file is useful the first time > user clones the repository, it will fetch spice-common automatically. Hmm, but is this 'subproject' command going to handle git submodule updates

Re: [Spice-devel] [PATCH spice-server 1/2] Add support for building with meson

Couple of notes for people trying this, On Thu, Mar 22, 2018 at 02:18:20PM -0300, Eduardo Lima (Etrunko) wrote: > diff --git a/meson.build b/meson.build > new file mode 100644 > index ..f373e662 > --- /dev/null > +++ b/meson.build > @@ -0,0 +1,175 @@ > +# > +# project definition > +# > +#

Re: [Spice-devel] [PATCH 0/2] Make plugin version checking more robust

On Tue, Mar 27, 2018 at 10:28:24AM +0200, Christophe de Dinechin wrote: > > > > On 27 Mar 2018, at 10:12, Christophe Fergeau <cferg...@redhat.com> wrote: > > > > With the right patch attached this time.. ;) I've only compile-tested > > this as th

Re: [Spice-devel] [PATCH 0/2] Make plugin version checking more robust

With the right patch attached this time.. ;) I've only compile-tested this as this really is just a proof of concept at this point. Christophe On Mon, Mar 26, 2018 at 06:47:08PM +0200, Christophe Fergeau wrote: > On Mon, Mar 26, 2018 at 11:27:19AM +0200, Christophe de Dinechin wr

Re: [Spice-devel] [PATCH 1/2] Ensure that plugins cannot bypass version check

On Fri, Mar 23, 2018 at 01:05:23PM +0100, Christophe de Dinechin wrote: > 2. ODR-related problems > > The C++ One Definition Rule (ODR) states that all translation units > must see the same definitions. In the current code, when we call > Agent::PluginVersionIsCompatible from the plugin, it is an

Re: [Spice-devel] [PATCH 0/2] Make plugin version checking more robust

On Mon, Mar 26, 2018 at 11:27:19AM +0200, Christophe de Dinechin wrote: > > > > On 26 Mar 2018, at 11:25, Christophe Fergeau <cferg...@redhat.com> wrote: > > > > Hey, > > > > On Fri, Mar 23, 2018 at 01:05:22PM +0100, Christophe de Dinechin wrote

Re: [Spice-devel] [PATCH 0/2] Make plugin version checking more robust

Hey, On Fri, Mar 23, 2018 at 01:05:22PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > Ensure that plugin version checking cannot be bypassed. > Implement version checking without violating the C++ ODR. > Improve ABI version numbering when

Re: [Spice-devel] [PATCH spice-server v2] stream-channel: Send the full frame in a single message

Hey, For some reason, I was finding this quite hard to review until I split it in some preparation patch with no behaviour change followed by the actual changes, see the attachement for how I split things, though some small details might be wrong. On Wed, Mar 07, 2018 at 12:01:49PM +,

Re: [Spice-devel] [PATCH spice-server 1/2] Change ENABLE_EXTRA_CHECKS statements to #ifdef

On Tue, Mar 13, 2018 at 10:37:46AM -0300, Eduardo Lima (Etrunko) wrote: > On 13/03/18 04:21, Frediano Ziglio wrote: > >> > >> This patch makes it clear that this is a configure switch and not a > >> variable defined somewhere else in the code. > >> > > > > The code is intended that way to make

[Spice-devel] [spice-gtk] build: Remove SPICE_GTK_LOCALEDIR

This became obsolete after 93b213e 'spicy-*: Remove translation support' Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- configure.ac| 3 --- src/Makefile.am | 1 - 2 files changed, 4 deletions(-) diff --git a/configure.ac b/configure.ac index 2a14055b..17799c1a

Re: [Spice-devel] [PATCH spice-server 1/3] ci: Add glib-networking package

for the series, Acked-by: Christophe Fergeau <cferg...@redhat.com> On Tue, Mar 13, 2018 at 10:12:56PM +, Frediano Ziglio wrote: > This is required by the new test-listen test to connect using > TLS. > > Signed-off-by: Frediano Ziglio <fzig...@redhat.com> > --- &g

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

) which differs to distinguish > between “xfree86” and “evdev” when the there is no keycode name. extra "the" here, apart from that, looks good to me, thanks a lot for fixing this! I'll push this in spice-gtk this week. Acked-by: Christophe Fergeau <cferg...@redhat.com> Christophe &

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

-server instance. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/reds.c | 34 +- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/server/reds.c b/server/reds.c index 73c9ec20f..415d570fa 100644 --- a/server/reds.c +++ b/server/

[Spice-devel] [spice-server v2 2/7] build: Bump glib version

From spice-gtk b312ca08 commit: "At the moment: - Fedora 26 has 2.52 - Fedora 25 has 2.50 - Fedora 24 has 2.48 - CentOS 7 has 2.46 - Debian 9 has 2.50" RHEL6 only have 2.28, but glib 2.32 is only used in a test case at the moment. Signed-off-by: Christophe Ferg

[Spice-devel] [spice-server v2 6/7] test-listen: Add TLS test

Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/tests/test-listen.c | 106 +++-- 1 file changed, 103 insertions(+), 3 deletions(-) diff --git a/server/tests/test-listen.c b/server/tests/test-listen.c index 7843c9b67..77a185abf

[Spice-devel] [spice-server v2 4/7] test-listen: Add test case for port/address configuration

sockets, TLS sockets, ... Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/tests/Makefile.am | 1 + server/tests/test-listen.c | 191 + 2 files changed, 192 insertions(+) create mode 100644 server/tests/test-listen.c diff --git a/

[Spice-devel] [spice-server v2 3/7] tests: basic-event-loop: Silence debug message

There is currently a debug printf which is always shown when a mainloop event is triggered. This is unlikely to be useful unless one is debugging the event loop code. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/tests/basic-event-loop.c | 2 +- 1 file changed, 1 ins

[Spice-devel] [spice-server v2 5/7] test-listen: Add event loop helpers

These factor a bit of common code, and more importantly, help with freeing all event loop related data at the end of each test. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/tests/test-listen.c | 118 +++-- 1 file chang

[Spice-devel] [spice-server v2 7/7] test-listen: Add Unix socket test

Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- configure.ac | 3 +++ server/tests/Makefile.am | 4 +++- server/tests/test-listen.c | 38 +++--- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/configur

[Spice-devel] [spice-server v2 0/7] Add test-listen test case

While working on some bug/new feature for SPICE, I added a test case for our spice_server_set_port/_set_tls/... API. While the work I did this for still needs some work, this test case should be good enough on its own. Christophe Christophe Fergeau (7): reds: Close sockets when failing

Re: [Spice-devel] [PATCH spice-server] red-record-qxl: fix clang warning

Ok, Acked-by: Christophe Fergeau <cferg...@redhat.com> On Mon, Mar 12, 2018 at 12:33:47PM +, Frediano Ziglio wrote: > Fix clang warning: > > red-record-qxl.c:893:13: error: variable 'fd_in' is used uninitialized > whenever 'if' condition is false [-Werror,-Wsomet

Re: [Spice-devel] [spice-server 4/8] test-listen: Add test case port/address configuration

On Fri, Mar 09, 2018 at 06:36:24AM -0500, Frediano Ziglio wrote: > > +static void test_connect_plain(void) > > +{ > > +GThread *thread; > > +int result; > > + > > +/* server */ > > +SpiceServer *server = spice_server_new(); > > +core = basic_event_loop_init(); > > +

Re: [Spice-devel] [PATCH spice-protocol 2/2] macros: Use Visual C++ built-ins for byte swapping if available

ing that macro. Not really a problem to have these anyway, Acked-by: Christophe Fergeau <cferg...@redhat.com> signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-protocol 1/2] macros: Use GCC built-ins to swap bytes

, very happy to see that ! Acked-by: Christophe Fergeau <cferg...@redhat.com> signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

On Thu, Mar 08, 2018 at 03:54:56PM +0200, Uri Lublin wrote: > 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 go

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

-device.c:182:1: error: the frame size of 32992 bytes is larger than > 20460 bytes [-Werror=frame-larger-than=] > > Allow larger stack on "makecheck" to avoid this issue. Acked-by: Christophe Fergeau <cferg...@redhat.com> > > Signed-off-by: Frediano Ziglio &l

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] [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] [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

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 <cferg...@redhat.com> though maybe people more familiar with the streaming channel will want to take a look too. Christophe On Wed, Mar 07, 2018 at 08:2

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

ee > <http://www.gnu.org/licenses/>. > +*/ > + > +#ifndef STREAM_DEVICE_H > +#define STREAM_DEVICE_H > + > +#include "char-device.h" > + > +G_BEGIN_DECLS > + > +/** > + * This type it's a RedCharDevice class. > + * A pointer to StreamDevice can be convert

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] [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 1/8] reds: Close sockets when failing to watch them

On Wed, Mar 07, 2018 at 12:01:48PM +0200, Uri Lublin wrote: > On 03/06/2018 03:53 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 open even if we > > are not go

Re: [Spice-devel] [spice-server 2/8] build: Bump glib version

On Wed, Mar 07, 2018 at 05:13:57AM -0500, Frediano Ziglio wrote: > > > > Hey, > > > > On Wed, Mar 07, 2018 at 01:56:20AM -0500, Frediano Ziglio wrote: > > > > > > > > From spice-gtk b312ca08 commit: > > > > "At the moment: > > > > - Fedora 26 has 2.52 > > > > - Fedora 25 has 2.50 >

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

On Wed, Mar 07, 2018 at 11:08:58AM +, Frediano Ziglio wrote: > Assuming the packing of structure can be a portability issue. > Currently there are no real-world ABI that would break these structure > however to be safe force a 4 byte packing. I don't think this is a "4 byte packing",

Re: [Spice-devel] [PATCH spice-streaming-agent v5 2/2] Implement handling of error messages from the server

On Wed, Mar 07, 2018 at 03:01:43PM +0100, Christophe de Dinechin wrote: > Also, on an arch like ARM. the packed attribute gives the compiler the > opportunity to use special mis-aligned load and store instructions. So > it’s a good thing, isn’t it? Iirc the compiler was not doing that much magic

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

On Wed, Mar 07, 2018 at 12:28:57PM +0100, Christophe de Dinechin wrote: > > This behaviour seems inconsistent with std::runtime_error(const char *) > > which as far as I can tell does not have this lifetime expectation. > > You are correct, but please note that std::runtime_error takes a string

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

On Wed, Mar 07, 2018 at 11:28:41AM +0100, Christophe de Dinechin wrote: > > > > On 6 Mar 2018, at 17:15, Christophe Fergeau <cferg...@redhat.com> wrote: > > > > On Thu, Mar 01, 2018 at 09:01:37PM +0100, Christophe de Dinechin wrote: > >>> On 28 Feb

Re: [Spice-devel] [spice-server 2/8] build: Bump glib version

Hey, On Wed, Mar 07, 2018 at 01:56:20AM -0500, Frediano Ziglio wrote: > > > > From spice-gtk b312ca08 commit: > > "At the moment: > > - Fedora 26 has 2.52 > > - Fedora 25 has 2.50 > > - Fedora 24 has 2.48 > > - CentOS 7 has 2.46 > > - Debian 9 has 2.50" > > > > RHEL6

Re: [Spice-devel] [spice-streaming-agent v2] build: Add --enable-tests

On Wed, Mar 07, 2018 at 04:26:50AM -0500, Frediano Ziglio wrote: > > > > On Thu, 2018-03-01 at 18:28 +0100, Christophe Fergeau wrote: > > > Tests require 'catch' to be installed, one might want to disable them if > > > catch is not available. This patch

Re: [Spice-devel] [PATCH spice-server] build: Rename spice-server-enums.tmpl.[ch] to spice-server-enums.[ch].tmpl

Hey, On Tue, Mar 06, 2018 at 03:00:30PM -0300, Eduardo Lima (Etrunko) wrote: > On 06/03/18 13:37, Christophe Fergeau wrote: > > On Tue, Mar 06, 2018 at 11:46:33AM -0300, Eduardo Lima (Etrunko) wrote: > >> This is a preparation for meson build, which has built-in support for >

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

On Tue, Mar 06, 2018 at 11:38:15AM -0500, Frediano Ziglio wrote: > Looks fine. > Actually when this function returns -1 spice_server_init returns -1 and Qemu > calls exit so there's no much difference at runtime. > But I suppose this is necessary for your tests. I did not try the tests without

Re: [Spice-devel] [PATCH spice-server] build: Rename spice-server-enums.tmpl.[ch] to spice-server-enums.[ch].tmpl

On Tue, Mar 06, 2018 at 11:46:33AM -0300, Eduardo Lima (Etrunko) wrote: > This is a preparation for meson build, which has built-in support for > generating enums, but requires the template files to be renamed. It uses > the basename of template files to generate the output, and in this case > it

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

On Thu, Mar 01, 2018 at 09:01:37PM +0100, Christophe de Dinechin wrote: > > On 28 Feb 2018, at 17:36, Christophe Fergeau <cferg...@redhat.com> wrote: > > > > My understanding is that the previous iteration was quite controversial, > > I would just drop it from the s

[Spice-devel] [spice-server 8/8] test-listen: Add Unix socket test

--- configure.ac | 3 +++ server/tests/Makefile.am | 4 +++- server/tests/test-listen.c | 38 +++--- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index bcd4bb4d5..863834343 100644 --- a/configure.ac

<    4   5   6   7   8   9   10   11   12   13   >