[Spice-devel] [PATCH x11spice] Fix a bug in the tests.

2019-07-18 Thread Jeremy White
We were overly dependent on timing for success; now we deliberately wait for our GC and drawing to finish. Signed-off-by: Jeremy White --- src/tests/xcb.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/tests/xcb.c b/src/tests/xcb.c index 6be4a69b..709cdf

[Spice-devel] [PATCH v2 x11spice 3/4] Bug fix: support --config= semantics.

2019-07-18 Thread Jeremy White
Instead of trying to parse argv ourselves, just reuse getopt_long_only. Signed-off-by: Jeremy White --- src/main.c| 8 +--- src/options.c | 50 -- src/options.h | 2 +- 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/

[Spice-devel] [PATCH v2 x11spice 0/4] Resend two small fixes

2019-07-18 Thread Jeremy White
v2: Use glib memory routines with options.c Fix a memory use bug in options Just call getopt_long twice, rather than trying to do a poor implementation For the listen bug fix, use == NULL, and g_strdup ___ Spice-devel mailing list Spice-

[Spice-devel] [PATCH v2 x11spice 1/4] Convert to the use of glib memory routines in options.c.

2019-07-18 Thread Jeremy White
Signed-off-by: Jeremy White --- src/options.c | 44 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/src/options.c b/src/options.c index b7f487c5..a206c92c 100644 --- a/src/options.c +++ b/src/options.c @@ -52,10 +52,10 @@ void options_i

[Spice-devel] [PATCH v2 x11spice 4/4] Bug fix: a listen specification from the config file was ignored

2019-07-18 Thread Jeremy White
Signed-off-by: Jeremy White --- src/options.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/options.c b/src/options.c index 5a55f2c9..a3dd20ac 100644 --- a/src/options.c +++ b/src/options.c @@ -339,7 +339,9 @@ int options_parse_arguments(int argc, char *argv[], optio

[Spice-devel] [PATCH v2 x11spice 2/4] Free the SSL and password_file option fields.

2019-07-18 Thread Jeremy White
Signed-off-by: Jeremy White --- src/options.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/src/options.c b/src/options.c index a206c92c..303c07de 100644 --- a/src/options.c +++ b/src/options.c @@ -50,6 +50,17 @@ void options_init(options_t *options) memset(options, 0,

Re: [Spice-devel] [PATCH spice-protocol 2/2] qxl_dev: Move QXLReleaseInfoExt out of start/end-packed.h

2019-07-18 Thread Frediano Ziglio
ping > > This structure is not declares as SPICE_ATTR_PACKED resulting it > as aligned and no packed using GCC (no MingW). > This structure is only packed under MingW or Microsoft compilers. > This structure is not technically a definition for QXL device but > is used only for spice-server QXL in

Re: [Spice-devel] [PATCH spice-server] red-replay-qxl: Fix some issue of alignment

2019-07-18 Thread Frediano Ziglio
ping > > Do not pass unaligned QXLPHYSICAL but pass a valid pointer and > then cast. > > Signed-off-by: Frediano Ziglio > --- > server/red-replay-qxl.c | 16 > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c

[Spice-devel] [PATCH spice-protocol] start-packet: Correct misleading comment

2019-07-18 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio --- spice/start-packed.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spice/start-packed.h b/spice/start-packed.h index de0c595..d1d090e 100644 --- a/spice/start-packed.h +++ b/spice/start-packed.h @@ -30,8 +30,8 @@ */ /* Ideally this

Re: [Spice-devel] [PATCH spice-protocol 1/2 v2] qxl_dev: Fix alignment for QXLReleaseInfo

2019-07-18 Thread Frediano Ziglio
> > > On 7/18/19 5:51 PM, Frediano Ziglio wrote: > >> Hi, > >> > >> > >> IIRC this was related to some compiler warning, no? > > Yes, recent compilers are reporting it, see below. > > > >> If it is I'd mentioning it , otherwise, ack. > >> > > Just this patch or the entire series? > > > No, actu

Re: [Spice-devel] [PATCH spice-protocol 1/2 v2] qxl_dev: Fix alignment for QXLReleaseInfo

2019-07-18 Thread Snir Sheriber
On 7/18/19 5:51 PM, Frediano Ziglio wrote: Hi, IIRC this was related to some compiler warning, no? Yes, recent compilers are reporting it, see below. If it is I'd mentioning it , otherwise, ack. Just this patch or the entire series? No, actually i started looking at the second one and

Re: [Spice-devel] [PATCH spice-server v3] ci: Workaround an issue with GLib on Fedora 30

2019-07-18 Thread Uri Lublin
On 7/17/19 3:06 PM, Frediano Ziglio wrote: This remove this error running test-listen test on a Fedora 30 docker image: (/builds/spice/spice/build/server/tests/test-listen:2233): GLib-GIO-CRITICAL **: 15:29:03.123: g_file_new_for_path: assertion 'path != NULL' failed This error is due to some

Re: [Spice-devel] [PATCH x11spice 1/2] Bug fix: --config= did not work.

2019-07-18 Thread Uri Lublin
On 7/18/19 5:31 PM, Jeremy White wrote: Signed-off-by: Jeremy White Hi, The patch looks good to me. See a comment below. --- src/options.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/options.c b/src/options.c index b7f487c5..0d3138d0

Re: [Spice-devel] [PATCH x11spice 1/2] Bug fix: --config= did not work.

2019-07-18 Thread Frediano Ziglio
It seems that x11spice is more X11 complaint than getopt_long so I don't see the --config=/-config= mandatory. Giving that -config is not GNU convention (they are 5 options!) I would avoid the mix. > > Signed-off-by: Jeremy White > --- > src/options.c | 24 ++-- > 1 file ch

Re: [Spice-devel] [PATCH x11spice 2/2] Bug fix: a listen specification from the config file was ignored

2019-07-18 Thread Frediano Ziglio
> > Signed-off-by: Jeremy White > --- > src/options.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/options.c b/src/options.c > index 0d3138d0..cf75e54e 100644 > --- a/src/options.c > +++ b/src/options.c > @@ -355,7 +355,9 @@ int options_parse_arguments(int arg

Re: [Spice-devel] [PATCH spice-protocol 1/2 v2] qxl_dev: Fix alignment for QXLReleaseInfo

2019-07-18 Thread Frediano Ziglio
> > Hi, > > > IIRC this was related to some compiler warning, no? Yes, recent compilers are reporting it, see below. > If it is I'd mentioning it , otherwise, ack. > Just this patch or the entire series? > Snir. > > > On 7/4/19 4:56 PM, Frediano Ziglio wrote: > > Do not declare the struct

Re: [Spice-devel] [PATCH spice-server 3/3] ci: Add some Valgrind suppressions for Fedora 30

2019-07-18 Thread Frediano Ziglio
> > On 7/18/19 10:32 AM, Frediano Ziglio wrote: > > Signed-off-by: Frediano Ziglio > > Hi Frediano, > > I've been playing with it too. > Had similar rules but different. > > Your version works for me. > Some minor comments below. > > > --- > > server/tests/valgrind/spice.supp | 25 +

Re: [Spice-devel] [PATCH spice-server 0/3] Update Valgrind suppression files

2019-07-18 Thread Uri Lublin
On 7/18/19 10:32 AM, Frediano Ziglio wrote: This series split and update Valgrind suppression files in order for make Gitlab CI pass. Works for me. Some minor comments for patch 3. Ack series. Uri. Results at https://gitlab.freedesktop.org/fziglio/spice/-/jobs/430006. Note that to pass Va

Re: [Spice-devel] [PATCH spice-server 3/3] ci: Add some Valgrind suppressions for Fedora 30

2019-07-18 Thread Uri Lublin
On 7/18/19 10:32 AM, Frediano Ziglio wrote: Signed-off-by: Frediano Ziglio Hi Frediano, I've been playing with it too. Had similar rules but different. Your version works for me. Some minor comments below. --- server/tests/valgrind/spice.supp | 25 + 1 file chang

[Spice-devel] [PATCH x11spice 1/2] Bug fix: --config= did not work.

2019-07-18 Thread Jeremy White
Signed-off-by: Jeremy White --- src/options.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/options.c b/src/options.c index b7f487c5..0d3138d0 100644 --- a/src/options.c +++ b/src/options.c @@ -213,14 +213,34 @@ void options_handle_ssl_file_opti

[Spice-devel] [PATCH x11spice 2/2] Bug fix: a listen specification from the config file was ignored

2019-07-18 Thread Jeremy White
Signed-off-by: Jeremy White --- src/options.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/options.c b/src/options.c index 0d3138d0..cf75e54e 100644 --- a/src/options.c +++ b/src/options.c @@ -355,7 +355,9 @@ int options_parse_arguments(int argc, char *argv[], optio

[Spice-devel] [PATCH x11spice] Use C99 struct initializiers instead of memset for local structures.

2019-07-18 Thread Jeremy White
Signed-off-by: Jeremy White --- src/gui.c| 3 +-- src/listen.c | 3 +-- src/main.c | 4 +--- src/spice.c | 3 +-- 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/gui.c b/src/gui.c index 6748f66e..88acf5c9 100644 --- a/src/gui.c +++ b/src/gui.c @@ -147,10 +147,9 @@ void s

Re: [Spice-devel] [PATCH spice-protocol 1/2 v2] qxl_dev: Fix alignment for QXLReleaseInfo

2019-07-18 Thread Snir Sheriber
Hi, IIRC this was related to some compiler warning, no? If it is I'd mentioning it , otherwise, ack. Snir. On 7/4/19 4:56 PM, Frediano Ziglio wrote: Do not declare the structure as aligned. The start/end-packed.h headers affects structures without specification only using MingW or Microsoft

Re: [Spice-devel] [PATCH spice-server 06/13] red-channel: Inline red_channel_pipes_create_batch

2019-07-18 Thread Frediano Ziglio
ping > > ping > > > > > ping > > > > > > > > The function is called only by red_channel_pipes_new_add. > > > > > > Signed-off-by: Frediano Ziglio > > > --- > > > server/red-channel.c | 20 > > > 1 file changed, 4 insertions(+), 16 deletions(-) > > > > > > diff --git a

Re: [Spice-devel] [PATCH spice-server v2 1/4] spicevmc: Do not use RedCharDevice pipe items handling

2019-07-18 Thread Frediano Ziglio
ping > > ping the series > > > > > As we don't use any token there's no reason to not queue directly instead > > of passing through RedCharDevice. > > This will make easier to limit the queue which currently is unlimited. > > > > RedCharDevice flow control has some problems: > > - it's really

Re: [Spice-devel] [PATCH spice-server 03/13] char-device: Allocate all write buffer in a single block

2019-07-18 Thread Frediano Ziglio
ping > > ping > > > > > ping > > > > > > > > There are no much data other than the buffer, reduce the > > > allocations. > > > > > > Signed-off-by: Frediano Ziglio > > > --- > > > server/char-device.c | 27 +++ > > > server/char-device.h | 2 +- > > > 2 files chang

Re: [Spice-devel] [PATCH spice-server v2] char-device: Remove unused red_char_device_destroy function

2019-07-18 Thread Frediano Ziglio
ping > > ping > > > > > ping > > > > > > > > ping > > > > > > > > > > > ping > > > > > > > > > > > > > > ping > > > > > > > > > > > > > > > > > g_object_unref is directly used. > > > > > > > > > > > > Signed-off-by: Frediano Ziglio > > > > > > --- > > > > > > server/char-device.c | 6

Re: [Spice-devel] [PATCH spice-server 13/13] red-channel-client: Add some comment on the flush code

2019-07-18 Thread Frediano Ziglio
ping > > ping > > > > > ping > > > > > > > > ping > > > > > > > > > > > ping > > > > > > > > > > > > > > Signed-off-by: Frediano Ziglio > > > > > --- > > > > > server/red-channel-client.c | 6 ++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/server/red-

Re: [Spice-devel] [PATCH spice-server 02/13] char-device: Pull more code into red_char_device_send_to_client_tokens_absorb

2019-07-18 Thread Frediano Ziglio
ping > > ping > > > > > ping > > > > > > > > ping > > > > > > > > > > > The 2 callers red_char_device_send_to_client_tokens_set and > > > > red_char_device_send_to_client_tokens_add are doing mostly > > > > the same thing so put common code to > > > > red_char_device_send_to_client_tokens_a

Re: [Spice-devel] [PATCH spice-server v2 1/2] char-device: Don't use RedClient API

2019-07-18 Thread Frediano Ziglio
ping > > ping > > > ping > > > > > > > > ping the series > > > > > > > > > > > RedClient was an opaque structure for RedCharDevice. > > > > It started to be used when RedsState started to contain all > > > > the global state. > > > > Make it opaque again using a new RedCharDeviceClientOpaque

Re: [Spice-devel] [PATCH spice-server 05/13] red-worker: Remove warning

2019-07-18 Thread Frediano Ziglio
ping > > ping > > > > > ping > > > > > > > > ping > > > > > > > > > > > ping > > > > > > > > > > > > > > In some configuration _GNU_SOURCE is defined by the compiler > > > > > and defining again cause a warning. > > > > > Do not define again to avoid the warning. > > > > > > > > > > Sign

Re: [Spice-devel] [PATCH spice-server 12/13] red-channel-client: Reduce indentation of some code

2019-07-18 Thread Frediano Ziglio
ping > > ping > > > > > ping > > > > > > > > ping > > > > > > > > > > > Just a style change, return earlier to avoid some indentation. > > > > > > > > Signed-off-by: Frediano Ziglio > > > > --- > > > > server/red-channel-client.c | 37 +++-- > > > > 1 file

Re: [Spice-devel] [PATCH spice-server 04/13] spicevmc: Remove reds parameter from spicevmc_device_disconnect

2019-07-18 Thread Frediano Ziglio
ping > > ping > > > > > ping > > > > > > > > ping > > > > > > > > > > > ping > > > > > > > > > > > > > > Unused. > > > > > Also the devices should be able to release themselves. > > > > > > > > > > Signed-off-by: Frediano Ziglio > > > > > --- > > > > > server/char-device.h | 3 +-- > >

[Spice-devel] [PATCH spice-server 1/3] ci: Separate SPICE specific Valgrind suppression

2019-07-18 Thread Frediano Ziglio
Previously we add suppression to glib.supp file (suppressions from Glib). Keep the glib.supp file pristine and add another file specific for SPICE. Signed-off-by: Frediano Ziglio --- server/tests/Makefile.am | 2 +- server/tests/valgrind/glib.supp | 39

[Spice-devel] [PATCH spice-server 2/3] ci: Update glib.supp file

2019-07-18 Thread Frediano Ziglio
Sync with Glib master file. Signed-off-by: Frediano Ziglio --- server/tests/valgrind/glib.supp | 222 +++- 1 file changed, 220 insertions(+), 2 deletions(-) diff --git a/server/tests/valgrind/glib.supp b/server/tests/valgrind/glib.supp index 9236b3d16..ddc2b1729 1006

[Spice-devel] [PATCH spice-server 0/3] Update Valgrind suppression files

2019-07-18 Thread Frediano Ziglio
This series split and update Valgrind suppression files in order for make Gitlab CI pass. Results at https://gitlab.freedesktop.org/fziglio/spice/-/jobs/430006. Note that to pass Valgrind checks some other patches are needed but these series is complete for the suppression part. Frediano Ziglio (

[Spice-devel] [PATCH spice-server 3/3] ci: Add some Valgrind suppressions for Fedora 30

2019-07-18 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio --- server/tests/valgrind/spice.supp | 25 + 1 file changed, 25 insertions(+) diff --git a/server/tests/valgrind/spice.supp b/server/tests/valgrind/spice.supp index 1bfe81006..dd3663c68 100644 --- a/server/tests/valgrind/spice.supp +++ b/ser