[Spice-devel] [PATCH spice-gtk] Use same sink settings for direct rendering

2018-11-19 Thread Frediano Ziglio
From: Snir Sheriber 

Set "sync" and "drop" as normal streaming.
---
 src/channel-display-gst.c | 19 +++
 1 file changed, 19 insertions(+)

Well, I extracted this patch from a more bigger Snir sent, not much
changes so I kept the authorship.

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 2c07f350..5f49c3bb 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -382,6 +382,24 @@ static void app_source_setup(GstElement *pipeline 
G_GNUC_UNUSED,
 }
 #endif
 
+// This function is used to set properties in dynamically added sink (if 
overlay is used)
+static void
+add_elem_cb(GstBin * pipeline, GstBin * bin, GstElement * element, 
SpiceGstDecoder *decoder)
+{
+char *name = gst_element_get_name(element);
+
+spice_debug("Adding element: %s", name);
+
+if (GST_IS_BASE_SINK(element)) {
+g_object_set(element,
+ "sync", FALSE,
+ "drop", FALSE,
+ NULL);
+spice_debug("SINK");
+}
+g_free(name);
+}
+
 static gboolean create_pipeline(SpiceGstDecoder *decoder)
 {
 GstBus *bus;
@@ -442,6 +460,7 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
 }
 }
 
+g_signal_connect(playbin, "deep-element-added", G_CALLBACK(add_elem_cb), 
decoder);
 g_signal_connect(playbin, "source-setup", G_CALLBACK(app_source_setup), 
decoder);
 
 g_object_set(playbin,
-- 
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-common] Integrate recorder library

2018-11-19 Thread Frediano Ziglio
> 
> From: Christophe de Dinechin 
> 

Well... probably I started with some other patches and git kept the author.

> Allow to use recorder library. See https://github.com/c3d/recorder for
> details.
> The main usage will be to collect statistics while the programs will run.
> By default the recorder will be disabled at compile time.
> Both autoconf and Meson are supported.
> Autoconf requires the addition of SPICE_CHECK_RECORDER call in configure.ac.
> Meson requires to add recorder option.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  .gitmodules |  3 ++
>  common/Makefile.am  | 10 +
>  common/log.c|  3 ++
>  common/meson.build  | 12 +-
>  common/recorder |  1 +
>  common/recorder.h   | 75 +
>  configure.ac|  8 +++-
>  m4/spice-deps.m4| 15 
>  meson.build | 12 +-
>  meson_options.txt   |  6 +++
>  tests/Makefile.am   | 12 ++
>  tests/test-dummy-recorder.c | 53 ++
>  12 files changed, 206 insertions(+), 4 deletions(-)
>  create mode 16 common/recorder
>  create mode 100644 common/recorder.h
>  create mode 100644 tests/test-dummy-recorder.c
> 
> diff --git a/.gitmodules b/.gitmodules
> index e69de29..a7ea8b1 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -0,0 +1,3 @@
> +[submodule "common/recorder"]
> + path = common/recorder
> + url = https://github.com/c3d/recorder.git
> diff --git a/common/Makefile.am b/common/Makefile.am
> index da0d941..4c0a6cd 100644
> --- a/common/Makefile.am
> +++ b/common/Makefile.am
> @@ -51,8 +51,18 @@ libspice_common_la_SOURCES =   \
>   snd_codec.c \
>   snd_codec.h \
>   verify.h\
> + recorder.h  \
>   $(NULL)
>  
> +if ENABLE_RECORDER
> +libspice_common_la_SOURCES += \
> + recorder/recorder.c \
> + recorder/recorder.h \
> + recorder/recorder_ring.c\
> + recorder/recorder_ring.h\
> + $(NULL)
> +endif
> +
>  # These 2 files are not build as part of spice-common
>  # build system, but modules using spice-common will build
>  # them with the appropriate options. We need to let automake
> diff --git a/common/log.c b/common/log.c
> index b59c8a8..a7806c5 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -27,6 +27,8 @@
>  #include 
>  #endif
>  
> +#include 
> +
>  #include "log.h"
>  #include "backtrace.h"
>  
> @@ -154,6 +156,7 @@ SPICE_CONSTRUCTOR_FUNC(spice_log_init)
>  if (!g_thread_supported())
>  g_thread_init(NULL);
>  #endif
> +recorder_dump_on_common_signals(0, 0);
>  }
>  
>  static void spice_logv(const char *log_domain,
> diff --git a/common/meson.build b/common/meson.build
> index 6ac55dc..e715e31 100644
> --- a/common/meson.build
> +++ b/common/meson.build
> @@ -35,9 +35,19 @@ spice_common_sources = [
>'rop3.h',
>'snd_codec.c',
>'snd_codec.h',
> -  'verify.h'
> +  'verify.h',
> +  'recorder.h'
>  ]
>  
> +if get_option('recorder')
> +  spice_common_sources += [
> +'recorder/recorder.c',
> +'recorder/recorder.h',
> +'recorder/recorder_ring.c',
> +'recorder/recorder_ring.h'
> +  ]
> +endif
> +
>  spice_common_lib = static_library('spice-common', spice_common_sources,
>install : false,
>include_directories :
>spice_common_include,
> diff --git a/common/recorder b/common/recorder
> new file mode 16
> index 000..8f450dc
> --- /dev/null
> +++ b/common/recorder
> @@ -0,0 +1 @@
> +Subproject commit 8f450dcf0ec725a41b80c495ecdd6110dd1fcdb8
> diff --git a/common/recorder.h b/common/recorder.h
> new file mode 100644
> index 000..4496de9
> --- /dev/null
> +++ b/common/recorder.h
> @@ -0,0 +1,75 @@
> +/*
> +  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
> .
> +*/
> +/* This file include recorder library headers or if disabled provide
> + * replacement declarations */
> +#ifndef ENABLE_RECORDER
> +
> +#include 
> +#include 
> +
> +/* Replacement declarations.
> + * There declarations should generate no code (beside when no optimization
> are
> + * sele

[Spice-devel] [PATCH spice-common] Integrate recorder library

2018-11-19 Thread Frediano Ziglio
From: Christophe de Dinechin 

Allow to use recorder library. See https://github.com/c3d/recorder for
details.
The main usage will be to collect statistics while the programs will run.
By default the recorder will be disabled at compile time.
Both autoconf and Meson are supported.
Autoconf requires the addition of SPICE_CHECK_RECORDER call in configure.ac.
Meson requires to add recorder option.

Signed-off-by: Frediano Ziglio 
---
 .gitmodules |  3 ++
 common/Makefile.am  | 10 +
 common/log.c|  3 ++
 common/meson.build  | 12 +-
 common/recorder |  1 +
 common/recorder.h   | 75 +
 configure.ac|  8 +++-
 m4/spice-deps.m4| 15 
 meson.build | 12 +-
 meson_options.txt   |  6 +++
 tests/Makefile.am   | 12 ++
 tests/test-dummy-recorder.c | 53 ++
 12 files changed, 206 insertions(+), 4 deletions(-)
 create mode 16 common/recorder
 create mode 100644 common/recorder.h
 create mode 100644 tests/test-dummy-recorder.c

diff --git a/.gitmodules b/.gitmodules
index e69de29..a7ea8b1 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -0,0 +1,3 @@
+[submodule "common/recorder"]
+   path = common/recorder
+   url = https://github.com/c3d/recorder.git
diff --git a/common/Makefile.am b/common/Makefile.am
index da0d941..4c0a6cd 100644
--- a/common/Makefile.am
+++ b/common/Makefile.am
@@ -51,8 +51,18 @@ libspice_common_la_SOURCES = \
snd_codec.c \
snd_codec.h \
verify.h\
+   recorder.h  \
$(NULL)
 
+if ENABLE_RECORDER
+libspice_common_la_SOURCES += \
+   recorder/recorder.c \
+   recorder/recorder.h \
+   recorder/recorder_ring.c\
+   recorder/recorder_ring.h\
+   $(NULL)
+endif
+
 # These 2 files are not build as part of spice-common
 # build system, but modules using spice-common will build
 # them with the appropriate options. We need to let automake
diff --git a/common/log.c b/common/log.c
index b59c8a8..a7806c5 100644
--- a/common/log.c
+++ b/common/log.c
@@ -27,6 +27,8 @@
 #include 
 #endif
 
+#include 
+
 #include "log.h"
 #include "backtrace.h"
 
@@ -154,6 +156,7 @@ SPICE_CONSTRUCTOR_FUNC(spice_log_init)
 if (!g_thread_supported())
 g_thread_init(NULL);
 #endif
+recorder_dump_on_common_signals(0, 0);
 }
 
 static void spice_logv(const char *log_domain,
diff --git a/common/meson.build b/common/meson.build
index 6ac55dc..e715e31 100644
--- a/common/meson.build
+++ b/common/meson.build
@@ -35,9 +35,19 @@ spice_common_sources = [
   'rop3.h',
   'snd_codec.c',
   'snd_codec.h',
-  'verify.h'
+  'verify.h',
+  'recorder.h'
 ]
 
+if get_option('recorder')
+  spice_common_sources += [
+'recorder/recorder.c',
+'recorder/recorder.h',
+'recorder/recorder_ring.c',
+'recorder/recorder_ring.h'
+  ]
+endif
+
 spice_common_lib = static_library('spice-common', spice_common_sources,
   install : false,
   include_directories : spice_common_include,
diff --git a/common/recorder b/common/recorder
new file mode 16
index 000..8f450dc
--- /dev/null
+++ b/common/recorder
@@ -0,0 +1 @@
+Subproject commit 8f450dcf0ec725a41b80c495ecdd6110dd1fcdb8
diff --git a/common/recorder.h b/common/recorder.h
new file mode 100644
index 000..4496de9
--- /dev/null
+++ b/common/recorder.h
@@ -0,0 +1,75 @@
+/*
+  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 .
+*/
+/* This file include recorder library headers or if disabled provide
+ * replacement declarations */
+#ifndef ENABLE_RECORDER
+
+#include 
+#include 
+
+/* Replacement declarations.
+ * There declarations should generate no code (beside when no optimization are
+ * selected) but catch some possible programming warnings/errors at
+ * compile/link time like:
+ * - usage of undeclared recorders;
+ * - recording formats and arguments;
+ * - matching RECORD_TIMING_BEGIN and RECORD_TIMING_END.
+ * The only exceptions are tweaks which just generate a variable to hold the
+ * value.
+ */
+
+typedef struct SpiceEmptyStruct {
+char dummy[0];
+} SpiceEmptyStruct;

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

2018-11-19 Thread Frediano Ziglio
> 
> On Mon, 2018-11-19 at 10:23 +, Frediano Ziglio wrote:
> > Do not try  indefinitely to connect to the daemon, should not
> > take long to activate.
> > 
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  src/vdagent/vdagent.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > Changes since v1:
> > - log error when we reach the limit
> > 
> > diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> > index f7c8b72..7a58150 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,11 @@ 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) {
> > +syslog(LOG_ERR, "Attempted to contact daemon for 5 minutes,
> > giving up");
> > +goto err_init;
> > +}
> 
> (in reference to trying to keep it simple from v1 thread:)
> I think using 60 instead of 1 in g_timeout_add_seconds() wouldn't be
> much of a complication... unless there's something else I'm missing.
> I'm also thinking this is being launched from a .desktop file so AFAIK
> there is no easy way for a user to restart the service, a relogin is
> needed?
> 

It reconnects to the new daemon after detecting the disconnection.
Not sure what will happen if the agent is updated... I think nothing
or possibly the agent will end if not compatible with the new daemon.
If you retry every minute is possible that the user has to wait 1
minute instead of 1 second.
Probably the optimal way would be to use inotify on the socket to
wait for the new daemon, so something like:
- try 10 times with 1 second delay
- setup inotify
- try once to avoid races
- wait inotify
- attempt connection again (repeat from first step)
This way if the daemon is restored after even hours you don't have to
keep polling.

> I also just realized you should reset connection_attempts to 0 after a
> successful connect?
> 

The object is created again so will be 0 again (not that would hurt
to reset on a successful connection).

> Cheers,
> Lukas
> 
> >  g_timeout_add_seconds(1, vdagent_init_async_cb, agent);
> >  return G_SOURCE_REMOVE;
> >  }
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] Windows compilation

2018-11-19 Thread Frediano Ziglio
> > Use MingW from Linux (cross compilation).
> 

> > Frediano
> 

> Tried that too, but got "undefined reference to `_imp___lock_file'" and
> "undefined reference to `_imp___unlock_file'" when compiling vdlog.cpp .

> Using an up-to-date CentOS 7. Sources from
> https://gitlab.freedesktop.org/spice/win32/vd_agent.git

Interesting! Old MingW? Looks like some compatibility functions
were introduced after 5.0.1 version.
From rpmfind looks like EPEL 7 has still 4.0.x version.

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


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

2018-11-19 Thread Victor Toso
Hi,

On Mon, Nov 19, 2018 at 05:27:43AM -0500, Frediano Ziglio wrote:
> > 
> > Hi,
> > 
> > On Fri, 2018-11-16 at 16:21 +, Frediano Ziglio wrote:
> > > 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;
> > > +}
> > 
> > Please add a log message saying you're giving up trying to connect...
> > 
> 
> Nice idea.
> 
> > Though would it not be better to just increase the interval to ~30-60
> > seconds instead of giving up altogether after some amount of quick
> > tries?
> > 
> 
> Mainly I wanted to keep it simple.
> This code (the retry) was written for daemon not starting fast enough,
> see https://bugzilla.redhat.com/show_bug.cgi?id=681797.
> With systemd now the socket is always there reducing the original issue.

Should add this rationale on commit log. So, we if are on
systemd, we don't need retry at all?

> But in some cases it seems that the daemon fails to start.

And in that case you want to limit to 5 min... ok.

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 vd_agent_linux v2] vdagent: Stop trying to connect to the daemon after a while

2018-11-19 Thread Victor Toso
Hi,

On Mon, Nov 19, 2018 at 10:23:22AM +, Frediano Ziglio wrote:
> Do not try  indefinitely to connect to the daemon, should not
> take long to activate.

Why?

> 
> Signed-off-by: Frediano Ziglio 
> ---
>  src/vdagent/vdagent.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> Changes since v1:
> - log error when we reach the limit
> 
> diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> index f7c8b72..7a58150 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,11 @@ 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) {
> +syslog(LOG_ERR, "Attempted to contact daemon for 5 minutes, 
> giving up");
> +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


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 vd_agent_linux v2] vdagent: Stop trying to connect to the daemon after a while

2018-11-19 Thread Lukáš Hrázký
On Mon, 2018-11-19 at 10:23 +, Frediano Ziglio wrote:
> Do not try  indefinitely to connect to the daemon, should not
> take long to activate.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  src/vdagent/vdagent.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> Changes since v1:
> - log error when we reach the limit
> 
> diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> index f7c8b72..7a58150 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,11 @@ 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) {
> +syslog(LOG_ERR, "Attempted to contact daemon for 5 minutes, 
> giving up");
> +goto err_init;
> +}

(in reference to trying to keep it simple from v1 thread:)
I think using 60 instead of 1 in g_timeout_add_seconds() wouldn't be
much of a complication... unless there's something else I'm missing.
I'm also thinking this is being launched from a .desktop file so AFAIK
there is no easy way for a user to restart the service, a relogin is
needed?

I also just realized you should reset connection_attempts to 0 after a
successful connect?

Cheers,
Lukas

>  g_timeout_add_seconds(1, vdagent_init_async_cb, agent);
>  return G_SOURCE_REMOVE;
>  }
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] Windows compilation

2018-11-19 Thread Frediano Ziglio
> Hi guys/gals,

> What's the recommended way to compile spice vdagent for Windows? Tried Msys2
> and had to change the code in order for it to compile - and service start
> thereafter. I'm new to the list, couldn't find this info; sorry if it's a
> recurring topic.

> Thanks,
> Ivo Cavalcante

Use MingW from Linux (cross compilation).

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


[Spice-devel] Windows compilation

2018-11-19 Thread Ivo
Hi guys/gals,

What's the recommended way to compile spice vdagent for Windows? Tried
Msys2 and had to change the code in order for it to compile - and service
start thereafter. I'm new to the list, couldn't find this info; sorry if
it's a recurring topic.


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


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

2018-11-19 Thread Frediano Ziglio
> 
> Hi,
> 
> On Fri, 2018-11-16 at 16:21 +, Frediano Ziglio wrote:
> > 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;
> > +}
> 
> Please add a log message saying you're giving up trying to connect...
> 

Nice idea.

> Though would it not be better to just increase the interval to ~30-60
> seconds instead of giving up altogether after some amount of quick
> tries?
> 

Mainly I wanted to keep it simple.
This code (the retry) was written for daemon not starting fast enough,
see https://bugzilla.redhat.com/show_bug.cgi?id=681797.
With systemd now the socket is always there reducing the original issue.
But in some cases it seems that the daemon fails to start.

> Cheers,
> Lukas
> 
> >  g_timeout_add_seconds(1, vdagent_init_async_cb, agent);
> >  return G_SOURCE_REMOVE;
> >  }
> 

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


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

2018-11-19 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 | 6 ++
 1 file changed, 6 insertions(+)

Changes since v1:
- log error when we reach the limit

diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
index f7c8b72..7a58150 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,11 @@ 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) {
+syslog(LOG_ERR, "Attempted to contact daemon for 5 minutes, giving 
up");
+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 vd_agent_linux] vdagent: Stop trying to connect to the daemon after a while

2018-11-19 Thread Lukáš Hrázký
Hi,

On Fri, 2018-11-16 at 16:21 +, Frediano Ziglio wrote:
> 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;
> +}

Please add a log message saying you're giving up trying to connect...

Though would it not be better to just increase the interval to ~30-60
seconds instead of giving up altogether after some amount of quick
tries?

Cheers,
Lukas

>  g_timeout_add_seconds(1, vdagent_init_async_cb, agent);
>  return G_SOURCE_REMOVE;
>  }
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel