Re: [Spice-devel] [PATCH 7/7] worker: move red_record_command

2015-08-14 Thread Jonathon Jongsma
Again, I think it may be worthwhile to simply combine this commit with an earlier one. On Thu, 2015-08-13 at 16:25 +0100, Frediano Ziglio wrote: > From: Marc-André Lureau > > --- > server/red_record_qxl.c | 35 --- > server/red_record_qxl.h | 18 +++--

Re: [Spice-devel] [PATCH 6/7] worker: move red_record_event

2015-08-14 Thread Jonathon Jongsma
I think that perhaps this commit could be combined with a previous one. Another comment below On Thu, 2015-08-13 at 16:25 +0100, Frediano Ziglio wrote: > From: Marc-André Lureau > > --- > server/red_record_qxl.c | 12 > server/red_record_qxl.h | 2 ++ > server/red_worker.c | 2

Re: [Spice-devel] [PATCH 4/7] server/red_worker: record to SPICE_WORKER_RECORD_FILENAME

2015-08-14 Thread Jonathon Jongsma
On Thu, 2015-08-13 at 16:25 +0100, Frediano Ziglio wrote: > From: Alon Levy > > if the environment variable in the title is set and can be > opened for writing a log of all display commands (no cursor > commands yet) and any QXLWorker calls (particularily primary > create and destroy) will be log

Re: [Spice-devel] [PATCH 0/7] Implement record/replay

2015-08-14 Thread Jonathon Jongsma
On Fri, 2015-08-14 at 13:15 -0500, Jonathon Jongsma wrote: > On Thu, 2015-08-13 at 16:25 +0100, Frediano Ziglio wrote: > > With this series of patches is possible to record what happens to > > spice-server and replay it. > > The main purpose is debugging. > > Note that these patches are quite old a

Re: [Spice-devel] [PATCH 0/7] Implement record/replay

2015-08-14 Thread Jonathon Jongsma
On Thu, 2015-08-13 at 16:25 +0100, Frediano Ziglio wrote: > With this series of patches is possible to record what happens to > spice-server and replay it. > The main purpose is debugging. > Note that these patches are quite old and none is mine (I just did > some minor changes). > These are part o

Re: [Spice-devel] [PATCH 1/2] build-sys: Require a new enough spice-protocol in .pc file

2015-08-14 Thread Fabiano Fidencio
- Original Message - > From: "Christophe Fergeau" > To: spice-de...@freedesktop.org > Sent: Friday, August 14, 2015 6:19:01 PM > Subject: [Spice-devel] [PATCH 1/2] build-sys: Require a new enough > spice-protocol in .pc file > > spice-server headers expose SpiceImageCompression whic

Re: [Spice-devel] [PATCH 5/7] server/tests/replay: introduce

2015-08-14 Thread Jonathon Jongsma
On Thu, 2015-08-13 at 16:25 +0100, Frediano Ziglio wrote: > From: Alon Levy > > usage: replay This usage doesn't really match the implementation below. It should be something more like: replay -p -c While we're on the subject, though, can we use a more specific name? e.g. spice-replay >

[Spice-devel] [spice-server PATCH v2 07/10] migration_protocol: use SPICE_MAGIC_CONST

2015-08-14 Thread Victor Toso
spice-protocol has a new define to create the magic constants, let's use that. --- server/migration_protocol.h | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/server/migration_protocol.h b/server/migration_protocol.h index fa17c7c..21d3ec8 100644 --- a/server/migrat

[Spice-devel] [spice-server PATCH v2 09/10] smartcard: use SPICE_ALIGNED_CAST

2015-08-14 Thread Victor Toso
In order to avoid false posive alignment warnings clang: smartcard.c:131:29: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'VSCMsgHeader *' (aka 'struct VSCMsgHeader *') increases required alignment from 1 to 4 [-Werror,-Wcast-align] VSCMsgHeader *vheader = (VSCMsgHeader*)state->buf;

[Spice-devel] [spice-server PATCH v2 05/10] glz: WindowImageSegment lines lines_end as void*

2015-08-14 Thread Victor Toso
Instead of using uint8_t* which can cause several warnings on casting as the example below: ./glz_encode_tmpl.c:321:29: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'rgb16_pixel_t *' (aka 'unsigned short *') increases required alignment from 1 to 2 [-Werror,-Wcast-align] ref_limit = (P

[Spice-devel] [spice-server PATCH v2 10/10] fix warnings about memory alignment

2015-08-14 Thread Victor Toso
- By either using SPICE_ALIGNED_CAST to false positives and SPICE_UNALIGNED_CAST to the cast that could lead to problems; or - By using a better type to the variable; --- server/lz4_encoder.c | 2 +- server/red_parse_qxl.c | 4 ++-- server/red_worker.c| 4 ++-- server/reds.c | 8 +++

[Spice-devel] [spice-server PATCH v2 06/10] mjpeg and jpeg encoder: fix alignment warnings

2015-08-14 Thread Victor Toso
As the input line could be uint8_t*, uint16_t* or uint32_t*, changing the default from uint8_t* to void* seems the correct choice to deal with upcasting warnings. Regarding chunks->data allocation, I quote Frediano explantion: "Lines came from spice_bitmap_get_line. This function assume that bitma

[Spice-devel] [spice-protocol PATCH v2 02/10] macros: fix alignment issue reported by clang

2015-08-14 Thread Victor Toso
char_device.c:131:52: warning: cast from 'uint8_t *' (aka 'unsigned char *') to 'SpiceCharDeviceMsgToClientItem *' (aka 'struct SpiceCharDeviceMsgToClientItem *') increases required alignment from 1 to 8 [-Wcast-align] SpiceCharDeviceMsgToClientItem *msg_item = SPICE_CONTAINEROF(item,

[Spice-devel] [spice-protocol PATCH v2 00/10] building with clang

2015-08-14 Thread Victor Toso
Here we go with v2. Maybe I could squash some of these patches... Patch 0005 is acked-by Frediano; Thanks for any input, changes from v1 -> drop patch 02 and 12: on behaf of Frediano's better approach (upstream) http://cgit.freedesktop.org/spice/spice-protocol/commit/?id=295d05e7330da45b705aa5

[Spice-devel] [spice-protocol PATCH v2 01/10] macros: verify if __alloc_size__ works with clang

2015-08-14 Thread Victor Toso
So we can avoid using using an attribute not supported for the compiler. warning: ../spice-common/common/mem.h:91:80: warning: unknown attribute '__alloc_size__' ignored [-Wunknown-attributes] void *spice_malloc0_n(size_t n_blocks, size_t n_block_bytes) SPICE_GNUC_MALLOC SPICE_GNUC_ALLOC_SIZE2(1,

[Spice-devel] [spice-common PATCH v2 04/10] fix warnings about memory alignment

2015-08-14 Thread Victor Toso
- By either using SPICE_ALIGNED_CAST to false positives and SPICE_UNALIGNED_CAST to the cast that could lead to problems; or - By using a better type to the variable; --- common/canvas_base.c | 16 +--- common/sw_canvas.c | 10 ++ 2 files changed, 15 insertions(+), 11 deletio

[Spice-devel] [spice-server PATCH v2 08/10] unaligned type with spice_marshaller_reserve_space

2015-08-14 Thread Victor Toso
--- server/char_device.c | 6 -- server/red_worker.c | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/server/char_device.c b/server/char_device.c index 54357f0..27681d3 100644 --- a/server/char_device.c +++ b/server/char_device.c @@ -920,8 +920,10 @@ void spice_char_de

[Spice-devel] [spice-protocol PATCH v2 03/10] macros: new define to help with aligment warnings

2015-08-14 Thread Victor Toso
and also to help track possible alignment issues. --- spice/macros.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/spice/macros.h b/spice/macros.h index feac557..463c137 100644 --- a/spice/macros.h +++ b/spice/macros.h @@ -423,4 +423,10 @@ ((uint32_t)((s[3]&0xffu)|((s[2]&0xffu)<<

[Spice-devel] [PATCH 1/2] build-sys: Require a new enough spice-protocol in .pc file

2015-08-14 Thread Christophe Fergeau
spice-server headers expose SpiceImageCompression which is only available from recent spice-protocol releases. This dependency must be expressed in Requires and not Requires.private --- configure.ac | 5 +++-- spice-server.pc.in | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff -

[Spice-devel] [PATCH 2/2] server: Readd spice-experimental.h

2015-08-14 Thread Christophe Fergeau
Commit 3c6b4e41 removed spice-experimental.h as this header was not used, nor supposed to be used. However, QEMU had been including it (without using any of its symbols) until commit v2.3.0-rc0~135^2~1 As this is fairly recent (Nov 2014), building older QEMUs with new spice-server releases, or eve

Re: [Spice-devel] [spice-common PATCH v1 9/12] clang-noise

2015-08-14 Thread Victor Toso
Hey, Thanks for taking time to review this. On Thu, Aug 06, 2015 at 09:46:29AM -0400, Frediano Ziglio wrote: > > > > --- > > common/canvas_base.c | 12 ++-- > > common/sw_canvas.c | 6 +++--- > > 2 files changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/common/canvas_bas

Re: [Spice-devel] [spice-common] build-sys: Remove code generation files from EXTRA_DIST

2015-08-14 Thread Christophe Fergeau
Thanks for the tests/reviews, pushed now with the change Victor suggested. Christophe On Fri, Aug 14, 2015 at 10:17:10AM -0400, Frediano Ziglio wrote: > > > > On Fri, Aug 14, 2015 at 03:38:40PM +0200, Victor Toso wrote: > > > Hey, > > > > > > On Fri, Aug 14, 2015 at 11:22:22AM +0200, Christophe

Re: [Spice-devel] [spice-gtk 1/2] build-sys: Remove spice-protocol submodule

2015-08-14 Thread Christophe Fergeau
On Fri, Aug 14, 2015 at 04:08:35PM +0200, Christophe Fergeau wrote: > On Fri, Aug 14, 2015 at 03:27:12PM +0200, Victor Toso wrote: > > Hi, > > > > On Fri, Aug 07, 2015 at 12:49:22PM +0200, Christophe Fergeau wrote: > > > It's seeing regular releases and is API stable, so we don't need to > > > bun

Re: [Spice-devel] [spice-common] build-sys: Remove code generation files from EXTRA_DIST

2015-08-14 Thread Frediano Ziglio
> > On Fri, Aug 14, 2015 at 03:38:40PM +0200, Victor Toso wrote: > > Hey, > > > > On Fri, Aug 14, 2015 at 11:22:22AM +0200, Christophe Fergeau wrote: > > > This breaks make distcheck otherwise > > > > ... due spice-protocol removal c48b0a0672ccfbdfc67dc4a29eff97a57989f786 > > I'll add that (cor

Re: [Spice-devel] [spice-gtk 1/2] build-sys: Remove spice-protocol submodule

2015-08-14 Thread Christophe Fergeau
On Fri, Aug 14, 2015 at 03:27:12PM +0200, Victor Toso wrote: > Hi, > > On Fri, Aug 07, 2015 at 12:49:22PM +0200, Christophe Fergeau wrote: > > It's seeing regular releases and is API stable, so we don't need to > > bundle it with spice-gtk > > --- > > configure.ac | 8 +--- > > spice-common |

Re: [Spice-devel] [spice-common] build-sys: Remove code generation files from EXTRA_DIST

2015-08-14 Thread Christophe Fergeau
On Fri, Aug 14, 2015 at 03:38:40PM +0200, Victor Toso wrote: > Hey, > > On Fri, Aug 14, 2015 at 11:22:22AM +0200, Christophe Fergeau wrote: > > This breaks make distcheck otherwise > > ... due spice-protocol removal c48b0a0672ccfbdfc67dc4a29eff97a57989f786 I'll add that (correct commit is 7665dc

Re: [Spice-devel] [spice-common] build-sys: Remove code generation files from EXTRA_DIST

2015-08-14 Thread Victor Toso
Hey, On Fri, Aug 14, 2015 at 11:22:22AM +0200, Christophe Fergeau wrote: > This breaks make distcheck otherwise ... due spice-protocol removal c48b0a0672ccfbdfc67dc4a29eff97a57989f786 > --- > Makefile.am | 8 > 1 file changed, 8 deletions(-) > > diff --git a/Makefile.am b/Makefile.am

Re: [Spice-devel] [spice-gtk 2/2] Adjust to new SpiceImageCompress name

2015-08-14 Thread Victor Toso
Hi, On Fri, Aug 07, 2015 at 12:49:23PM +0200, Christophe Fergeau wrote: > This has been renamed to SpiceImageCompression in order to avoid clashes > with older spice-server in the SPICE_IMAGE_COMPRESS_ namespace. This > commit is a straight rename of SpiceImageCompress to > SpiceImageCompression a

Re: [Spice-devel] [PATCH v6 42/42] proto: Describe Quic image format from dissector

2015-08-14 Thread Frediano Ziglio
> > On Fri, Aug 14, 2015 at 08:37:49AM -0400, Frediano Ziglio wrote: > > The enumerations are different as there is no such garbage collector. > > It would make sense from the visual prospective (users that read > > spice.proto > > could understand fast that are only for dissector). > > > > I thi

Re: [Spice-devel] [spice-gtk 1/2] build-sys: Remove spice-protocol submodule

2015-08-14 Thread Victor Toso
Hi, On Fri, Aug 07, 2015 at 12:49:22PM +0200, Christophe Fergeau wrote: > It's seeing regular releases and is API stable, so we don't need to > bundle it with spice-gtk > --- > configure.ac | 8 +--- > spice-common | 2 +- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/co

Re: [Spice-devel] [PATCH v6 42/42] proto: Describe Quic image format from dissector

2015-08-14 Thread Christophe Fergeau
On Fri, Aug 14, 2015 at 08:37:49AM -0400, Frediano Ziglio wrote: > The enumerations are different as there is no such garbage collector. > It would make sense from the visual prospective (users that read spice.proto > could understand fast that are only for dissector). > > I think is up to you...

Re: [Spice-devel] [PATCH v6 42/42] proto: Describe Quic image format from dissector

2015-08-14 Thread Frediano Ziglio
> On Thu, Aug 13, 2015 at 02:12:21PM +0100, Frediano Ziglio wrote: > > Signed-off-by: Frediano Ziglio > > --- > > spice.proto | 25 - > > 1 file changed, 24 insertions(+), 1 deletion(-) > > > > diff --git a/spice.proto b/spice.proto > > index c7944ac..6a51645 100644 > > -

Re: [Spice-devel] [PATCH v6 42/42] proto: Describe Quic image format from dissector

2015-08-14 Thread Christophe Fergeau
On Thu, Aug 13, 2015 at 02:12:21PM +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > spice.proto | 25 - > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/spice.proto b/spice.proto > index c7944ac..6a51645 100644 > --- a/spice.proto >

Re: [Spice-devel] [PATCH v6 01/42] Supports ifdef attribute on enumerations

2015-08-14 Thread Christophe Fergeau
Hey, Ah, this adds some #ifdef to the enums.h file. I was hoping we could totally omit them from the output of the generator ;) Should do, but then the @ifdef should be SPICE_DISSECTOR, not a non-namespaced DISSECTOR. Christophe On Thu, Aug 13, 2015 at 02:11:40PM +0100, Frediano Ziglio wrote: >

Re: [Spice-devel] [PATCH v2 2/2] build-sys: Remove spice-protocol submodule

2015-08-14 Thread Victor Toso
Hey, On Fri, Aug 14, 2015 at 11:20:05AM +0200, Christophe Fergeau wrote: > It's seeing regular releases and is API stable, so we don't need to > bundle it with spice-server Both patches are fine by me and I'm current using it locally. toso > --- > configure.ac | 5 - > server/

[Spice-devel] [spice-common] build-sys: Remove code generation files from EXTRA_DIST

2015-08-14 Thread Christophe Fergeau
This breaks make distcheck otherwise --- Makefile.am | 8 1 file changed, 8 deletions(-) diff --git a/Makefile.am b/Makefile.am index ecf7bd9..4b138fe 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3,14 +3,6 @@ ACLOCAL_AMFLAGS = -I m4 SUBDIRS = common -EXTRA_DIST =

[Spice-devel] [PATCH v2 1/2] build-sys: Remove test_spice_version.h

2015-08-14 Thread Christophe Fergeau
This script was used at make distcheck time to verify consistency of the version number defined in configure.ac and in spice-server headers. Since commit ab12cf414c, these 2 version numbers can no longer be out of sync, so we can drop this script. --- server/Makefile.am | 3 --- s

[Spice-devel] [PATCH v2 2/2] build-sys: Remove spice-protocol submodule

2015-08-14 Thread Christophe Fergeau
It's seeing regular releases and is API stable, so we don't need to bundle it with spice-server --- configure.ac | 5 - server/Makefile.am | 1 + server/tests/Makefile.am | 1 + spice-common | 2 +- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/co

Re: [Spice-devel] [spice-server PATCH v1] build-sys: remove -I spice-protocol as submodule

2015-08-14 Thread Christophe Fergeau
On Thu, Aug 13, 2015 at 12:19:57PM -0400, Frediano Ziglio wrote: > > > Hey, > > > > This is the same as > > http://lists.freedesktop.org/archives/spice-devel/2015-August/021310.html > > which is slightly more complete (checks for new enough spice-protocol, > > and one more change in server/tests/