Re: [Spice-devel] [spice-gtk] clipboard: don't request targets without owner on X11

[Victor Toso]

On Sun, 2019-01-27 at 18:14 +0100, jjanku at redhat.com wrote:
> On X11, if the owner in GdkEventOwnerChange is set to NULL,
> it means there's no data in the clipboard, so it's pointless to
> request targets as the request will fail anyway.
> 
> On Wayland, owner is always NULL, so don't do anything there.
> 
> Signed-off-by: Jakub Janků 
> ---
>  src/spice-gtk-session.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 1a19bca..b48f92a 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -674,6 +674,14 @@ static void
> clipboard_owner_change(GtkClipboard*clipboard,
>  }
>  
>  s->clipboard_by_guest[selection] = FALSE;
> +
> +#ifdef GDK_WINDOWING_X11
> +if (!event->owner &&
> GDK_IS_X11_DISPLAY(gdk_display_get_default())) {
> +s->clip_hasdata[selection] = FALSE;
> +return;
> +}
> +#endif
> +
>  s->clip_hasdata[selection] = TRUE;
>  if (s->auto_clipboard_enable && !read_only(self))
>  gtk_clipboard_request_targets(clipboard,
> clipboard_get_targets,

I plan to push this later today if no one complains.

Cheers,

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

Re: [Spice-devel] [PATCH] spec: call semanage in posttrans not in post

[Christophe Fergeau]

On Tue, Feb 05, 2019 at 01:24:38PM -0500, Frediano Ziglio wrote:
> > On Tue, Feb 05, 2019 at 09:30:39AM -0500, Frediano Ziglio wrote:
> > > > 
> > > > It can happen that selinux-policy (targeted) is installed only after
> > > > spice-streaming-agent (upon system installation). In that case
> > > > running semanage in post scriptlet will fail.
> > > > 
> > > > In posttrans all packages are already installed, so it should be
> > > > safe to call semanage at that point.
> > > > 
> > > > rhbz#1647789
> > > > 
> > > > Signed-off-by: Uri Lublin 
> > > > ---
> > > > 
> > > > In a first patch I wrote I also added a condition that
> > > > checks if selinuxenabled. If people feel it's better
> > > > I'll send a V2 with it.
> > > > 
> > > 
> > > I see no reason why adding to selinux-policy should be a stopover
> > > for this fix in the meanwhile.
> > > 
> > > Acked-by: Frediano Ziglio 
> > 
> > Ensuring that a bug is fixed in the right place, and explaining this in
> > the commit log should be a stopper though.
> > 
> > Christophe
> > 
> 
> It's not clear what you are suggesting.
> Adding a sentence in the commit message?

Did we file a selinux bug asking for this addition?
If yes, where is this bug?
And yes, if the right fix is for this hypothetical bug to be fixed, then
this should be explained in the commit log.

However, I think spice-streaming-agent is not yet available in Fedora ?
In which case it would be too early for the aforementioned bug I think
:-/

Christophe


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-common 1/2] log: remove deprecated SPICE_ABORT_LEVEL support

[Christophe Fergeau]

On Tue, Jan 29, 2019 at 10:49:23AM +, Frediano Ziglio wrote:
> This feature was marked obsolete by efd1d3cb4d8eee more than
> three years ago.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  common/log.c | 30 +-
>  tests/test-logging.c | 39 +--
>  2 files changed, 2 insertions(+), 67 deletions(-)
> 
> diff --git a/common/log.c b/common/log.c
> index a7806c5..5819974 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -33,11 +33,7 @@
>  #include "backtrace.h"
>  
>  static int glib_debug_level = INT_MAX;
> -static int abort_mask = 0;
> -
> -#ifndef SPICE_ABORT_MASK_DEFAULT
> -#define SPICE_ABORT_MASK_DEFAULT (G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_ERROR)
> -#endif
> +static const int abort_mask = G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_ERROR;

after this commit, abort_mask is used in spice_logv:

  g_log(log_domain, log_level, "%s", log_msg->str);

  if ((abort_mask & log_level) != 0) {
  spice_backtrace();
  abort();
  }

This allows to keep spice_critical() behaviour which aborts contrary to
g_critical() which does not abort by default. I'm quite sure that
G_LOG_LEVEL_ERROR will cause g_log() to abort.
In other words, I think you could get rid of abort_mask and just use:
  if ((log_level & G_LOG_LEVEL_CRITICAL) != 0) {
  spice_backtrace();
  abort();
  }

Looks good even without this change,

Acked-by: Christophe Fergeau 



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-common 2/2] log: remove deprecated SPICE_DEBUG_LEVEL support

[Christophe Fergeau]

On Tue, Jan 29, 2019 at 10:49:24AM +, Frediano Ziglio wrote:
>  SPICE_CONSTRUCTOR_FUNC(spice_log_init)
>  {
> -
> -spice_log_set_debug_level();
> -if (glib_debug_level != INT_MAX) {
> -/* If SPICE_DEBUG_LEVEL is set, we need a custom handler, which is
> - * going to break use of g_log_set_default_handler() by apps
> - */
> -g_log_set_handler(G_LOG_DOMAIN,
> -  G_LOG_LEVEL_MASK | G_LOG_FLAG_FATAL | 
> G_LOG_FLAG_RECURSION,
> -  spice_logger, NULL);
> -}
>  /* Threading is always enabled from 2.31.0 onwards */
>  /* Our logging is potentially used from different threads.
>   * Older glibs require that g_thread_init() is called when

After this commit, the full function is:
 SPICE_CONSTRUCTOR_FUNC(spice_log_init)
  {
  /* Threading is always enabled from 2.31.0 onwards */
  /* Our logging is potentially used from different threads.
   * Older glibs require that g_thread_init() is called when
   * doing that. */
  #if !GLIB_CHECK_VERSION(2, 31, 0)
  if (!g_thread_supported())
  g_thread_init(NULL);
  #endif
  }

and spice-common/spice-gtk/spice-server all require a newer version of
glib than 2.31 so this function can be removed in a follow-up commit.

Acked-by: Christophe Fergeau 



> @@ -140,10 +58,6 @@ static void spice_logv(const char *log_domain,
>  {
>  GString *log_msg;
>  
> -if ((log_level & G_LOG_LEVEL_MASK) > glib_debug_level) {
> -return; // do not print anything
> -}
> -
>  log_msg = g_string_new(NULL);
>  if (strloc && function) {
>  g_string_append_printf(log_msg, "%s:%s: ", strloc, function);
> diff --git a/tests/test-logging.c b/tests/test-logging.c
> index 3b17f44..ff2d8bd 100644
> --- a/tests/test-logging.c
> +++ b/tests/test-logging.c
> @@ -217,71 +217,6 @@ static void test_log_levels(void)
>  g_test_trap_assert_stdout_unmatched("*other_debug*");
>  }
>  
> -/* Checks that SPICE_DEBUG_LEVEL impacts spice_debug(), g_debug() but not 
> other_debug() */
> -static void test_spice_debug_level(void)
> -{
> -if (g_test_subprocess()) {
> -/* g_test_expected_message only checks whether the appropriate 
> messages got up to g_log()
> - * The following calls will be caught by the parent process to check 
> what was (not) printed
> - * to stdout/stderr
> - */
> -spice_info("spice_info");
> -g_debug("g_debug");
> -spice_debug("spice_debug");
> -other_debug("other_debug");
> -
> -return;
> -}
> -
> -g_unsetenv("G_MESSAGES_DEBUG");
> -g_setenv("SPICE_DEBUG_LEVEL", "5", TRUE);
> -g_test_trap_subprocess(NULL, 0, 0);
> -g_unsetenv("SPICE_DEBUG_LEVEL");
> -g_test_trap_assert_passed();
> -g_test_trap_assert_stderr("*SPICE_DEBUG_LEVEL*deprecated*");
> -g_test_trap_assert_stdout("*spice_info\n*g_debug\n*spice_debug\n");
> -g_test_trap_assert_stdout_unmatched("*other_debug*");
> -}
> -
> -/* Checks that raising SPICE_DEBUG_LEVEL allows to only show spice_warning() 
> and spice_critical()
> - * messages, as well as g_warning() and g_critical(), but does not impact 
> other_message()
> - */
> -static void test_spice_debug_level_warning(void)
> -{
> -if (g_test_subprocess()) {
> -spice_info("spice_info");
> -spice_debug("spice_debug");
> -spice_warning("spice_warning");
> -g_debug("g_debug");
> -g_info("g_info");
> -g_message("g_message");
> -g_warning("g_warning");
> -g_critical("g_critical");
> -other_debug("other_debug");
> -other_info("other_info");
> -other_message("other_message");
> -other_warning("other_warning");
> -other_critical("other_critical");
> -
> -return;
> -}
> -
> -g_setenv("SPICE_DEBUG_LEVEL", "1", TRUE);
> -g_test_trap_subprocess(NULL, 0, 0);
> -g_unsetenv("SPICE_DEBUG_LEVEL");
> -g_test_trap_assert_passed();
> -g_test_trap_assert_stderr("*SPICE_DEBUG_LEVEL*deprecated*");
> -
> g_test_trap_assert_stderr("*g_critical\n*other_message\n*other_warning\n*other_critical\n");
> -g_test_trap_assert_stdout_unmatched("*spice_info*");
> -g_test_trap_assert_stdout_unmatched("*spice_debug*");
> -g_test_trap_assert_stderr_unmatched("*spice_warning*");
> -g_test_trap_assert_stdout_unmatched("*g_debug*");
> -g_test_trap_assert_stdout_unmatched("*g_info*");
> -g_test_trap_assert_stderr_unmatched("*g_message*");
> -g_test_trap_assert_stderr_unmatched("*g_warning*");
> -g_test_trap_assert_stdout_unmatched("*other_info*");
> -}
> -
>  /* Checks that setting G_MESSAGES_DEBUG to 'Spice' impacts spice_debug() and
>   * g_debug() but not other_debug() */
>  static void test_spice_g_messages_debug(void)
> @@ -358,8 +293,6 @@ int main(int argc, char **argv)
>  g_log_set_always_fatal(fatal_mask & G_LOG_LEVEL_MASK);
>  
>  #if GLIB_CHECK_VERSION(2,38,0)
> -g_

[Spice-devel] [spice-server] doc: Document G_MESSAGES_DEBUG use

[Christophe Fergeau]

Explain how to get more verbose logs out of spice-server

Signed-off-by: Christophe Fergeau 
---
 docs/manual/manual.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/manual/manual.txt b/docs/manual/manual.txt
index 99f6a5c84..312df8c5c 100644
--- a/docs/manual/manual.txt
+++ b/docs/manual/manual.txt
@@ -1203,10 +1203,19 @@ Server side
 
 If the virtual machine was started using QEMU directly, SPICE server logs will 
be output to
 your console stdout.
+
 When using libvirt, logs are located in `/var/log/libvirt/qemu/` for the qemu
 system instance (`qemu:///system`), and in `~/.cache/libvirt/qemu/log` for the
 qemu session instance (`qemu:///session`).
 
+It's possible to get more verbose output by setting the `G_MESSAGES_DEBUG`
+environment variable to `Spice` or `all` before starting QEMU.
+
+When using libvirt, the environment variable needs to be set from the XML
+domain definition as described in
+https://libvirt.org/drvqemu.html#qemucommand[libvirt documentation].
+
+
 Client side
 ---
 
-- 
2.20.1

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

Re: [Spice-devel] [spice-server v2] ssl: Dump OpenSSL error stack on errors

[Uri Lublin]

On 1/14/19 12:59 PM, Christophe Fergeau wrote:

Bugs such as https://bugzilla.redhat.com/show_bug.cgi?id=1651882 can be
quite tricky to figure out without the detailed OpenSSL error. This
commit adds a detailed dump of the OpenSSL error stack when an OpenSSL
failure happens.

In the bug above, this would have displayed:
(process:13154): Spice-WARNING **: 05:43:10.139: reds.c:2816:reds_init_ssl: 
Could not load certificates from /etc/pki/libvirt-spice/server-cert.pem

(process:13154): Spice-WARNING **: 05:43:10.140: error:140AB18F:SSL 
routines:SSL_CTX_use_certificate:ee key too small

Signed-off-by: Christophe Fergeau 


Acked-by: Uri Lublin 

Uri.


---
Changes since v1:
- forgot to squash 2 changes in the previous patch


  server/red-stream.c |  2 +-
  server/reds.c   | 20 +++-
  server/utils.c  | 14 ++
  server/utils.h  |  2 ++
  4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/server/red-stream.c b/server/red-stream.c
index fd5b8cd12..0baa7eb88 100644
--- a/server/red-stream.c
+++ b/server/red-stream.c
@@ -498,7 +498,7 @@ RedStreamSslStatus red_stream_ssl_accept(RedStream *stream)
  }
  }
  
-ERR_print_errors_fp(stderr);

+red_dump_openssl_errors();
  spice_warning("SSL_accept failed, error=%d", ssl_error);
  SSL_free(stream->priv->ssl);
  stream->priv->ssl = NULL;
diff --git a/server/reds.c b/server/reds.c
index 221989266..38e8cc2d2 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1575,11 +1575,13 @@ static bool reds_send_link_ack(RedsState *reds, 
RedLinkInfo *link)
  || !red_link_info_test_capability(link, SPICE_COMMON_CAP_AUTH_SASL)) {
  if (!(link->tiTicketing.rsa = RSA_new())) {
  spice_warning("RSA new failed");
+red_dump_openssl_errors();
  return FALSE;
  }
  
  if (!(bio = BIO_new(BIO_s_mem( {

  spice_warning("BIO new failed");
+red_dump_openssl_errors();
  return FALSE;
  }
  
@@ -1587,9 +1589,9 @@ static bool reds_send_link_ack(RedsState *reds, RedLinkInfo *link)

  SPICE_TICKET_KEY_PAIR_LENGTH,
  link->tiTicketing.bn,
  NULL) != 1) {
-spice_warning("Failed to generate %d bits RSA key: %s",
-  SPICE_TICKET_KEY_PAIR_LENGTH,
-  ERR_error_string(ERR_get_error(), NULL));
+spice_warning("Failed to generate %d bits RSA key",
+  SPICE_TICKET_KEY_PAIR_LENGTH);
+red_dump_openssl_errors();
  goto end;
  }
  link->tiTicketing.rsa_size = RSA_size(link->tiTicketing.rsa);
@@ -1889,6 +1891,7 @@ static void openssl_init(RedLinkInfo *link)
  link->tiTicketing.bn = BN_new();
  
  if (!link->tiTicketing.bn) {

+red_dump_openssl_errors();
  spice_error("OpenSSL BIGNUMS alloc failed");
  }
  
@@ -2087,8 +2090,8 @@ static void reds_handle_ticket(void *opaque)

  link->tiTicketing.rsa,
  RSA_PKCS1_OAEP_PADDING);
  if (password_size == -1) {
-spice_warning("failed to decrypt RSA encrypted password: %s",
-  ERR_error_string(ERR_get_error(), NULL));
+spice_warning("failed to decrypt RSA encrypted password");
+red_dump_openssl_errors();
  goto error;
  }
  password[password_size] = '\0';
@@ -2687,6 +2690,7 @@ static int load_dh_params(SSL_CTX *ctx, char *file)
  
  if ((bio = BIO_new_file(file, "r")) == NULL) {

  spice_warning("Could not open DH file");
+red_dump_openssl_errors();
  return -1;
  }
  
@@ -2694,12 +2698,14 @@ static int load_dh_params(SSL_CTX *ctx, char *file)

  BIO_free(bio);
  if (ret == 0) {
  spice_warning("Could not read DH params");
+red_dump_openssl_errors();
  return -1;
  }
  
  
  if (SSL_CTX_set_tmp_dh(ctx, ret) < 0) {

  spice_warning("Could not set DH params");
+red_dump_openssl_errors();
  return -1;
  }
  
@@ -2747,6 +2753,7 @@ static void openssl_thread_setup(void)

   * don't do it twice to avoid possible races.
   */
  if (CRYPTO_get_locking_callback() != NULL) {
+red_dump_openssl_errors();
  return;
  }
  
@@ -2794,6 +2801,7 @@ static int reds_init_ssl(RedsState *reds)

  reds->ctx = SSL_CTX_new(ssl_method);
  if (!reds->ctx) {
  spice_warning("Could not allocate new SSL context");
+red_dump_openssl_errors();
  return -1;
  }
  
@@ -2806,6 +2814,7 @@ static int reds_init_ssl(RedsState *reds)

  spice_debug("Loaded certificates from %s", 
reds->config->ssl_parameters.certs_file);
  } else {
  spice_warning("Could not load certificates from %s", 
reds->config->ssl_parameters.certs_file);
+re

Re: [Spice-devel] [PATCH spice-common 1/2] log: remove deprecated SPICE_ABORT_LEVEL support

[Frediano Ziglio]

> On Tue, Jan 29, 2019 at 10:49:23AM +, Frediano Ziglio wrote:
> > This feature was marked obsolete by efd1d3cb4d8eee more than
> > three years ago.
> > 
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  common/log.c | 30 +-
> >  tests/test-logging.c | 39 +--
> >  2 files changed, 2 insertions(+), 67 deletions(-)
> > 
> > diff --git a/common/log.c b/common/log.c
> > index a7806c5..5819974 100644
> > --- a/common/log.c
> > +++ b/common/log.c
> > @@ -33,11 +33,7 @@
> >  #include "backtrace.h"
> >  
> >  static int glib_debug_level = INT_MAX;
> > -static int abort_mask = 0;
> > -
> > -#ifndef SPICE_ABORT_MASK_DEFAULT
> > -#define SPICE_ABORT_MASK_DEFAULT (G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_ERROR)
> > -#endif
> > +static const int abort_mask = G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_ERROR;
> 
> after this commit, abort_mask is used in spice_logv:
> 
>   g_log(log_domain, log_level, "%s", log_msg->str);
> 
>   if ((abort_mask & log_level) != 0) {
>   spice_backtrace();
>   abort();
>   }
> 
> This allows to keep spice_critical() behaviour which aborts contrary to
> g_critical() which does not abort by default. I'm quite sure that
> G_LOG_LEVEL_ERROR will cause g_log() to abort.
> In other words, I think you could get rid of abort_mask and just use:
>   if ((log_level & G_LOG_LEVEL_CRITICAL) != 0) {
>   spice_backtrace();
>   abort();
>   }

Sure, maybe the abort output would be a bit different but I agree

> 
> Looks good even without this change,
> 
> Acked-by: Christophe Fergeau 
> 
> 

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

[Spice-devel] [PATCH spice-common] Obsolete Glib cleanup

[Frediano Ziglio]

We require at least GLib 2.38, remove code and check to
support earlier versions.

Signed-off-by: Frediano Ziglio 
---
 common/log.c | 8 
 tests/test-logging.c | 4 
 2 files changed, 12 deletions(-)

diff --git a/common/log.c b/common/log.c
index f9cdd60..054fd7f 100644
--- a/common/log.c
+++ b/common/log.c
@@ -36,14 +36,6 @@
 
 SPICE_CONSTRUCTOR_FUNC(spice_log_init)
 {
-/* Threading is always enabled from 2.31.0 onwards */
-/* Our logging is potentially used from different threads.
- * Older glibs require that g_thread_init() is called when
- * doing that. */
-#if !GLIB_CHECK_VERSION(2, 31, 0)
-if (!g_thread_supported())
-g_thread_init(NULL);
-#endif
 recorder_dump_on_common_signals(0, 0);
 }
 
diff --git a/tests/test-logging.c b/tests/test-logging.c
index ff2d8bd..6a79ca9 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -43,7 +43,6 @@ LOG_OTHER_HELPER(message, MESSAGE)
 LOG_OTHER_HELPER(warning, WARNING)
 LOG_OTHER_HELPER(critical, CRITICAL)
 
-#if GLIB_CHECK_VERSION(2,38,0)
 /* Checks that spice_warning() aborts after setting G_DEBUG=fatal-warnings */
 static void test_spice_fatal_warning(void)
 {
@@ -268,7 +267,6 @@ static void test_spice_g_messages_debug_all(void)
 
g_test_trap_assert_stdout("*spice_debug\n*spice_info\n*g_debug\n*g_info\n*other_debug\n*other_info\n");
 g_test_trap_assert_stderr("*g_message\n*other_message\n");
 }
-#endif /* GLIB_CHECK_VERSION(2,38,0) */
 
 static void handle_sigabrt(int sig G_GNUC_UNUSED)
 {
@@ -292,7 +290,6 @@ int main(int argc, char **argv)
  * test cases are going to test */
 g_log_set_always_fatal(fatal_mask & G_LOG_LEVEL_MASK);
 
-#if GLIB_CHECK_VERSION(2,38,0)
 g_test_add_func("/spice-common/spice-g-messages-debug", 
test_spice_g_messages_debug);
 g_test_add_func("/spice-common/spice-g-messages-debug-all", 
test_spice_g_messages_debug_all);
 g_test_add_func("/spice-common/spice-log-levels", test_log_levels);
@@ -302,7 +299,6 @@ int main(int argc, char **argv)
 g_test_add_func("/spice-common/spice-fatal-return-if-fail", 
test_spice_fatal_return_if_fail);
 g_test_add_func("/spice-common/spice-non-fatal-greturn-if-fail", 
test_spice_non_fatal_g_return_if_fail);
 g_test_add_func("/spice-common/spice-fatal-warning", 
test_spice_fatal_warning);
-#endif /* GLIB_CHECK_VERSION(2,38,0) */
 
 return g_test_run();
 }
-- 
2.20.1

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

Re: [Spice-devel] [spice-server] doc: Document G_MESSAGES_DEBUG use

[Frediano Ziglio]

> Explain how to get more verbose logs out of spice-server
> 
> Signed-off-by: Christophe Fergeau 
> ---
>  docs/manual/manual.txt | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/docs/manual/manual.txt b/docs/manual/manual.txt
> index 99f6a5c84..312df8c5c 100644
> --- a/docs/manual/manual.txt
> +++ b/docs/manual/manual.txt
> @@ -1203,10 +1203,19 @@ Server side
>  
>  If the virtual machine was started using QEMU directly, SPICE server logs
>  will be output to
>  your console stdout.
> +
>  When using libvirt, logs are located in `/var/log/libvirt/qemu/` for the
>  qemu
>  system instance (`qemu:///system`), and in `~/.cache/libvirt/qemu/log` for
>  the
>  qemu session instance (`qemu:///session`).
>  
> +It's possible to get more verbose output by setting the `G_MESSAGES_DEBUG`
> +environment variable to `Spice` or `all` before starting QEMU.
> +

Would it be useful to put a link to GLib documentation about this
setting?

> +When using libvirt, the environment variable needs to be set from the XML
> +domain definition as described in
> +https://libvirt.org/drvqemu.html#qemucommand[libvirt documentation].
> +
> +
>  Client side
>  ---
>  

Otherwise,
   Acked-by: Frediano Ziglio 


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

[Spice-devel] [PATCH spice-server v4 09/20] net-utils: Use socket compatibility layer

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/net-utils.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/server/net-utils.c b/server/net-utils.c
index 802509a4..ad66a328 100644
--- a/server/net-utils.c
+++ b/server/net-utils.c
@@ -35,6 +35,7 @@
 #include 
 
 #include "net-utils.h"
+#include "sys-socket.h"
 
 /**
  * red_socket_set_keepalive:
@@ -101,6 +102,7 @@ bool red_socket_set_no_delay(int fd, bool no_delay)
  */
 bool red_socket_set_non_blocking(int fd, bool non_blocking)
 {
+#ifndef _WIN32
 int flags;
 
 if ((flags = fcntl(fd, F_GETFL)) == -1) {
@@ -120,6 +122,15 @@ bool red_socket_set_non_blocking(int fd, bool non_blocking)
 }
 
 return true;
+#else
+u_long ioctl_nonblocking = 1;
+
+if (ioctlsocket(fd, FIONBIO, &ioctl_nonblocking) != 0) {
+spice_warning("ioctlsocket(FIONBIO) failed, %d", WSAGetLastError());
+return false;
+}
+return true;
+#endif
 }
 
 /**
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 08/20] sys-socket: Add socket_newpair utility

[Frediano Ziglio]

Allows to easier port socketpair.
Windows does not have this function, we need to create a pair
using 2 internet sockets and connecting one to the other.

Signed-off-by: Frediano Ziglio 
---
 server/sys-socket.c | 75 +
 server/sys-socket.h |  3 ++
 2 files changed, 78 insertions(+)

diff --git a/server/sys-socket.c b/server/sys-socket.c
index 7ce5dab1..837da18e 100644
--- a/server/sys-socket.c
+++ b/server/sys-socket.c
@@ -209,4 +209,79 @@ SPICE_CONSTRUCTOR_FUNC(socket_win32_init)
 WSADATA wsaData;
 WSAStartup(MAKEWORD(2, 2), &wsaData);
 }
+
+int socket_newpair(int type, int protocol, int sv[2])
+{
+struct sockaddr_in sa, sa2;
+socklen_t addrlen;
+SOCKET s, pairs[2];
+
+if (!sv) {
+return -1;
+}
+
+/* create a listener */
+s = socket(AF_INET, type, 0);
+if (s == INVALID_SOCKET) {
+return -1;
+}
+
+pairs[1] = INVALID_SOCKET;
+
+pairs[0] = socket(AF_INET, type, 0);
+if (pairs[0] == INVALID_SOCKET) {
+goto cleanup;
+}
+
+/* bind to a random port */
+sa.sin_family = AF_INET;
+sa.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+sa.sin_port = 0;
+if (bind(s, (struct sockaddr*) &sa, sizeof(sa)) < 0) {
+goto cleanup;
+}
+if (listen(s, 1) < 0) {
+goto cleanup;
+}
+
+/* connect to kernel choosen port */
+addrlen = sizeof(sa);
+if (getsockname(s, (struct sockaddr*) &sa, &addrlen) < 0) {
+goto cleanup;
+}
+if (connect(pairs[0], (struct sockaddr*) &sa, sizeof(sa)) < 0) {
+goto cleanup;
+}
+addrlen = sizeof(sa2);
+pairs[1] = accept(s, (struct sockaddr*) &sa2, &addrlen);
+if (pairs[1] == INVALID_SOCKET) {
+goto cleanup;
+}
+
+/* check proper connection */
+addrlen = sizeof(sa);
+if (getsockname(pairs[0], (struct sockaddr*) &sa, &addrlen) < 0) {
+goto cleanup;
+}
+addrlen = sizeof(sa2);
+if (getpeername(pairs[1], (struct sockaddr*) &sa2, &addrlen) < 0) {
+goto cleanup;
+}
+if (sa.sin_family != sa2.sin_family || sa.sin_port != sa2.sin_port
+|| sa.sin_addr.s_addr != sa2.sin_addr.s_addr) {
+goto cleanup;
+}
+
+closesocket(s);
+sv[0] = pairs[0];
+sv[1] = pairs[1];
+return 0;
+
+cleanup:
+socket_win32_set_errno();
+closesocket(s);
+closesocket(pairs[0]);
+closesocket(pairs[1]);
+return -1;
+}
 #endif
diff --git a/server/sys-socket.h b/server/sys-socket.h
index 65062571..3a3b7878 100644
--- a/server/sys-socket.h
+++ b/server/sys-socket.h
@@ -134,6 +134,9 @@ socket_accept(int sock, struct sockaddr *addr, int *addrlen)
 }
 #undef accept
 #define accept socket_accept
+
+int socket_newpair(int type, int protocol, int sv[2]);
+#define socketpair(family, type, protocol, sv) socket_newpair(type, protocol, 
sv)
 #endif
 
 #endif // RED_SYS_SOCKET_H_
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 04/20] windows: Undefine some conflicting preprocessor macros

[Frediano Ziglio]

"interface" and "MAX_MONITORS" are defined in some Windows system
headers causing garbage code to be fed to the compiler.

Signed-off-by: Frediano Ziglio 
---
 server/red-qxl.c | 4 
 server/reds.c| 6 ++
 2 files changed, 10 insertions(+)

diff --git a/server/red-qxl.c b/server/red-qxl.c
index 0dd26b22..9b9b8c17 100644
--- a/server/red-qxl.c
+++ b/server/red-qxl.c
@@ -40,6 +40,10 @@
 
 #include "red-qxl.h"
 
+#ifdef _WIN32
+// undefine conflicting preprocessor macros
+#undef interface
+#endif
 
 #define MAX_MONITORS_COUNT 16
 
diff --git a/server/reds.c b/server/reds.c
index 8c1c10dc..97023b38 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -79,6 +79,12 @@
 #include "net-utils.h"
 #include "red-stream-device.h"
 
+#ifdef _WIN32
+// undefine conflicting preprocessor macros
+#undef MAX_MONITORS
+#undef interface
+#endif
+
 #define REDS_MAX_STAT_NODES 100
 
 static void reds_client_monitors_config(RedsState *reds, VDAgentMonitorsConfig 
*monitors_config);
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 05/20] tests: Provide alarm replacement for Windows

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
Reviewed-by: Marc-André Lureau 
---
 server/tests/Makefile.am  |  2 +
 server/tests/test-channel.c   |  1 +
 server/tests/test-loop.c  |  1 +
 server/tests/test-stream-device.c |  1 +
 server/tests/win-alarm.c  | 65 +++
 server/tests/win-alarm.h  | 26 +
 6 files changed, 96 insertions(+)
 create mode 100644 server/tests/win-alarm.c
 create mode 100644 server/tests/win-alarm.h

diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
index d7f7af9b..7668739f 100644
--- a/server/tests/Makefile.am
+++ b/server/tests/Makefile.am
@@ -35,6 +35,8 @@ libtest_a_SOURCES =   \
test-display-base.h \
test-glib-compat.c  \
test-glib-compat.h  \
+   win-alarm.c \
+   win-alarm.h \
$(NULL)
 
 LDADD =\
diff --git a/server/tests/test-channel.c b/server/tests/test-channel.c
index 1c9148df..ec0fdceb 100644
--- a/server/tests/test-channel.c
+++ b/server/tests/test-channel.c
@@ -28,6 +28,7 @@
 #include "red-client.h"
 #include "cursor-channel.h"
 #include "net-utils.h"
+#include "win-alarm.h"
 
 /*
  * Declare a RedTestChannel to be used for the test
diff --git a/server/tests/test-loop.c b/server/tests/test-loop.c
index 1e3b39e5..82af80ab 100644
--- a/server/tests/test-loop.c
+++ b/server/tests/test-loop.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include "basic-event-loop.h"
+#include "win-alarm.h"
 
 static SpiceCoreInterface *core = NULL;
 static GMainLoop *loop = NULL;
diff --git a/server/tests/test-stream-device.c 
b/server/tests/test-stream-device.c
index f1707d2f..e1868789 100644
--- a/server/tests/test-stream-device.c
+++ b/server/tests/test-stream-device.c
@@ -33,6 +33,7 @@
 #include "test-glib-compat.h"
 #include "stream-channel.h"
 #include "reds.h"
+#include "win-alarm.h"
 
 static SpiceCharDeviceInstance vmc_instance;
 
diff --git a/server/tests/win-alarm.c b/server/tests/win-alarm.c
new file mode 100644
index ..225d0709
--- /dev/null
+++ b/server/tests/win-alarm.c
@@ -0,0 +1,65 @@
+/* -*- 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 .
+*/
+
+#include 
+#include 
+
+#ifdef _WIN32
+#include 
+#include "win-alarm.h"
+
+static HANDLE alarm_cond = NULL;
+
+static DWORD WINAPI alarm_thread_proc(LPVOID arg)
+{
+unsigned int timeout = (uintptr_t) arg;
+switch (WaitForSingleObject(alarm_cond, timeout * 1000)) {
+case WAIT_OBJECT_0:
+return 0;
+}
+abort();
+return 0;
+}
+
+void alarm(unsigned int timeout)
+{
+static HANDLE thread = NULL;
+
+// create an event to stop the alarm thread
+if (alarm_cond == NULL) {
+alarm_cond = CreateEvent(NULL, TRUE, FALSE, NULL);
+g_assert(alarm_cond != NULL);
+}
+
+// stop old alarm
+if (thread) {
+SetEvent(alarm_cond);
+g_assert(WaitForSingleObject(thread, INFINITE) == WAIT_OBJECT_0);
+CloseHandle(thread);
+thread = NULL;
+}
+
+if (timeout) {
+ResetEvent(alarm_cond);
+
+// start alarm thread
+thread = CreateThread(NULL, 0, alarm_thread_proc, (LPVOID) (uintptr_t) 
timeout, 0, NULL);
+g_assert(thread);
+}
+}
+#endif
diff --git a/server/tests/win-alarm.h b/server/tests/win-alarm.h
new file mode 100644
index ..a7233a8f
--- /dev/null
+++ b/server/tests/win-alarm.h
@@ -0,0 +1,26 @@
+/* -*- 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 Pub

[Spice-devel] [PATCH spice-server v4 01/20] Use proper format strings for spice_log

[Frediano Ziglio]

Formatting string should be compatible with GLib.
GLib uses formatting types compatible with GNU.
For Linux this is not an issue as both systems (like a printf) and
GLib one uses the same formatting type.  However on Windows they
differs potentially causing issues.
This is also make worse as GLib 2.58 changed format attribute from
__printf__ to gnu_printf (Microsoft compatibility formats like %I64d
are still supported but you'll get warnings using GCC/Clang
compilers).

Signed-off-by: Frediano Ziglio 
---
 server/char-device.c | 3 ++-
 server/gstreamer-encoder.c   | 4 ++--
 server/main-channel-client.c | 6 +++---
 server/memslot.c | 2 +-
 server/mjpeg-encoder.c   | 6 --
 server/red-channel.c | 6 --
 server/red-client.c  | 6 --
 server/red-replay-qxl.c  | 9 +
 server/reds.c| 4 ++--
 9 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/server/char-device.c b/server/char-device.c
index 64b41a94..c00e96ef 100644
--- a/server/char-device.c
+++ b/server/char-device.c
@@ -786,7 +786,8 @@ void red_char_device_client_remove(RedCharDevice *dev,
 }
 
 if (dev->priv->clients == NULL) {
-spice_debug("client removed, memory pool will be freed (%"PRIu64" 
bytes)", dev->priv->cur_pool_size);
+spice_debug("client removed, memory pool will be freed 
(%"G_GUINT64_FORMAT" bytes)",
+dev->priv->cur_pool_size);
 write_buffers_queue_free(&dev->priv->write_bufs_pool);
 dev->priv->cur_pool_size = 0;
 }
diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
index 04f0c02f..972c86e6 100644
--- a/server/gstreamer-encoder.c
+++ b/server/gstreamer-encoder.c
@@ -1080,10 +1080,10 @@ static void set_gstenc_bitrate(SpiceGstEncoder *encoder)
 break;
 }
 default:
-spice_warning("the %s property has an unsupported type %zu",
+spice_warning("the %s property has an unsupported type %" 
G_GSIZE_FORMAT,
   prop, param->value_type);
 }
-spice_debug("setting the GStreamer %s to %"PRIu64, prop, gst_bit_rate);
+spice_debug("setting the GStreamer %s to %"G_GUINT64_FORMAT, prop, 
gst_bit_rate);
 }
 
 /* A helper for spice_gst_encoder_encode_frame() */
diff --git a/server/main-channel-client.c b/server/main-channel-client.c
index 54be9934..3b6ae269 100644
--- a/server/main-channel-client.c
+++ b/server/main-channel-client.c
@@ -529,8 +529,8 @@ void main_channel_client_handle_pong(MainChannelClient 
*mcc, SpiceMsgPing *ping,
 if (roundtrip <= mcc->priv->latency) {
 // probably high load on client or server result with incorrect 
values
 red_channel_debug(red_channel_client_get_channel(rcc),
-  "net test: invalid values, latency %" PRIu64
-  " roundtrip %" PRIu64 ". assuming high"
+  "net test: invalid values, latency %" 
G_GUINT64_FORMAT
+  " roundtrip %"G_GUINT64_FORMAT". assuming high"
   "bandwidth", mcc->priv->latency, roundtrip);
 mcc->priv->latency = 0;
 mcc->priv->net_test_stage = NET_TEST_STAGE_INVALID;
@@ -542,7 +542,7 @@ void main_channel_client_handle_pong(MainChannelClient 
*mcc, SpiceMsgPing *ping,
 / (roundtrip - mcc->priv->latency);
 mcc->priv->net_test_stage = NET_TEST_STAGE_COMPLETE;
 red_channel_debug(red_channel_client_get_channel(rcc),
-  "net test: latency %f ms, bitrate %"PRIu64" bps (%f 
Mbps)%s",
+  "net test: latency %f ms, bitrate 
%"G_GUINT64_FORMAT" bps (%f Mbps)%s",
   (double)mcc->priv->latency / 1000,
   mcc->priv->bitrate_per_sec,
   (double)mcc->priv->bitrate_per_sec / 1024 / 1024,
diff --git a/server/memslot.c b/server/memslot.c
index 97311b2e..91ea53bd 100644
--- a/server/memslot.c
+++ b/server/memslot.c
@@ -105,7 +105,7 @@ void *memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL 
addr, uint32_t add_size
 slot_id = memslot_get_id(info, addr);
 if (slot_id >= info->num_memslots) {
 print_memslots(info);
-spice_critical("slot_id %d too big, addr=%" PRIx64, slot_id, addr);
+spice_critical("slot_id %d too big, addr=%" G_GINT64_MODIFIER "x", 
slot_id, addr);
 return NULL;
 }
 
diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c
index d4b5c6fc..d64fcfe0 100644
--- a/server/mjpeg-encoder.c
+++ b/server/mjpeg-encoder.c
@@ -614,7 +614,8 @@ static void 
mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
 
 spice_debug("cur-fps=%u new-fps=%u (new/old=%.2f) |"
 "bit-rate=%.2f (Mbps) latency=%u (ms) quality=%d |"
-" new-size-avg %"PRIu64" , base-size %"PRIu64", (new/old=%.2f) 
",
+" new-size-avg %"G_GUINT64_FORMAT" ,"
+" base-size %

[Spice-devel] [PATCH spice-server v4 00/20] Port SPICE server to Windows

[Frediano Ziglio]

Windows support is useful to use with Qemu under Windows as host or
to implement servers like Xspice.
Mainly SPICE server uses lot of libraries to expose a TCP protocol.
As TCP is implemented with socket library which is quite portable was
not that hard to port.
Beside some minor feature (see REAME.Windows) all was ported.
During porting was choosen to keep Unix as the main platform, if a
change would require too much changes some Windows wrapper is
preferred instead. Not too complicated stuff, the main "wrapper" is
that Windows errors from socket are written back into errno to avoid
to change lot of code handling errors.

Changes since v3:
- reduce compatibility layer size:
  - do not change function name if not required;
  - use int instead of a new socket_t.

Changes since v2:
- better %m replacement;
- many comments updates;
- typo and grammar fixes;
- use int to store socket descriptors/handles;
- merge all v2 updates into a single series;
- split formatting string patch.

Frediano Ziglio (20):
  Use proper format strings for spice_log
  build: Detect Windows build and change some definitions
  Avoids %m in formatting for Windows
  windows: Undefine some conflicting preprocessor macros
  tests: Provide alarm replacement for Windows
  test-stat: Adjust delay checks
  sys-socket: Introduce some utility to make sockets more portable
  sys-socket: Add socket_newpair utility
  net-utils: Use socket compatibility layer
  reds: Use socket compatibility layer
  red-stream: Use socket compatibility layer
  dispatcher: Use socket compatibility layer
  test-leaks: Use socket compatibility layer
  test-channel: Use socket compatibility layer
  windows: Disable code not working on Windows
  dispatcher: Port to Windows
  tests: Exclude tests that cannot work on Windows
  red-stream: Fix SSL connection for Windows
  Disable recording filtering for Windows
  Add some notes for the Windows port

 README.Windows|  18 ++
 configure.ac  |  20 ++-
 server/Makefile.am|   2 +
 server/char-device.c  |   3 +-
 server/dispatcher.c   |  28 ++-
 server/gstreamer-encoder.c|   4 +-
 server/main-channel-client.c  |   6 +-
 server/memslot.c  |   2 +-
 server/mjpeg-encoder.c|   6 +-
 server/net-utils.c|  11 ++
 server/red-channel-client.c   |   2 +
 server/red-channel.c  |   6 +-
 server/red-client.c   |   6 +-
 server/red-common.h   |   1 +
 server/red-qxl.c  |   4 +
 server/red-record-qxl.c   |   7 +
 server/red-replay-qxl.c   |   9 +-
 server/red-stream.c   |  48 -
 server/red-stream.h   |   2 +
 server/red-worker.c   |   6 +
 server/reds.c |  53 +++---
 server/sound.c|   5 +-
 server/stat-file.c|   2 +
 server/sys-socket.c   | 287 ++
 server/sys-socket.h   | 142 +++
 server/tests/Makefile.am  |  11 +-
 server/tests/basic-event-loop.c   |   2 +
 server/tests/replay.c |   2 +
 server/tests/stat-test.c  |  12 +-
 server/tests/test-channel.c   |   9 +-
 server/tests/test-leaks.c |   5 +-
 server/tests/test-loop.c  |   1 +
 server/tests/test-record.c|   7 +-
 server/tests/test-stream-device.c |   1 +
 server/tests/win-alarm.c  |  65 +++
 server/tests/win-alarm.h  |  26 +++
 tools/Makefile.am |   2 +
 37 files changed, 753 insertions(+), 70 deletions(-)
 create mode 100644 README.Windows
 create mode 100644 server/sys-socket.c
 create mode 100644 server/sys-socket.h
 create mode 100644 server/tests/win-alarm.c
 create mode 100644 server/tests/win-alarm.h

-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 14/20] test-channel: Use socket compatibility layer

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/tests/test-channel.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/server/tests/test-channel.c b/server/tests/test-channel.c
index ec0fdceb..a634a662 100644
--- a/server/tests/test-channel.c
+++ b/server/tests/test-channel.c
@@ -194,7 +194,7 @@ static void send_ack_sync(int socket, uint32_t generation)
 msg.len = GUINT32_TO_LE(sizeof(generation));
 msg.generation = GUINT32_TO_LE(generation);
 
-g_assert_cmpint(write(socket, &msg.type, 10), ==, 10);
+g_assert_cmpint(socket_write(socket, &msg.type, 10), ==, 10);
 }
 
 static SpiceTimer *waked_up_timer;
@@ -209,7 +209,7 @@ static void timer_wakeup(void *opaque)
 ssize_t len;
 alarm(1);
 char buffer[256];
-while ((len=recv(client_socket, buffer, sizeof(buffer), 0)) > 0)
+while ((len=socket_read(client_socket, buffer, sizeof(buffer))) > 0)
 got_data += len;
 alarm(0);
 
@@ -229,7 +229,7 @@ static void timeout_watch_count(void *opaque)
 // get all pending data
 alarm(1);
 char buffer[256];
-while (recv(client_socket, buffer, sizeof(buffer), 0) > 0)
+while (socket_read(client_socket, buffer, sizeof(buffer)) > 0)
 continue;
 alarm(0);
 
@@ -237,7 +237,7 @@ static void timeout_watch_count(void *opaque)
 watch_called_countdown = 20;
 
 // send ack reply, this should unblock data from RedChannelClient
-g_assert_cmpint(write(client_socket, "\2\0\0\0\0\0", 6), ==, 6);
+g_assert_cmpint(socket_write(client_socket, "\2\0\0\0\0\0", 6), ==, 6);
 
 // expect data soon
 waked_up_timer = core->timer_add(timer_wakeup, core);
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 03/20] Avoids %m in formatting for Windows

[Frediano Ziglio]

Not supported, %m is a GNU extension of sscanf.

Signed-off-by: Frediano Ziglio 
---
 server/reds.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index d3f73d8e..8c1c10dc 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3698,8 +3698,7 @@ static const int video_codec_caps[] = {
  * @codec: a location to return the parsed codec
  * @return the position of the next codec in the string
  */
-static const char* parse_next_video_codec(const char *codecs, char **encoder,
-  char **codec)
+static char* parse_next_video_codec(char *codecs, char **encoder, char **codec)
 {
 if (!codecs) {
 return NULL;
@@ -3708,14 +3707,15 @@ static const char* parse_next_video_codec(const char 
*codecs, char **encoder,
 if (!*codecs) {
 return NULL;
 }
-int n;
+int end_encoder, end_codec = -1;
 *encoder = *codec = NULL;
-if (sscanf(codecs, "%m[0-9a-zA-Z_]:%m[0-9a-zA-Z_]%n", encoder, codec, &n) 
== 2) {
-// this avoids accepting "encoder:codec" followed by garbage like "$%*"
-if (codecs[n] != ';' && codecs[n] != '\0') {
-free(*codec);
-*codec = NULL;
-}
+if (sscanf(codecs, "%*[0-9a-zA-Z_]:%n%*[0-9a-zA-Z_];%n", &end_encoder, 
&end_codec) == 0
+&& end_codec > 0) {
+codecs[end_encoder - 1] = '\0';
+codecs[end_codec - 1] = '\0';
+*encoder = codecs;
+*codec = codecs + end_encoder;
+return codecs + end_codec;
 }
 return codecs + strcspn(codecs, ";");
 }
@@ -3732,7 +3732,8 @@ static void reds_set_video_codecs_from_string(RedsState 
*reds, const char *codec
 }
 
 video_codecs = g_array_new(FALSE, FALSE, sizeof(RedVideoCodec));
-const char *c = codecs;
+char *codecs_copy = g_strdup_printf("%s;", codecs);
+char *c = codecs_copy;
 while ( (c = parse_next_video_codec(c, &encoder_name, &codec_name)) ) {
 uint32_t encoder_index, codec_index;
 if (!encoder_name || !codec_name) {
@@ -3755,11 +3756,9 @@ static void reds_set_video_codecs_from_string(RedsState 
*reds, const char *codec
 g_array_append_val(video_codecs, new_codec);
 }
 
-/* these are allocated by sscanf, do not use g_free */
-free(encoder_name);
-free(codec_name);
 codecs = c;
 }
+g_free(codecs_copy);
 
 if (video_codecs->len == 0) {
 spice_warning("Failed to set video codecs, input string: '%s'", 
codecs);
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 15/20] windows: Disable code not working on Windows

[Frediano Ziglio]

- global signals;
- CLOEXEC flag;
- mmap and statistics;
- IPTOS_LOWDELAY flag;
- Unix sockets;
- sharing file descriptors through Unix sockets;
- TCP_CORK flag.

Signed-off-by: Frediano Ziglio 
---
 server/red-channel-client.c |  2 ++
 server/red-stream.c | 11 ++-
 server/red-stream.h |  2 ++
 server/red-worker.c |  6 ++
 server/reds.c   |  7 ++-
 server/sound.c  |  5 +++--
 server/stat-file.c  |  2 ++
 server/tests/basic-event-loop.c |  2 ++
 server/tests/replay.c   |  2 ++
 tools/Makefile.am   |  2 ++
 10 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 375a60b3..9ee0ec6c 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -627,6 +627,7 @@ static void 
red_channel_client_restore_main_sender(RedChannelClient *rcc)
 
 static void red_channel_client_msg_sent(RedChannelClient *rcc)
 {
+#ifndef _WIN32
 int fd;
 
 if (spice_marshaller_get_fd(rcc->priv->send_data.marshaller, &fd)) {
@@ -640,6 +641,7 @@ static void red_channel_client_msg_sent(RedChannelClient 
*rcc)
 if (fd != -1)
 close(fd);
 }
+#endif
 
 red_channel_client_clear_sent_item(rcc);
 
diff --git a/server/red-stream.c b/server/red-stream.c
index 8f36d1d4..3641f0ce 100644
--- a/server/red-stream.c
+++ b/server/red-stream.c
@@ -41,7 +41,7 @@
 #include "reds.h"
 
 // compatibility for *BSD systems
-#ifndef TCP_CORK
+#if !defined(TCP_CORK) && !defined(_WIN32)
 #define TCP_CORK TCP_NOPUSH
 #endif
 
@@ -102,6 +102,7 @@ struct RedStreamPrivate {
 SpiceCoreInterfaceInternal *core;
 };
 
+#ifndef _WIN32
 /**
  * Set TCP_CORK on socket
  */
@@ -111,6 +112,12 @@ static int socket_set_cork(int socket, int enabled)
 SPICE_VERIFY(sizeof(enabled) == sizeof(int));
 return setsockopt(socket, IPPROTO_TCP, TCP_CORK, &enabled, 
sizeof(enabled));
 }
+#else
+static inline int socket_set_cork(int socket, int enabled)
+{
+return -1;
+}
+#endif
 
 static ssize_t stream_write_cb(RedStream *s, const void *buf, size_t size)
 {
@@ -318,6 +325,7 @@ int red_stream_get_no_delay(RedStream *stream)
 return red_socket_get_no_delay(stream->socket);
 }
 
+#ifndef _WIN32
 int red_stream_send_msgfd(RedStream *stream, int fd)
 {
 struct msghdr msgh = { 0, };
@@ -360,6 +368,7 @@ int red_stream_send_msgfd(RedStream *stream, int fd)
 
 return r;
 }
+#endif
 
 ssize_t red_stream_writev(RedStream *s, const struct iovec *iov, int iovcnt)
 {
diff --git a/server/red-stream.h b/server/red-stream.h
index 9a7cc617..ca6dc71a 100644
--- a/server/red-stream.h
+++ b/server/red-stream.h
@@ -67,7 +67,9 @@ int red_stream_get_family(const RedStream *stream);
 bool red_stream_is_plain_unix(const RedStream *stream);
 bool red_stream_set_no_delay(RedStream *stream, bool no_delay);
 int red_stream_get_no_delay(RedStream *stream);
+#ifndef _WIN32
 int red_stream_send_msgfd(RedStream *stream, int fd);
+#endif
 
 /**
  * Set auto flush flag.
diff --git a/server/red-worker.c b/server/red-worker.c
index c74ae888..82e71f06 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -1360,22 +1360,28 @@ static void *red_worker_main(void *arg)
 
 bool red_worker_run(RedWorker *worker)
 {
+#ifndef _WIN32
 sigset_t thread_sig_mask;
 sigset_t curr_sig_mask;
+#endif
 int r;
 
 spice_return_val_if_fail(worker, FALSE);
 spice_return_val_if_fail(!worker->thread, FALSE);
 
+#ifndef _WIN32
 sigfillset(&thread_sig_mask);
 sigdelset(&thread_sig_mask, SIGILL);
 sigdelset(&thread_sig_mask, SIGFPE);
 sigdelset(&thread_sig_mask, SIGSEGV);
 pthread_sigmask(SIG_SETMASK, &thread_sig_mask, &curr_sig_mask);
+#endif
 if ((r = pthread_create(&worker->thread, NULL, red_worker_main, worker))) {
 spice_error("create thread failed %d", r);
 }
+#ifndef _WIN32
 pthread_sigmask(SIG_SETMASK, &curr_sig_mask, NULL);
+#endif
 pthread_setname_np(worker->thread, "SPICE Worker");
 
 return r == 0;
diff --git a/server/reds.c b/server/reds.c
index 3404d5a2..1fdf9e84 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2652,9 +2652,11 @@ static int reds_init_socket(const char *addr, int 
portnr, int family)
 static const int on=1, off=0;
 struct addrinfo ai,*res,*e;
 char port[33];
-int slisten, rc, len;
+int slisten, rc;
 
 if (family == AF_UNIX) {
+#ifndef _WIN32
+int len;
 struct sockaddr_un local = { 0, };
 
 if ((slisten = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
@@ -2673,6 +2675,9 @@ static int reds_init_socket(const char *addr, int portnr, 
int family)
 }
 
 goto listen;
+#else
+return -1;
+#endif
 }
 
 memset(&ai,0, sizeof(ai));
diff --git a/server/sound.c b/server/sound.c
index 44b27dec..c08b8bcf 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -775,7 +775,6 @@ static void record_channel_send_item(RedChannelClient *r

[Spice-devel] [PATCH spice-server v4 11/20] red-stream: Use socket compatibility layer

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/red-stream.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/server/red-stream.c b/server/red-stream.c
index 55ad170f..8f36d1d4 100644
--- a/server/red-stream.c
+++ b/server/red-stream.c
@@ -114,7 +114,7 @@ static int socket_set_cork(int socket, int enabled)
 
 static ssize_t stream_write_cb(RedStream *s, const void *buf, size_t size)
 {
-return write(s->socket, buf, size);
+return socket_write(s->socket, buf, size);
 }
 
 static ssize_t stream_writev_cb(RedStream *s, const struct iovec *iov, int 
iovcnt)
@@ -132,7 +132,7 @@ static ssize_t stream_writev_cb(RedStream *s, const struct 
iovec *iov, int iovcn
 for (i = 0; i < tosend; i++) {
 expected += iov[i].iov_len;
 }
-n = writev(s->socket, iov, tosend);
+n = socket_writev(s->socket, iov, tosend);
 if (n <= expected) {
 if (n > 0)
 ret += n;
@@ -148,7 +148,7 @@ static ssize_t stream_writev_cb(RedStream *s, const struct 
iovec *iov, int iovcn
 
 static ssize_t stream_read_cb(RedStream *s, void *buf, size_t size)
 {
-return read(s->socket, buf, size);
+return socket_read(s->socket, buf, size);
 }
 
 static ssize_t stream_ssl_write_cb(RedStream *s, const void *buf, size_t size)
@@ -404,7 +404,7 @@ void red_stream_free(RedStream *s)
 }
 
 red_stream_remove_watch(s);
-close(s->socket);
+socket_close(s->socket);
 
 g_free(s);
 }
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 07/20] sys-socket: Introduce some utility to make sockets more portable

[Frediano Ziglio]

Between Unix and Windows socket are quite different:
- on Windows sockets have a different namespace from C file
  descriptors so you can't use read/write/close or similar functions;
- errors are not stored in errno but you must be read/write the
  errors with specific function;
- sometimes sockets are put in non-blocking mode automatically
  calling some functions;
- SOCKET type is 64 bit on Windows 64 which does not fit technically
  in an int. Is however safe to assume them to fit in an int.

So encapsulate the socket APIs in some definition to make easier
and more safe to deal with them.
Where the portability to Windows would make to code more offuscated a Unix
style was preferred. For instance if errors are detected errno is set from
Windows socket error instead of changing all code handling.
Fortunately on Windows Qemu core interface accepts socket (but not
other types like C file descriptors!).

Signed-off-by: Frediano Ziglio 
---
 server/Makefile.am  |   2 +
 server/sys-socket.c | 212 
 server/sys-socket.h | 139 +
 3 files changed, 353 insertions(+)
 create mode 100644 server/sys-socket.c
 create mode 100644 server/sys-socket.h

diff --git a/server/Makefile.am b/server/Makefile.am
index 34ec22ad..7f260612 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -166,6 +166,8 @@ libserver_la_SOURCES =  \
stat.h  \
stream-channel.c\
stream-channel.h\
+   sys-socket.h\
+   sys-socket.c\
red-stream-device.c \
red-stream-device.h \
sw-canvas.c \
diff --git a/server/sys-socket.c b/server/sys-socket.c
new file mode 100644
index ..7ce5dab1
--- /dev/null
+++ b/server/sys-socket.c
@@ -0,0 +1,212 @@
+/* -*- 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 .
+*/
+#ifdef HAVE_CONFIG_H
+#include 
+#endif
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#ifndef _WIN32
+#include 
+#include 
+#include 
+#include 
+#include 
+#endif
+
+#include 
+
+#include "sys-socket.h"
+
+#ifdef _WIN32
+// Map Windows socket errors to standard C ones
+// See 
https://msdn.microsoft.com/en-us/library/windows/desktop/ms740668(v=vs.85).aspx
+void socket_win32_set_errno(void)
+{
+int err = EPIPE; // default
+switch (WSAGetLastError()) {
+case WSAEWOULDBLOCK:
+case WSAEINPROGRESS:
+err = EAGAIN;
+break;
+case WSAEINTR:
+err = EINTR;
+break;
+case WSAEBADF:
+err = EBADF;
+break;
+case WSA_INVALID_HANDLE:
+case WSA_INVALID_PARAMETER:
+case WSAEINVAL:
+err = EINVAL;
+break;
+case WSAENOTSOCK:
+err = ENOTSOCK;
+break;
+case WSA_NOT_ENOUGH_MEMORY:
+err = ENOMEM;
+break;
+case WSAEPROTONOSUPPORT:
+case WSAESOCKTNOSUPPORT:
+case WSAEOPNOTSUPP:
+case WSAEPFNOSUPPORT:
+case WSAEAFNOSUPPORT:
+case WSAVERNOTSUPPORTED:
+err = ENOTSUP;
+break;
+case WSAEFAULT:
+err = EFAULT;
+break;
+case WSAEACCES:
+err = EACCES;
+break;
+case WSAEMFILE:
+err = EMFILE;
+break;
+case WSAENAMETOOLONG:
+err = ENAMETOOLONG;
+break;
+case WSAENOTEMPTY:
+err = ENOTEMPTY;
+break;
+case WSA_OPERATION_ABORTED:
+case WSAECANCELLED:
+case WSA_E_CANCELLED:
+err = ECANCELED;
+break;
+case WSAEADDRINUSE:
+err = EADDRINUSE;
+break;
+case WSAENETDOWN:
+err = ENETDOWN;
+break;
+case WSAENETUNREACH:
+err = ENETUNREACH;
+break;
+case WSAENETRESET:
+err = ENETRESET;
+break;
+case WSAECONNABORTED:
+err = ECONNABORTED;
+break;
+case WSAECONNRESET:
+err = ECONNRESET;
+break;
+case WSAEISCONN:
+err = EISCONN;
+break;
+case WSAENOTCONN:
+err = ENOTCONN;
+break;
+case WSAETIMEDOUT:
+err = ETIMEDOUT;
+break;
+case WSAECONNR

[Spice-devel] [PATCH spice-server v4 16/20] dispatcher: Port to Windows

[Frediano Ziglio]

Replace poll call with select.
As socket is set to non-blocking we must support it so if
we detect an EAGAIN error wait for data.

Signed-off-by: Frediano Ziglio 
---
 server/dispatcher.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/server/dispatcher.c b/server/dispatcher.c
index 657bfc7d..de221113 100644
--- a/server/dispatcher.c
+++ b/server/dispatcher.c
@@ -197,6 +197,7 @@ static int read_safe(int fd, uint8_t *buf, size_t size, int 
block)
 }
 
 if (!block) {
+#ifndef _WIN32
 struct pollfd pollfd = {.fd = fd, .events = POLLIN, .revents = 0};
 while ((ret = poll(&pollfd, 1, 0)) == -1) {
 if (errno == EINTR) {
@@ -209,6 +210,15 @@ static int read_safe(int fd, uint8_t *buf, size_t size, 
int block)
 if (!(pollfd.revents & POLLIN)) {
 return 0;
 }
+#else
+struct timeval tv = { 0, 0 };
+fd_set fds;
+FD_ZERO(&fds);
+FD_SET(fd, &fds);
+if (select(1, &fds, NULL, NULL, &tv) < 1) {
+return 0;
+}
+#endif
 }
 while (read_size < size) {
 ret = socket_read(fd, buf + read_size, size - read_size);
@@ -217,6 +227,16 @@ static int read_safe(int fd, uint8_t *buf, size_t size, 
int block)
 spice_debug("EINTR in read");
 continue;
 }
+#ifdef _WIN32
+// Windows turns this socket not-blocking
+if (errno == EAGAIN) {
+fd_set fds;
+FD_ZERO(&fds);
+FD_SET(fd, &fds);
+select(1, &fds, NULL, NULL, NULL);
+continue;
+}
+#endif
 return -1;
 }
 if (ret == 0) {
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 20/20] Add some notes for the Windows port

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 README.Windows | 18 ++
 1 file changed, 18 insertions(+)
 create mode 100644 README.Windows

diff --git a/README.Windows b/README.Windows
new file mode 100644
index ..a953813d
--- /dev/null
+++ b/README.Windows
@@ -0,0 +1,18 @@
+SPICE server Windows support
+
+
+SPICE server was ported from Unix/Linux to Windows.
+
+Most features are present, with some exceptions:
+- Unix sockets;
+- signal handling;
+- CLOEXEC flag (completely different handling on Windows);
+- IPTOS_LOWDELAY flag (Linux specific);
+- TCP_CORK (Linux/*BSD specific).
+
+Some features could be ported but currently are not:
+- statistics exported through mapped files. Disabled by default and mainly
+  used for development;
+- filtering while recording (SPICE_WORKER_RECORD_FILTER environment).
+  Recording is used for debugging or development work;
+- TCP_KEEPIDLE setting. Default is used.
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 06/20] test-stat: Adjust delay checks

[Frediano Ziglio]

usleep under Windows does not seem to have the required precision.
Use milliseconds and adjust check times according.

Signed-off-by: Frediano Ziglio 
---
 server/tests/stat-test.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/server/tests/stat-test.c b/server/tests/stat-test.c
index e4a83f4f..444ff7e3 100644
--- a/server/tests/stat-test.c
+++ b/server/tests/stat-test.c
@@ -57,13 +57,13 @@ void TEST_NAME(void)
 
 stat_init(&info, "test", CLOCK_MONOTONIC);
 stat_start_time_init(&start_time, &info);
-usleep(2);
+usleep(2000);
 stat_add(&info, start_time);
 
 #ifdef RED_WORKER_STAT
 g_assert_cmpuint(info.count, ==, 1);
 g_assert_cmpuint(info.min, ==, info.max);
-g_assert_cmpuint(info.min, >=, 2000);
+g_assert_cmpuint(info.min, >=, 200);
 g_assert_cmpuint(info.min, <, 1);
 #endif
 
@@ -71,17 +71,17 @@ void TEST_NAME(void)
 
 stat_compress_init(&info, "test", CLOCK_MONOTONIC);
 stat_start_time_init(&start_time, &info);
-usleep(2);
+usleep(2000);
 stat_compress_add(&info, start_time, 100, 50);
-usleep(1);
+usleep(1000);
 stat_compress_add(&info, start_time, 1000, 500);
 
 #ifdef COMPRESS_STAT
 g_assert_cmpuint(info.count, ==, 2);
 g_assert_cmpuint(info.min, !=, info.max);
-g_assert_cmpuint(info.min, >=, 2000);
+g_assert_cmpuint(info.min, >=, 200);
 g_assert_cmpuint(info.min, <, 1);
-g_assert_cmpuint(info.total, >=, 5000);
+g_assert_cmpuint(info.total, >=, 500);
 g_assert_cmpuint(info.orig_size, ==, 1100);
 g_assert_cmpuint(info.comp_size, ==, 550);
 #endif
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 13/20] test-leaks: Use socket compatibility layer

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/tests/test-leaks.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/server/tests/test-leaks.c b/server/tests/test-leaks.c
index 64130c22..be9fe2d2 100644
--- a/server/tests/test-leaks.c
+++ b/server/tests/test-leaks.c
@@ -35,6 +35,7 @@
 #include "test-glib-compat.h"
 #include "basic-event-loop.h"
 #include "test-display-base.h"
+#include "sys-socket.h"
 
 #define PKI_DIR SPICE_TOP_SRCDIR "/server/tests/pki/"
 
@@ -70,11 +71,11 @@ static void server_leaks(void)
 g_test_expect_message(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING,
   "*SSL_accept failed*");
 g_assert_cmpint(socketpair(AF_LOCAL, SOCK_STREAM, 0, sv), ==, 0);
-close(sv[1]);
+socket_close(sv[1]);
 result = spice_server_add_ssl_client(server, sv[0], 1);
 g_assert_cmpint(result, ==, -1);
 /* if the function fails, it should not close the socket */
-g_assert_cmpint(close(sv[0]), ==, 0);
+g_assert_cmpint(socket_close(sv[0]), ==, 0);
 
 spice_server_destroy(server);
 basic_event_loop_destroy();
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 12/20] dispatcher: Use socket compatibility layer

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/dispatcher.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/server/dispatcher.c b/server/dispatcher.c
index 3e27f2c2..657bfc7d 100644
--- a/server/dispatcher.c
+++ b/server/dispatcher.c
@@ -109,8 +109,8 @@ dispatcher_finalize(GObject *object)
 {
 Dispatcher *self = DISPATCHER(object);
 g_free(self->priv->messages);
-close(self->priv->send_fd);
-close(self->priv->recv_fd);
+socket_close(self->priv->send_fd);
+socket_close(self->priv->recv_fd);
 pthread_mutex_destroy(&self->priv->lock);
 g_free(self->priv->payload);
 G_OBJECT_CLASS(dispatcher_parent_class)->finalize(object);
@@ -211,7 +211,7 @@ static int read_safe(int fd, uint8_t *buf, size_t size, int 
block)
 }
 }
 while (read_size < size) {
-ret = read(fd, buf + read_size, size - read_size);
+ret = socket_read(fd, buf + read_size, size - read_size);
 if (ret == -1) {
 if (errno == EINTR) {
 spice_debug("EINTR in read");
@@ -238,7 +238,7 @@ static int write_safe(int fd, uint8_t *buf, size_t size)
 int ret;
 
 while (written_size < size) {
-ret = write(fd, buf + written_size, size - written_size);
+ret = socket_write(fd, buf + written_size, size - written_size);
 if (ret == -1) {
 if (errno != EINTR) {
 return -1;
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 17/20] tests: Exclude tests that cannot work on Windows

[Frediano Ziglio]

test-stream test is passing file descriptor using Unix socket.
test-stat-file needs some porting work of mmap feature.

Signed-off-by: Frediano Ziglio 
Reviewed-by: Marc-André Lureau 
---
 server/tests/Makefile.am | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
index 7668739f..c50826e6 100644
--- a/server/tests/Makefile.am
+++ b/server/tests/Makefile.am
@@ -53,11 +53,9 @@ check_PROGRAMS = \
test-codecs-parsing \
test-options\
test-stat   \
-   test-stream \
test-agent-msg-filter   \
test-loop   \
test-qxl-parsing\
-   test-stat-file  \
test-leaks  \
test-vdagent\
test-fail-on-null-core-interface\
@@ -68,6 +66,13 @@ check_PROGRAMS = \
test-record \
$(NULL)
 
+if !OS_WIN32
+check_PROGRAMS +=  \
+   test-stream \
+   test-stat-file  \
+   $(NULL)
+endif
+
 noinst_PROGRAMS =  \
test-display-no-ssl \
test-display-streaming  \
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 10/20] reds: Use socket compatibility layer

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/red-common.h |  1 +
 server/reds.c   | 11 ++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/server/red-common.h b/server/red-common.h
index 181ed283..6b5d0b2e 100644
--- a/server/red-common.h
+++ b/server/red-common.h
@@ -35,6 +35,7 @@
 
 #include "spice.h"
 #include "utils.h"
+#include "sys-socket.h"
 
 #define SPICE_UPCAST(type, ptr) \
 (verify_expr(SPICE_OFFSETOF(type, base) == 0,SPICE_CONTAINEROF(ptr, type, 
base)))
diff --git a/server/reds.c b/server/reds.c
index 97023b38..3404d5a2 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -114,6 +114,7 @@ static void adapter_timer_remove(const 
SpiceCoreInterfaceInternal *iface, SpiceT
 static SpiceWatch *adapter_watch_add(const SpiceCoreInterfaceInternal *iface,
  int fd, int event_mask, SpiceWatchFunc 
func, void *opaque)
 {
+// note: Qemu API is fine having a SOCKET on Windows
 return iface->public_interface->watch_add(fd, event_mask, func, opaque);
 }
 
@@ -2667,7 +2668,7 @@ static int reds_init_socket(const char *addr, int portnr, 
int family)
 len = SUN_LEN(&local);
 if (bind(slisten, (struct sockaddr *)&local, len) == -1) {
 perror("bind");
-close(slisten);
+socket_close(slisten);
 return -1;
 }
 
@@ -2715,7 +2716,7 @@ static int reds_init_socket(const char *addr, int portnr, 
int family)
 freeaddrinfo(res);
 goto listen;
 }
-close(slisten);
+socket_close(slisten);
 }
 spice_warning("binding socket to %s:%d failed", addr, portnr);
 freeaddrinfo(res);
@@ -2724,7 +2725,7 @@ static int reds_init_socket(const char *addr, int portnr, 
int family)
 listen:
 if (listen(slisten, SOMAXCONN) != 0) {
 spice_warning("listen: %s", strerror(errno));
-close(slisten);
+socket_close(slisten);
 return -1;
 }
 return slisten;
@@ -2762,14 +2763,14 @@ static void reds_cleanup_net(SpiceServer *reds)
 if (reds->listen_socket != -1) {
reds_core_watch_remove(reds, reds->listen_watch);
if (reds->config->spice_listen_socket_fd != reds->listen_socket) {
-  close(reds->listen_socket);
+  socket_close(reds->listen_socket);
}
reds->listen_watch = NULL;
reds->listen_socket = -1;
 }
 if (reds->secure_listen_socket != -1) {
reds_core_watch_remove(reds, reds->secure_listen_watch);
-   close(reds->secure_listen_socket);
+   socket_close(reds->secure_listen_socket);
reds->secure_listen_watch = NULL;
reds->secure_listen_socket = -1;
 }
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 02/20] build: Detect Windows build and change some definitions

[Frediano Ziglio]

Windows needs some specific setting to use network.

Signed-off-by: Frediano Ziglio 
---
 configure.ac | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 604a41b2..f8b41f37 100644
--- a/configure.ac
+++ b/configure.ac
@@ -68,6 +68,20 @@ case $host_cpu in
 SPICE_WARNING([spice-server on non-x86_64 architectures has not been 
extensively tested])
 esac
 
+AC_MSG_CHECKING([for native Win32])
+case "$host_os" in
+ *mingw*|*cygwin*)
+os_win32=yes
+dnl AI_ADDRCONFIG and possibly some other code require at least Vista
+AC_DEFINE([_WIN32_WINNT], [0x600], [Minimal Win32 version])]
+;;
+ *)
+os_win32=no
+;;
+esac
+AC_MSG_RESULT([$os_win32])
+AM_CONDITIONAL([OS_WIN32],[test "$os_win32" = "yes"])
+
 dnl =
 dnl Check optional features
 SPICE_CHECK_SMARTCARD
@@ -154,6 +168,9 @@ AC_CHECK_LIB(rt, clock_gettime, LIBRT="-lrt")
 AC_SUBST(LIBRT)
 
 AS_VAR_APPEND([SPICE_NONPKGCONFIG_LIBS], [" -pthread $LIBM $LIBRT"])
+AS_IF([test "x$os_win32" = "xyes"], [
+AS_VAR_APPEND([SPICE_NONPKGCONFIG_LIBS], [" -lws2_32"])
+])
 
 SPICE_REQUIRES=""
 
@@ -176,7 +193,8 @@ PKG_CHECK_MODULES([GOBJECT2], [gobject-2.0 >= 
$GLIB2_REQUIRED])
 AS_VAR_APPEND([SPICE_REQUIRES], [" gobject-2.0 >= $GLIB2_REQUIRED"])
 
 #used only by tests
-PKG_CHECK_MODULES([GIO_UNIX], [gio-unix-2.0 >= $GLIB2_REQUIRED])
+AS_IF([test "x$os_win32" != "xyes"],
+  PKG_CHECK_MODULES([GIO_UNIX], [gio-unix-2.0 >= $GLIB2_REQUIRED]))
 
 PIXMAN_REQUIRED=0.17.7
 PKG_CHECK_MODULES(PIXMAN, pixman-1 >= $PIXMAN_REQUIRED)
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 19/20] Disable recording filtering for Windows

[Frediano Ziglio]

Although this feature can be ported to Windows doing so would
require the usage of g_spawn_async_with_fds, which is only available
in GLib 2.58 or some specific Win32 code.

Signed-off-by: Frediano Ziglio 
---
 server/red-record-qxl.c| 7 +++
 server/tests/test-record.c | 7 +--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/server/red-record-qxl.c b/server/red-record-qxl.c
index 30a3b0da..f3a2dc39 100644
--- a/server/red-record-qxl.c
+++ b/server/red-record-qxl.c
@@ -817,6 +817,7 @@ void red_record_qxl_command(RedRecord *record, 
RedMemSlotInfo *slots,
 pthread_mutex_unlock(&record->lock);
 }
 
+#ifndef _WIN32
 /**
  * Redirects child output to the file specified
  */
@@ -829,6 +830,7 @@ static void child_output_setup(gpointer user_data)
 }
 close(fd);
 }
+#endif
 
 RedRecord *red_record_new(const char *filename)
 {
@@ -845,6 +847,7 @@ RedRecord *red_record_new(const char *filename)
 
 filter = getenv("SPICE_WORKER_RECORD_FILTER");
 if (filter) {
+#ifndef _WIN32
 gint argc;
 gchar **argv = NULL;
 GError *error = NULL;
@@ -870,6 +873,10 @@ RedRecord *red_record_new(const char *filename)
 }
 close(fd_in);
 g_spawn_close_pid(child_pid);
+#else
+// TODO
+spice_warning("recorder filter not supported under Windows");
+#endif
 }
 
 if (fwrite(header, sizeof(header)-1, 1, f) != 1) {
diff --git a/server/tests/test-record.c b/server/tests/test-record.c
index de3c6f5b..8ee36ceb 100644
--- a/server/tests/test-record.c
+++ b/server/tests/test-record.c
@@ -35,9 +35,9 @@ test_record(bool compress)
 RedRecord *rec;
 const char *fn = OUTPUT_FILENAME;
 
-unsetenv("SPICE_WORKER_RECORD_FILTER");
+g_unsetenv("SPICE_WORKER_RECORD_FILTER");
 if (compress) {
-setenv("SPICE_WORKER_RECORD_FILTER", "gzip", 1);
+g_setenv("SPICE_WORKER_RECORD_FILTER", "gzip", 1);
 }
 
 // delete possible stale test output
@@ -95,6 +95,9 @@ int
 main(void)
 {
 test_record(false);
+// TODO implement on Windows
+#ifndef _WIN32
 test_record(true);
+#endif
 return 0;
 }
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server v4 18/20] red-stream: Fix SSL connection for Windows

[Frediano Ziglio]

Set correctly errno to make callers handle correctly encrypted
traffic.

Signed-off-by: Frediano Ziglio 
---
 server/red-stream.c | 29 +
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/server/red-stream.c b/server/red-stream.c
index 3641f0ce..d9e32845 100644
--- a/server/red-stream.c
+++ b/server/red-stream.c
@@ -158,15 +158,37 @@ static ssize_t stream_read_cb(RedStream *s, void *buf, 
size_t size)
 return socket_read(s->socket, buf, size);
 }
 
+static ssize_t stream_ssl_error(RedStream *s, int return_code)
+{
+SPICE_GNUC_UNUSED int ssl_error;
+
+ssl_error = SSL_get_error(s->priv->ssl, return_code);
+
+// OpenSSL can to return SSL_ERROR_WANT_READ if we attempt to read
+// data and the socket did not receive all SSL packet.
+// Under Windows errno is not set so potentially caller can detect
+// the wrong error so we need to set errno.
+#ifdef _WIN32
+if (ssl_error == SSL_ERROR_WANT_READ || ssl_error == SSL_ERROR_WANT_WRITE) 
{
+errno = EAGAIN;
+} else {
+errno = EPIPE;
+}
+#endif
+
+// red_peer_receive is expected to receive -1 on errors while
+// OpenSSL documentation just state a <0 value
+return -1;
+}
+
 static ssize_t stream_ssl_write_cb(RedStream *s, const void *buf, size_t size)
 {
 int return_code;
-SPICE_GNUC_UNUSED int ssl_error;
 
 return_code = SSL_write(s->priv->ssl, buf, size);
 
 if (return_code < 0) {
-ssl_error = SSL_get_error(s->priv->ssl, return_code);
+return stream_ssl_error(s, return_code);
 }
 
 return return_code;
@@ -175,12 +197,11 @@ static ssize_t stream_ssl_write_cb(RedStream *s, const 
void *buf, size_t size)
 static ssize_t stream_ssl_read_cb(RedStream *s, void *buf, size_t size)
 {
 int return_code;
-SPICE_GNUC_UNUSED int ssl_error;
 
 return_code = SSL_read(s->priv->ssl, buf, size);
 
 if (return_code < 0) {
-ssl_error = SSL_get_error(s->priv->ssl, return_code);
+return stream_ssl_error(s, return_code);
 }
 
 return return_code;
-- 
2.20.1

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

Re: [Spice-devel] [PATCH spice-common] Obsolete Glib cleanup

[Eduardo Lima (Etrunko)]

On 2/6/19 11:32 AM, Frediano Ziglio wrote:
> We require at least GLib 2.38, remove code and check to
> support earlier versions.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  common/log.c | 8 
>  tests/test-logging.c | 4 
>  2 files changed, 12 deletions(-)
> 
> diff --git a/common/log.c b/common/log.c
> index f9cdd60..054fd7f 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -36,14 +36,6 @@
>  
>  SPICE_CONSTRUCTOR_FUNC(spice_log_init)
>  {
> -/* Threading is always enabled from 2.31.0 onwards */
> -/* Our logging is potentially used from different threads.
> - * Older glibs require that g_thread_init() is called when
> - * doing that. */
> -#if !GLIB_CHECK_VERSION(2, 31, 0)
> -if (!g_thread_supported())
> -g_thread_init(NULL);
> -#endif
>  recorder_dump_on_common_signals(0, 0);
>  }
>  
> diff --git a/tests/test-logging.c b/tests/test-logging.c
> index ff2d8bd..6a79ca9 100644
> --- a/tests/test-logging.c
> +++ b/tests/test-logging.c
> @@ -43,7 +43,6 @@ LOG_OTHER_HELPER(message, MESSAGE)
>  LOG_OTHER_HELPER(warning, WARNING)
>  LOG_OTHER_HELPER(critical, CRITICAL)
>  
> -#if GLIB_CHECK_VERSION(2,38,0)
>  /* Checks that spice_warning() aborts after setting G_DEBUG=fatal-warnings */
>  static void test_spice_fatal_warning(void)
>  {
> @@ -268,7 +267,6 @@ static void test_spice_g_messages_debug_all(void)
>  
> g_test_trap_assert_stdout("*spice_debug\n*spice_info\n*g_debug\n*g_info\n*other_debug\n*other_info\n");
>  g_test_trap_assert_stderr("*g_message\n*other_message\n");
>  }
> -#endif /* GLIB_CHECK_VERSION(2,38,0) */
>  
>  static void handle_sigabrt(int sig G_GNUC_UNUSED)
>  {
> @@ -292,7 +290,6 @@ int main(int argc, char **argv)
>   * test cases are going to test */
>  g_log_set_always_fatal(fatal_mask & G_LOG_LEVEL_MASK);
>  
> -#if GLIB_CHECK_VERSION(2,38,0)
>  g_test_add_func("/spice-common/spice-g-messages-debug", 
> test_spice_g_messages_debug);
>  g_test_add_func("/spice-common/spice-g-messages-debug-all", 
> test_spice_g_messages_debug_all);
>  g_test_add_func("/spice-common/spice-log-levels", test_log_levels);
> @@ -302,7 +299,6 @@ int main(int argc, char **argv)
>  g_test_add_func("/spice-common/spice-fatal-return-if-fail", 
> test_spice_fatal_return_if_fail);
>  g_test_add_func("/spice-common/spice-non-fatal-greturn-if-fail", 
> test_spice_non_fatal_g_return_if_fail);
>  g_test_add_func("/spice-common/spice-fatal-warning", 
> test_spice_fatal_warning);
> -#endif /* GLIB_CHECK_VERSION(2,38,0) */
>  
>  return g_test_run();
>  }
> 
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 spice-server] Use new GObject define macros with private

[Eduardo Lima (Etrunko)]

G_ADD_PRIVATE was added in 2.38 and older functions are getting deprecated:
https://gitlab.gnome.org/GNOME/glib/merge_requests/7/commits

Signed-off-by: Eduardo Lima (Etrunko) 
---
 server/char-device.c |  4 +---
 server/common-graphics-channel.c | 23 ++-
 server/cursor-channel-client.c   | 12 +---
 server/dispatcher.c  | 11 ---
 server/inputs-channel-client.c   | 12 +---
 server/main-channel-client.c | 12 +---
 server/main-dispatcher.c | 10 +++---
 server/red-channel-client.c  |  4 ++--
 server/red-channel.c | 11 ---
 server/reds.c|  4 +---
 server/smartcard.c   | 15 ++-
 11 files changed, 46 insertions(+), 72 deletions(-)

diff --git a/server/char-device.c b/server/char-device.c
index 64b41a94..cda26be8 100644
--- a/server/char-device.c
+++ b/server/char-device.c
@@ -84,7 +84,7 @@ struct RedCharDevicePrivate {
 SpiceServer *reds;
 };
 
-G_DEFINE_TYPE(RedCharDevice, red_char_device, G_TYPE_OBJECT)
+G_DEFINE_TYPE_WITH_PRIVATE(RedCharDevice, red_char_device, G_TYPE_OBJECT)
 
 #define RED_CHAR_DEVICE_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), 
RED_TYPE_CHAR_DEVICE, RedCharDevicePrivate))
 
@@ -1119,8 +1119,6 @@ red_char_device_class_init(RedCharDeviceClass *klass)
 {
 GObjectClass *object_class = G_OBJECT_CLASS(klass);
 
-g_type_class_add_private(klass, sizeof (RedCharDevicePrivate));
-
 object_class->get_property = red_char_device_get_property;
 object_class->set_property = red_char_device_set_property;
 object_class->finalize = red_char_device_finalize;
diff --git a/server/common-graphics-channel.c b/server/common-graphics-channel.c
index 083ab3eb..3d76c82c 100644
--- a/server/common-graphics-channel.c
+++ b/server/common-graphics-channel.c
@@ -26,16 +26,6 @@
 
 #define CHANNEL_RECEIVE_BUF_SIZE 1024
 
-G_DEFINE_ABSTRACT_TYPE(CommonGraphicsChannel, common_graphics_channel, 
RED_TYPE_CHANNEL)
-
-G_DEFINE_TYPE(CommonGraphicsChannelClient, common_graphics_channel_client, 
RED_TYPE_CHANNEL_CLIENT)
-
-#define GRAPHICS_CHANNEL_PRIVATE(o) \
-(G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_COMMON_GRAPHICS_CHANNEL, 
CommonGraphicsChannelPrivate))
-#define GRAPHICS_CHANNEL_CLIENT_PRIVATE(o) \
-(G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_COMMON_GRAPHICS_CHANNEL_CLIENT, \
-CommonGraphicsChannelClientPrivate))
-
 struct CommonGraphicsChannelPrivate
 {
 int during_target_migrate; /* TRUE when the client that is associated with 
the channel
@@ -45,10 +35,20 @@ struct CommonGraphicsChannelPrivate
   of the primary surface) */
 };
 
+G_DEFINE_ABSTRACT_TYPE_WITH_PRIVATE(CommonGraphicsChannel, 
common_graphics_channel, RED_TYPE_CHANNEL)
+
+#define GRAPHICS_CHANNEL_PRIVATE(o) \
+(G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_COMMON_GRAPHICS_CHANNEL, 
CommonGraphicsChannelPrivate))
+
 struct CommonGraphicsChannelClientPrivate {
 uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
 };
 
+G_DEFINE_TYPE_WITH_PRIVATE(CommonGraphicsChannelClient, 
common_graphics_channel_client, RED_TYPE_CHANNEL_CLIENT)
+
+#define GRAPHICS_CHANNEL_CLIENT_PRIVATE(o) \
+(G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_COMMON_GRAPHICS_CHANNEL_CLIENT, \
+CommonGraphicsChannelClientPrivate))
 
 static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, 
uint32_t size)
 {
@@ -103,7 +103,6 @@ bool common_channel_client_config_socket(RedChannelClient 
*rcc)
 static void
 common_graphics_channel_class_init(CommonGraphicsChannelClass *klass)
 {
-g_type_class_add_private(klass, sizeof(CommonGraphicsChannelPrivate));
 }
 
 static void
@@ -133,8 +132,6 @@ 
common_graphics_channel_client_class_init(CommonGraphicsChannelClientClass *klas
 {
 RedChannelClientClass *client_class = RED_CHANNEL_CLIENT_CLASS(klass);
 
-g_type_class_add_private(klass, 
sizeof(CommonGraphicsChannelClientPrivate));
-
 client_class->config_socket = common_channel_client_config_socket;
 client_class->alloc_recv_buf = common_alloc_recv_buf;
 client_class->release_recv_buf = common_release_recv_buf;
diff --git a/server/cursor-channel-client.c b/server/cursor-channel-client.c
index 6d39e24e..8b50a3ab 100644
--- a/server/cursor-channel-client.c
+++ b/server/cursor-channel-client.c
@@ -35,11 +35,6 @@
 #define CURSOR_CACHE_HASH_KEY(id) ((id) & CURSOR_CACHE_HASH_MASK)
 #define CURSOR_CLIENT_TIMEOUT 300ULL //nano
 
-G_DEFINE_TYPE(CursorChannelClient, cursor_channel_client, 
TYPE_COMMON_GRAPHICS_CHANNEL_CLIENT)
-
-#define CURSOR_CHANNEL_CLIENT_PRIVATE(o) \
-(G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_CURSOR_CHANNEL_CLIENT, 
CursorChannelClientPrivate))
-
 struct CursorChannelClientPrivate
 {
 RedCacheItem *cursor_cache[CURSOR_CACHE_HASH_SIZE];
@@ -48,6 +43,11 @@ struct CursorChannelClientPrivate
 uint32_t cursor_cache_items;
 };
 
+G_DEFINE_TYPE_WITH_PRIVATE(CursorChannelClient, cursor_channel_client, 
TYPE_COMMON_GRAPHICS_CHANNEL_CLIENT