[Spice-devel] [PATCH] Fix typo, change DeviceName to device_name

2018-11-16 Thread 谢 昆明 
Signed-off-by: kunming.xie 
---
 vdagent/display_configuration.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vdagent/display_configuration.h b/vdagent/display_configuration.h
index ef4f989..7e2f8ac 100644
--- a/vdagent/display_configuration.h
+++ b/vdagent/display_configuration.h
@@ -56,7 +56,7 @@ public:
 bool query_display_config();
 bool set_display_config(LONG & error);
 bool update_mode_position(LPCTSTR device_name, DEVMODE* dev_mode);
-bool update_mode_size(LPCTSTR DeviceNmae, DEVMODE* dev_mode);
+bool update_mode_size(LPCTSTR device_name, DEVMODE* dev_mode);
 void update_detached_primary_state(LPCTSTR device_name, 
DISPLAYCONFIG_PATH_INFO * path_info);
 bool set_path_state(LPCTSTR device_name, MONITOR_STATE state);
 bool is_attached(LPCTSTR device_name);
@@ -172,4 +172,4 @@ private:
 CCD _ccd;
 };
 
-#endif
\ No newline at end of file
+#endif
-- 
2.14.2.windows.2

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 5/5] tests: Add a small test for red_record_ APIs

2018-11-16 Thread Eduardo Lima (Etrunko) 
On 11/11/18 7:31 AM, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  server/tests/.gitignore|   1 +
>  server/tests/Makefile.am   |   1 +
>  server/tests/meson.build   |   1 +
>  server/tests/test-record.c | 100 +
>  4 files changed, 103 insertions(+)
>  create mode 100644 server/tests/test-record.c
> 
> diff --git a/server/tests/.gitignore b/server/tests/.gitignore
> index db26b3ee..81b604bc 100644
> --- a/server/tests/.gitignore
> +++ b/server/tests/.gitignore
> @@ -27,5 +27,6 @@ test-vdagent
>  test-gst
>  test-leaks
>  test-sasl
> +test-record
>  /test-*.log
>  /test-*.trs
> diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
> index 46cbe8cf..d7f7af9b 100644
> --- a/server/tests/Makefile.am
> +++ b/server/tests/Makefile.am
> @@ -63,6 +63,7 @@ check_PROGRAMS =\
>   test-channel\
>   test-stream-device  \
>   test-listen \
> + test-record \
>   $(NULL)
>  
>  noinst_PROGRAMS =\
> diff --git a/server/tests/meson.build b/server/tests/meson.build
> index 23794cc9..b79b1108 100644
> --- a/server/tests/meson.build
> +++ b/server/tests/meson.build
> @@ -47,6 +47,7 @@ tests = [
>['test-channel', true],
>['test-stream-device', true],
>['test-listen', true],
> +  ['test-record', true],
>['test-display-no-ssl', false],
>['test-display-streaming', false],
>['test-playback', false],
> diff --git a/server/tests/test-record.c b/server/tests/test-record.c
> new file mode 100644
> index ..de3c6f5b
> --- /dev/null
> +++ b/server/tests/test-record.c
> @@ -0,0 +1,100 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> +   Copyright (C) 2018 Red Hat, Inc.
> +
> +   This library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   This library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with this library; if not, see 
> .
> +*/
> +/*
> + * Test some red_record_ APIs.
> + */
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "test-glib-compat.h"
> +#include "red-record-qxl.h"
> +
> +#define OUTPUT_FILENAME "rec1.txt"
> +
> +static void
> +test_record(bool compress)
> +{
> +RedRecord *rec;
> +const char *fn = OUTPUT_FILENAME;
> +
> +unsetenv("SPICE_WORKER_RECORD_FILTER");
> +if (compress) {
> +setenv("SPICE_WORKER_RECORD_FILTER", "gzip", 1);
> +}
> +
> +// delete possible stale test output
> +unlink(fn);
> +g_assert_cmpint(access(fn, F_OK), <, 0);
> +
> +// create recorder
> +rec = red_record_new(fn);
> +g_assert_nonnull(rec);
> +
> +// check file was created by recorder
> +g_assert_cmpint(access(fn, F_OK), ==, 0);
> +
> +g_assert_nonnull(red_record_ref(rec));
> +red_record_unref(rec);
> +
> +// record something
> +red_record_event(rec, 1, 123);
> +
> +red_record_unref(rec);
> +
> +// check content of the output file
> +FILE *f;
> +if (!compress) {
> +f = fopen(fn, "r");
> +} else {
> +f = popen("gzip -dc < " OUTPUT_FILENAME, "r");
> +}
> +g_assert_nonnull(f);
> +
> +char line[1024];
> +int version;
> +g_assert_nonnull(fgets(line, sizeof(line), f));
> +g_assert_cmpint(sscanf(line, "SPICE_REPLAY %d", &version), ==, 1);
> +
> +int w, t;
> +g_assert_nonnull(fgets(line, sizeof(line), f));
> +g_assert_cmpint(sscanf(line, "event %*d %d %d", &w, &t), ==, 2);
> +g_assert_cmpint(w, ==, 1);
> +g_assert_cmpint(t, ==, 123);
> +
> +g_assert_null(fgets(line, sizeof(line), f));
> +
> +if (!compress) {
> +fclose(f);
> +} else {
> +pclose(f);
> +}
> +
> +// clean test output file
> +unlink(fn);
> +}
> +
> +int
> +main(void)
> +{
> +test_record(false);
> +test_record(true);
> +return 0;
> +}
> 
Acked-by: Eduardo Lima (Etrunko) 

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 2/5] test-leaks: Avoid clashing with test-display-base ports

2018-11-16 Thread Eduardo Lima (Etrunko) 
On 11/11/18 7:31 AM, Frediano Ziglio wrote:
> Is possible that port 5913 is already in use as tests that uses
> test_new will attempt to use ports from 5912 to 5921 so use a port
> not in that range.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  server/tests/test-leaks.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/server/tests/test-leaks.c b/server/tests/test-leaks.c
> index 41734c57..64130c22 100644
> --- a/server/tests/test-leaks.c
> +++ b/server/tests/test-leaks.c
> @@ -50,7 +50,7 @@ static void server_leaks(void)
>  core = basic_event_loop_init();
>  g_assert_nonnull(core);
>  
> -result = spice_server_set_tls(server, 5913,
> +result = spice_server_set_tls(server, 5922,
>PKI_DIR "ca-cert.pem",
>PKI_DIR "server-cert.pem",
>PKI_DIR "server-key.pem",
> 
Acked-by: Eduardo Lima (Etrunko) 

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 1/5] test-display-base: Fix warning message avoidance

2018-11-16 Thread Eduardo Lima (Etrunko) 
On 11/11/18 7:31 AM, Frediano Ziglio wrote:
> test_new function attempts to detect attempts to listen to tcp ports
> already in listening state detecting some messages during
> spice_server_init. However the check is wrong (broken in recent
> 34a44d3e940bcc "test-display-base: Avoid spurious errors due to listen
> failures") and incomplete (missing message).
> 
> To better test this conditions put some of the ports in listening
> state (like with a "nc -l 5912 & nc -l 5913 &" command) and run
> tests in parallel (like with a "make check -j" command).
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  server/tests/test-display-base.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/server/tests/test-display-base.c 
> b/server/tests/test-display-base.c
> index f58f76d3..e9f6c54d 100644
> --- a/server/tests/test-display-base.c
> +++ b/server/tests/test-display-base.c
> @@ -905,8 +905,9 @@ static gboolean ignore_in_use_failures(const gchar 
> *log_domain,
>  if ((log_level & G_LOG_LEVEL_WARNING) == 0)  {
>  return true;
>  }
> -if (strstr(message, "reds_init_socket: binding socket to ") == NULL || 
> // bind failure
> -strstr(message, "reds_init_socket: listen: ") == NULL) { // listen 
> failure
> +if (strstr(message, "reds_init_socket: binding socket to ") == NULL && 
> // bind failure
> +strstr(message, "reds_init_socket: listen: ") == NULL && // listen 
> failure
> +strstr(message, "Failed to open SPICE sockets") == NULL) { // global
>  g_print("XXX [%s]\n", message);
>  return true;
>  }
> 
Acked-by: Eduardo Lima (Etrunko) 

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH vd_agent_linux] vdagent: Stop trying to connect to the daemon after a while

2018-11-16 Thread Frediano Ziglio 
Do not try  indefinitely to connect to the daemon, should not
take long to activate.

Signed-off-by: Frediano Ziglio 
---
 src/vdagent/vdagent.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
index f7c8b72..e0228d7 100644
--- a/src/vdagent/vdagent.c
+++ b/src/vdagent/vdagent.c
@@ -53,6 +53,7 @@ typedef struct VDAgent {
 struct vdagent_file_xfers *xfers;
 struct udscs_connection *conn;
 GIOChannel *x11_channel;
+guint connection_attempts;
 
 GMainLoop *loop;
 } VDAgent;
@@ -370,6 +371,10 @@ static gboolean vdagent_init_async_cb(gpointer user_data)
 daemon_read_complete, daemon_disconnect_cb,
 debug);
 if (agent->conn == NULL) {
+// limit connection attempts, this will try for 5 minutes
+if (++agent->connection_attempts > 5 * 60) {
+goto err_init;
+}
 g_timeout_add_seconds(1, vdagent_init_async_cb, agent);
 return G_SOURCE_REMOVE;
 }
-- 
2.17.2

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 1/2] ci: Merge new glib.supp file

2018-11-16 Thread Eduardo Lima (Etrunko) 
On 11/9/18 8:06 AM, Frediano Ziglio wrote:
> Merge from GLib master repository.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  server/tests/valgrind/glib.supp | 54 ++---
>  1 file changed, 43 insertions(+), 11 deletions(-)
> 
> diff --git a/server/tests/valgrind/glib.supp b/server/tests/valgrind/glib.supp
> index fb2cb4a7..e95f1cb2 100644
> --- a/server/tests/valgrind/glib.supp
> +++ b/server/tests/valgrind/glib.supp
> @@ -17,7 +17,7 @@
>  # This file should be updated if GLib introduces a new deliberate one-time 
> leak,
>  # or another false race positive in Valgrind: please file bugs at:
>  #
> -# https://bugzilla.gnome.org/enter_bug.cgi?product=glib
> +# https://gitlab.gnome.org/GNOME/glib/issues/new
>  
>  {
>   gnutls-init-calloc
> @@ -51,6 +51,36 @@
>   fun:initialize_module_inlock_reentrant
>  }
>  
> +# One-time allocation from libc for getpwnam() results
> +{
> + g-local-vfs-getpwnam
> + Memcheck:Leak
> + fun:malloc
> + ...
> + fun:getpwnam
> + fun:g_local_vfs_parse_name
> +}
> +
> +{
> + glib-init-malloc
> + Memcheck:Leak
> + fun:malloc
> + ...
> + fun:g_quark_init
> + ...
> + fun:glib_init_ctor
> +}
> +
> +{
> + glib-init-calloc
> + Memcheck:Leak
> + fun:calloc
> + ...
> + fun:g_quark_init
> + ...
> + fun:glib_init_ctor
> +}
> +
>  {
>   gobject-init-malloc
>   Memcheck:Leak
> @@ -232,6 +262,18 @@
>   fun:_g_io_module_get_default
>  }
>  
> +# One-time getaddrinfo() configuration loading
> +{
> + g-threaded-resolver-getaddrinfo-config
> + Memcheck:Leak
> + fun:malloc
> + ...
> + fun:__resolv_conf_allocate
> + ...
> + fun:getaddrinfo
> + fun:do_lookup_by_name
> +}
> +
>  # memcheck checks that the third argument to ioctl() is a valid pointer, but
>  # some ioctls use that argument as an integer
>  {
> @@ -502,16 +544,6 @@
>   fun:g_object_new_valist
>  }
>  
> -{
> - getaddrinfo
> - Memcheck:Leak
> - fun:malloc
> - ...
> - fun:__resolv_conf_allocate
> - ...
> - fun:getaddrinfo
> -}
> -
>  {
>   px_proxy_factory_get_proxies
>   Memcheck:Leak
> 
Acked-by: Eduardo Lima (Etrunko) 

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 2/2] ci: Ignore leak from GnuTLS p11 call

2018-11-16 Thread Eduardo Lima (Etrunko) 
On 11/9/18 8:06 AM, Frediano Ziglio wrote:
> Fix Fedora 29 one-time leak.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  server/tests/valgrind/glib.supp | 10 ++
>  1 file changed, 10 insertions(+)
> 
> See https://gitlab.freedesktop.org/fziglio/spice/-/jobs/41884 for
> results, this commit fix Gitlab CI.
> 
> diff --git a/server/tests/valgrind/glib.supp b/server/tests/valgrind/glib.supp
> index e95f1cb2..ca0684d6 100644
> --- a/server/tests/valgrind/glib.supp
> +++ b/server/tests/valgrind/glib.supp
> @@ -572,3 +572,13 @@
>   ...
>   fun:gnutls_rnd
>  }
> +
> +{
> + gnutls_tls_p11_kit_leak
> + Memcheck:Leak
> + fun:malloc
> + ...
> + fun:__tls_get_addr
> + ...
> + fun:p11_kit_modules_load
> +}
> 
Acked-by: Eduardo Lima (Etrunko) 

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 0/5] Various CI and tests improvements

2018-11-16 Thread Frediano Ziglio 
ping, CI still failing

> 
> Fix minor CI issues running tests in parallel.
> Fix Valgrind execution on Fedora 29 (which Gitlab CI now uses).
> Add a test.
> 
> Frediano Ziglio (5):
>   test-display-base: Fix warning message avoidance
>   test-leaks: Avoid clashing with test-display-base ports
>   ci: Merge new glib.supp file
>   ci: Ignore leak from GnuTLS p11 call
>   tests: Add a small test for red_record_ APIs
> 
>  server/tests/.gitignore  |   1 +
>  server/tests/Makefile.am |   1 +
>  server/tests/meson.build |   1 +
>  server/tests/test-display-base.c |   5 +-
>  server/tests/test-leaks.c|   2 +-
>  server/tests/test-record.c   | 100 +++
>  server/tests/valgrind/glib.supp  |  64 
>  7 files changed, 160 insertions(+), 14 deletions(-)
>  create mode 100644 server/tests/test-record.c
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server v2] red-replay-qxl: Remove useless end of line

2018-11-16 Thread Victor Toso 
Hi,

On Fri, Nov 16, 2018 at 09:43:13AM +, Frediano Ziglio wrote:
> spice_debug already add a end of line.

s/spice_debug/Spice log functions/ ?

Just a suggestion, either way

Acked-by: Victor Toso 

> Signed-off-by: Frediano Ziglio 
> ---
>  server/display-channel.c |  2 +-
>  server/lz4-encoder.c |  2 +-
>  server/memslot.c |  8 +---
>  server/red-parse-qxl.c   | 12 ++--
>  server/red-record-qxl.c  |  2 +-
>  server/red-replay-qxl.c  |  4 ++--
>  server/reds.c|  4 ++--
>  7 files changed, 18 insertions(+), 16 deletions(-)
> 
> Changes since v1:
> - add more occurrencies
> 
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 17fa4409..118f795f 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -2421,7 +2421,7 @@ gboolean 
> display_channel_validate_surface(DisplayChannel *display, uint32_t surf
>  return FALSE;
>  }
>  if (!display->priv->surfaces[surface_id].context.canvas) {
> -spice_warning("canvas address is %p for %d (and is NULL)\n",
> +spice_warning("canvas address is %p for %d (and is NULL)",
> &(display->priv->surfaces[surface_id].context.canvas), 
> surface_id);
>  spice_warning("failed on %d", surface_id);
>  return FALSE;
> diff --git a/server/lz4-encoder.c b/server/lz4-encoder.c
> index bf8f36f5..84a9bcc1 100644
> --- a/server/lz4-encoder.c
> +++ b/server/lz4-encoder.c
> @@ -116,7 +116,7 @@ int lz4_encode(Lz4EncoderContext *lz4, int height, int 
> stride, uint8_t *io_ptr,
>  
>  LZ4_freeStream(stream);
>  if (total_lines != height) {
> -spice_error("too many lines\n");
> +spice_error("too many lines");
>  out_size = 0;
>  }
>  
> diff --git a/server/memslot.c b/server/memslot.c
> index ede77e7a..c2931321 100644
> --- a/server/memslot.c
> +++ b/server/memslot.c
> @@ -61,7 +61,7 @@ int memslot_validate_virt(RedMemSlotInfo *info, unsigned 
> long virt, int slot_id,
>  
>  if (virt < slot->virt_start_addr || (virt + add_size) > 
> slot->virt_end_addr) {
>  print_memslots(info);
> -spice_warning("virtual address out of range\n"
> +spice_warning("virtual address out of range"
>"virt=0x%lx+0x%x slot_id=%d group_id=%d\n"
>"slot=0x%lx-0x%lx delta=0x%lx",
>virt, add_size, slot_id, group_id,
> @@ -114,8 +114,10 @@ void *memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL 
> addr, uint32_t add_size
>  generation = memslot_get_generation(info, addr);
>  if (generation != slot->generation) {
>  print_memslots(info);
> -spice_critical("address generation is not valid, group_id %d, 
> slot_id %d, gen %d, slot_gen %d\n",
> -  group_id, slot_id, generation, slot->generation);
> +spice_critical("address generation is not valid, group_id %d, 
> slot_id %d, "
> +   "gen %d, slot_gen %d",
> +   group_id, slot_id,
> +   generation, slot->generation);
>  return NULL;
>  }
>  
> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> index dd840be1..86abe3ca 100644
> --- a/server/red-parse-qxl.c
> +++ b/server/red-parse-qxl.c
> @@ -137,7 +137,7 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo 
> *slots, int group_id,
>   * Or made a circular list of chunks
>   */
>  if (++num_chunks >= MAX_CHUNKS) {
> -spice_warning("data split in too many chunks, avoiding DoS\n");
> +spice_warning("data split in too many chunks, avoiding DoS");
>  goto error;
>  }
>  
> @@ -167,7 +167,7 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo 
> *slots, int group_id,
>  data_size += chunk_data_size;
>  /* this can happen if client is sending nested chunks */
>  if (data_size > MAX_DATA_CHUNK) {
> -spice_warning("too much data inside chunks, avoiding DoS\n");
> +spice_warning("too much data inside chunks, avoiding DoS");
>  goto error;
>  }
>  if (!memslot_validate_virt(slots, (intptr_t)red->data, memslot_id, 
> red->data_size, group_id))
> @@ -430,14 +430,14 @@ static bool bitmap_consistent(SpiceBitmap *bitmap)
>  unsigned int bpp;
>  
>  if (bitmap->format >= 
> SPICE_N_ELEMENTS(MAP_BITMAP_FMT_TO_BITS_PER_PIXEL)) {
> -spice_warning("wrong format specified for image\n");
> +spice_warning("wrong format specified for image");
>  return false;
>  }
>  
>  bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format];
>  
>  if (bitmap->stride < (((uint64_t) bitmap->x * bpp + 7u) / 8u)) {
> -spice_warning("image stride too small for width: %d < ((%d * %d + 7) 
> / 8) (%s=%d)\n",
> +spice_warning("image stride too small for width: %d < ((%d * %d + 7) 
> / 8) (%s=%d)",
>  bitmap->stride, bitmap->x, bpp,
>

[Spice-devel] [PATCH spice-server v2] red-replay-qxl: Remove useless end of line

2018-11-16 Thread Frediano Ziglio 
spice_debug already add a end of line.

Signed-off-by: Frediano Ziglio 
---
 server/display-channel.c |  2 +-
 server/lz4-encoder.c |  2 +-
 server/memslot.c |  8 +---
 server/red-parse-qxl.c   | 12 ++--
 server/red-record-qxl.c  |  2 +-
 server/red-replay-qxl.c  |  4 ++--
 server/reds.c|  4 ++--
 7 files changed, 18 insertions(+), 16 deletions(-)

Changes since v1:
- add more occurrencies

diff --git a/server/display-channel.c b/server/display-channel.c
index 17fa4409..118f795f 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -2421,7 +2421,7 @@ gboolean display_channel_validate_surface(DisplayChannel 
*display, uint32_t surf
 return FALSE;
 }
 if (!display->priv->surfaces[surface_id].context.canvas) {
-spice_warning("canvas address is %p for %d (and is NULL)\n",
+spice_warning("canvas address is %p for %d (and is NULL)",
&(display->priv->surfaces[surface_id].context.canvas), 
surface_id);
 spice_warning("failed on %d", surface_id);
 return FALSE;
diff --git a/server/lz4-encoder.c b/server/lz4-encoder.c
index bf8f36f5..84a9bcc1 100644
--- a/server/lz4-encoder.c
+++ b/server/lz4-encoder.c
@@ -116,7 +116,7 @@ int lz4_encode(Lz4EncoderContext *lz4, int height, int 
stride, uint8_t *io_ptr,
 
 LZ4_freeStream(stream);
 if (total_lines != height) {
-spice_error("too many lines\n");
+spice_error("too many lines");
 out_size = 0;
 }
 
diff --git a/server/memslot.c b/server/memslot.c
index ede77e7a..c2931321 100644
--- a/server/memslot.c
+++ b/server/memslot.c
@@ -61,7 +61,7 @@ int memslot_validate_virt(RedMemSlotInfo *info, unsigned long 
virt, int slot_id,
 
 if (virt < slot->virt_start_addr || (virt + add_size) > 
slot->virt_end_addr) {
 print_memslots(info);
-spice_warning("virtual address out of range\n"
+spice_warning("virtual address out of range"
   "virt=0x%lx+0x%x slot_id=%d group_id=%d\n"
   "slot=0x%lx-0x%lx delta=0x%lx",
   virt, add_size, slot_id, group_id,
@@ -114,8 +114,10 @@ void *memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL 
addr, uint32_t add_size
 generation = memslot_get_generation(info, addr);
 if (generation != slot->generation) {
 print_memslots(info);
-spice_critical("address generation is not valid, group_id %d, slot_id 
%d, gen %d, slot_gen %d\n",
-  group_id, slot_id, generation, slot->generation);
+spice_critical("address generation is not valid, group_id %d, slot_id 
%d, "
+   "gen %d, slot_gen %d",
+   group_id, slot_id,
+   generation, slot->generation);
 return NULL;
 }
 
diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index dd840be1..86abe3ca 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -137,7 +137,7 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo 
*slots, int group_id,
  * Or made a circular list of chunks
  */
 if (++num_chunks >= MAX_CHUNKS) {
-spice_warning("data split in too many chunks, avoiding DoS\n");
+spice_warning("data split in too many chunks, avoiding DoS");
 goto error;
 }
 
@@ -167,7 +167,7 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo 
*slots, int group_id,
 data_size += chunk_data_size;
 /* this can happen if client is sending nested chunks */
 if (data_size > MAX_DATA_CHUNK) {
-spice_warning("too much data inside chunks, avoiding DoS\n");
+spice_warning("too much data inside chunks, avoiding DoS");
 goto error;
 }
 if (!memslot_validate_virt(slots, (intptr_t)red->data, memslot_id, 
red->data_size, group_id))
@@ -430,14 +430,14 @@ static bool bitmap_consistent(SpiceBitmap *bitmap)
 unsigned int bpp;
 
 if (bitmap->format >= SPICE_N_ELEMENTS(MAP_BITMAP_FMT_TO_BITS_PER_PIXEL)) {
-spice_warning("wrong format specified for image\n");
+spice_warning("wrong format specified for image");
 return false;
 }
 
 bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format];
 
 if (bitmap->stride < (((uint64_t) bitmap->x * bpp + 7u) / 8u)) {
-spice_warning("image stride too small for width: %d < ((%d * %d + 7) / 
8) (%s=%d)\n",
+spice_warning("image stride too small for width: %d < ((%d * %d + 7) / 
8) (%s=%d)",
 bitmap->stride, bitmap->x, bpp,
 bitmap_format_to_string(bitmap->format),
 bitmap->format);
@@ -486,12 +486,12 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, 
int group_id,
 red->u.bitmap.stride = qxl->bitmap.stride;
 palette = qxl->bitmap.palette;
 if (!bitmap_fmt_is_rgb(red->u.bitmap.format) && !palette && !is_mask) {
-spice_warning("guest error: missing palette

Re: [Spice-devel] [PATCH] Fix typo, change DeviceName to device_name

2018-11-16 Thread Frediano Ziglio 
> 
> ---
>  vdagent/display_configuration.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/vdagent/display_configuration.h
> b/vdagent/display_configuration.h
> index ef4f989..7e2f8ac 100644
> --- a/vdagent/display_configuration.h
> +++ b/vdagent/display_configuration.h
> @@ -56,7 +56,7 @@ public:
>  bool query_display_config();
>  bool set_display_config(LONG & error);
>  bool update_mode_position(LPCTSTR device_name, DEVMODE* dev_mode);
> -bool update_mode_size(LPCTSTR DeviceNmae, DEVMODE* dev_mode);
> +bool update_mode_size(LPCTSTR device_name, DEVMODE* dev_mode);
>  void update_detached_primary_state(LPCTSTR device_name,
>  DISPLAYCONFIG_PATH_INFO * path_info);
>  bool set_path_state(LPCTSTR device_name, MONITOR_STATE state);
>  bool is_attached(LPCTSTR device_name);
> @@ -172,4 +172,4 @@ private:
>  CCD _ccd;
>  };
>  
> -#endif
> \ No newline at end of file
> +#endif

The patch is good however your e-mail client entirely screw up your name and
we usually add the "Signed-off-by" line.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH] x11-randr: Fix typo in error message

2018-11-16 Thread Frediano Ziglio 
> 
> Hi,
> 
> On Thu, Nov 15, 2018 at 10:04:49PM +, Frediano Ziglio wrote:
> > The function name is XRRGetScreenSizeRange not RRGetScreenSizeRange.
> > 
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  src/vdagent/x11-randr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
> > index c5fba51..be6a43a 100644
> > --- a/src/vdagent/x11-randr.c
> > +++ b/src/vdagent/x11-randr.c
> > @@ -120,7 +120,7 @@ static void update_randr_res(struct vdagent_x11 *x11,
> > int poll)
> >&x11->randr.min_height,
> >&x11->randr.max_width,
> >&x11->randr.max_height) != 1) {
> > -syslog(LOG_ERR, "update_randr_res: RRGetScreenSizeRange failed");
> > +syslog(LOG_ERR, "update_randr_res: XRRGetScreenSizeRange failed");
> 
> That's correct. Out of curiosity, have you seen this error
> message in the logs?
> 

No, I was just looking at the code. It would have been great to understand
where Wayland is failing.

> Acked-by: Victor Toso 
> 
> >  }
> >  }
> >  

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH] Fix typo, change DeviceName to device_name

2018-11-16 Thread ? ?? 
---
 vdagent/display_configuration.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vdagent/display_configuration.h b/vdagent/display_configuration.h
index ef4f989..7e2f8ac 100644
--- a/vdagent/display_configuration.h
+++ b/vdagent/display_configuration.h
@@ -56,7 +56,7 @@ public:
 bool query_display_config();
 bool set_display_config(LONG & error);
 bool update_mode_position(LPCTSTR device_name, DEVMODE* dev_mode);
-bool update_mode_size(LPCTSTR DeviceNmae, DEVMODE* dev_mode);
+bool update_mode_size(LPCTSTR device_name, DEVMODE* dev_mode);
 void update_detached_primary_state(LPCTSTR device_name, 
DISPLAYCONFIG_PATH_INFO * path_info);
 bool set_path_state(LPCTSTR device_name, MONITOR_STATE state);
 bool is_attached(LPCTSTR device_name);
@@ -172,4 +172,4 @@ private:
 CCD _ccd;
 };
 
-#endif
\ No newline at end of file
+#endif
-- 
2.14.2.windows.2

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [linux/vd-agent] vdagent: Do not send empty screen resolution messages

2018-11-16 Thread Victor Toso 
Hi,

On Thu, Nov 15, 2018 at 03:33:17PM -0500, Frediano Ziglio wrote:
> > 
> > From: Victor Toso 
> > 
> > Easier to trigger on Wayland guest by running
> > 
> > > xrandr --output XWAYLAND0 --rotate left
> > 
> > In current master, this causes the spice-vdagentd to disconnect from
> > the client. In 0.18 branch (latest release), mouse becomes unusable as
> > mentioned in the referred bug below.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1641723
> > 
> > Signed-off-by: Victor Toso 
> > ---
> >  src/vdagent/x11-randr.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
> > index c5fba51..81a2cd6 100644
> > --- a/src/vdagent/x11-randr.c
> > +++ b/src/vdagent/x11-randr.c
> > @@ -959,6 +959,12 @@ no_info:
> >  }
> >  }
> >  
> > +if (screen_count == 0) {
> > +syslog(LOG_DEBUG, "Screen count is zero, are we on wayland?");
> > +g_free(res);
> > +return;
> > +}
> > +
> >  if (x11->debug) {
> >  syslog(LOG_DEBUG, "Sending guest screen resolutions to vdagentd:");
> >  for (i = 0; i < screen_count; i++) {
> 
> Sounds reasonable.
> 
> Acked-by: Frediano Ziglio 

Thanks,

> On the other hand looks like:
> - a bug in Wayland X11 emulation. Why Xrandr function fails?

It works fine with common resize of display. It fails with
rotate, maybe that's specific case. Don't really know.

> - partial. What happens to screen size? Does the mouse is limited
>   in a different are (can you move it all around the screen?)
>   Or maybe later another event with correct values arrives?

No other event. The logs show that when resizing, we get a log
with the correct values. When I run the xrandr command as pointed
in the commit log, the size of the message between session and
system agents are zero, because screen_count is zero. I saw that
this was not considered in the code and avoiding sending the
message seems enough.

Thanks again for the review,
Victor


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] need help explaining the algorithm

2018-11-16 Thread Victor Toso 
Hi,

On Fri, Nov 09, 2018 at 05:08:21PM +0800, Zihao Miao wrote:
> Hi, list:
> This is my first post to a mail list.. so..

Welcome.

> I am a college student and I am very interested in the
> algorithm that maintains the current ring of drawables to
> reduce the display data transfer.

> I currently read the code through the  trace of
> disolay_channel_process_draw->display_channel_add_drawable->current_add->exclude_region->__exclude_region,

heh, "TODO: What is the intended use of this function?"
https://gitlab.freedesktop.org/spice/spice/blob/master/server/display-channel.c#L764

> and gained some understanding of the code.
> However, I still don't quite get the concept of the difference
> between a draw item and a container?

> And there are questions like why current_add_with_shadow is
> much more complicated than the current_add; what

https://gitlab.freedesktop.org/spice/spice/blob/master/server/display-channel.c#L1424

> did the pip actually send? a drawable or the whole surface?
> where do compression come in?...

So many questions. I'm not really familiar with this code tbh so
I'd have to study myself to reply.

> Do you guys have any resources besides the newbee and spice
> protocol document to learn about this algorithm?

Not really, I trust more the comments and git history.

> Best wishes!
> Zihao Miao

Sorry that I could not help much right now but I hate to see this
kind of email going on not replied for so long. We welcome better
documentation, please do feel free to send patches with your
understanding as documentation. It might trigger the discussions
that you want.

Cheers,
Victor


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] x11-randr: Fix typo in error message

2018-11-16 Thread Victor Toso 
Hi,

On Thu, Nov 15, 2018 at 10:04:49PM +, Frediano Ziglio wrote:
> The function name is XRRGetScreenSizeRange not RRGetScreenSizeRange.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  src/vdagent/x11-randr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
> index c5fba51..be6a43a 100644
> --- a/src/vdagent/x11-randr.c
> +++ b/src/vdagent/x11-randr.c
> @@ -120,7 +120,7 @@ static void update_randr_res(struct vdagent_x11 *x11, int 
> poll)
>&x11->randr.min_height,
>&x11->randr.max_width,
>&x11->randr.max_height) != 1) {
> -syslog(LOG_ERR, "update_randr_res: RRGetScreenSizeRange failed");
> +syslog(LOG_ERR, "update_randr_res: XRRGetScreenSizeRange failed");

That's correct. Out of curiosity, have you seen this error
message in the logs?

Acked-by: Victor Toso 

>  }
>  }
>  
> -- 
> 2.17.2
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


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-server] red-replay-qxl: Remove useless end of line

2018-11-16 Thread Victor Toso 
Hi,

On Fri, Nov 02, 2018 at 09:36:06AM +, Frediano Ziglio wrote:
> spice_debug already add a end of line.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  server/red-replay-qxl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
> index bd33b581..6958a495 100644
> --- a/server/red-replay-qxl.c
> +++ b/server/red-replay-qxl.c
> @@ -1289,7 +1289,7 @@ static void replay_handle_dev_input(QXLWorker *worker, 
> SpiceReplay *replay,
>  // safe to ignore
>  break;
>  default:
> -spice_debug("unhandled %d\n", message);
> +spice_debug("unhandled %d", message);
>  }
>  }
>  
> @@ -1321,7 +1321,7 @@ SPICE_GNUC_VISIBLE QXLCommandExt* 
> spice_replay_next_cmd(SpiceReplay *replay,
>  cmd = replay_malloc0(replay, sizeof(QXLCommandExt));
>  cmd->cmd.type = type;
>  cmd->group_id = 0;
> -spice_debug("command %"SCNu64", %d\r", timestamp, cmd->cmd.type);
> +spice_debug("command %"SCNu64", %d", timestamp, cmd->cmd.type);
>  switch (cmd->cmd.type) {
>  case QXL_CMD_DRAW:
>  cmd->flags = 0;

Maybe a few more in the same patch/series? Quick check (not
strictly correct)

 (master 34a44d3e) $ grep -rniI "\\\n" server/*.c | grep -i spice
  server/display-channel.c:2424:spice_warning("canvas address is %p for 
%d (and is NULL)\n",
  server/lz4-encoder.c:119:spice_error("too many lines\n");
  server/memslot.c:64:spice_warning("virtual address out of range\n"
  server/memslot.c:117:spice_critical("address generation is not valid, 
group_id %d, slot_id %d, gen %d, slot_gen %d\n",
  server/red-parse-qxl.c:140:spice_warning("data split in too many 
chunks, avoiding DoS\n");
  server/red-parse-qxl.c:170:spice_warning("too much data inside 
chunks, avoiding DoS\n");
  server/red-parse-qxl.c:433:spice_warning("wrong format specified for 
image\n");
  server/red-parse-qxl.c:440:spice_warning("image stride too small for 
width: %d < ((%d * %d + 7) / 8) (%s=%d)\n",
  server/red-parse-qxl.c:489:spice_warning("guest error: missing 
palette on bitmap format=%d\n",
  server/red-parse-qxl.c:494:spice_warning("guest error: zero area 
bitmap\n");
  server/red-record-qxl.c:835:static const char header[] = "SPICE_REPLAY 
1\n";
  server/red-record-qxl.c:843:spice_error("failed to open recording 
file %s\n", filename);
  server/red-replay-qxl.c:1292:spice_debug("unhandled %d\n", message);
  server/red-replay-qxl.c:1424:if (fscanf(file, "SPICE_REPLAY %u\n", 
&version) == 1) {
  server/reds.c:545:spice_debug("Exiting server because of client 
disconnect.\n");
  server/reds.c:4358: spice_debug("QXLInterface::client_monitors_config 
failed\n");

Cheers,


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-gtk v2] channel-display-mjpeg: Fix encoding for big endian machines

2018-11-16 Thread Victor Toso 
Hi,

On Thu, Nov 15, 2018 at 03:22:04PM -0500, Frediano Ziglio wrote:
> > 
> > On Fri, Nov 02, 2018 at 09:38:12AM +, Frediano Ziglio wrote:
> > > Make sure components are ordered in the same way in memory.
> > > This was tested with a virtual MIPS machine.
> > > 
> > > Signed-off-by: Frediano Ziglio 
> > > ---
> > >  src/channel-display-mjpeg.c | 4 
> > >  1 file changed, 4 insertions(+)
> > > 
> > > Changes since v1:
> > > - fix typo in commit message title
> > > 
> > > diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
> > > index 83cd391b..94e56205 100644
> > > --- a/src/channel-display-mjpeg.c
> > > +++ b/src/channel-display-mjpeg.c
> > > @@ -108,7 +108,11 @@ static gboolean mjpeg_decoder_decode_frame(gpointer
> > > video_decoder)
> > >  
> > >  #ifdef JCS_EXTENSIONS
> > >  // requires jpeg-turbo
> > > +#  if SPICE_ENDIAN == SPICE_ENDIAN_LITTLE
> > >  decoder->mjpeg_cinfo.out_color_space = JCS_EXT_BGRX;
> > > +#  else
> > > +decoder->mjpeg_cinfo.out_color_space = JCS_EXT_XRGB;
> > > +#  endif
> > ^^
> > Why the spaces
> > 
> 
> indentation.

I got that. I just don't see elsewhere in the code. I don't
really mind.

> Maybe something like:
> 
> if (SPICE_ENDIAN == SPICE_ENDIAN_LITTLE) {
> decoder->mjpeg_cinfo.out_color_space = JCS_EXT_BGRX;
> } else {
> decoder->mjpeg_cinfo.out_color_space = JCS_EXT_XRGB;
> }
> 
> would be better. It looks like less efficient but if the compiler
> cannot turn this to just an assignment I would consider more a bug in
> the compiler. Also this form has the advantage to compile all code
> (big endian machines are neither common nor much tested).

I'd say to stick with v1. Don't recall the last time I saw
someone discussing big endian clients.

Cheers,

Acked-by: Victor Toso 

> > >  #else
> > >  #warning "You should consider building with libjpeg-turbo"
> > >  decoder->mjpeg_cinfo.out_color_space = JCS_RGB;
> 
> Frediano


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] udscs: Avoid file descriptor leak

2018-11-16 Thread Victor Toso 
Hi,

On Thu, Nov 15, 2018 at 10:06:57PM +, Frediano Ziglio wrote:
> If connection fails the socket descriptor is not closed causing
> a leak.
> 
> Signed-off-by: Frediano Ziglio 

Acked-by: Victor Toso 

> ---
>  src/udscs.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/udscs.c b/src/udscs.c
> index 62abc97..7fe74b9 100644
> --- a/src/udscs.c
> +++ b/src/udscs.c
> @@ -119,6 +119,7 @@ struct udscs_connection *udscs_connect(const char 
> *socketname,
>  if (conn->debug) {
>  syslog(LOG_DEBUG, "connect %s: %m", socketname);
>  }
> +close(conn->fd);
>  g_free(conn);
>  return NULL;
>  }
> -- 
> 2.17.2
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel