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] 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

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

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

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

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

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

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