[Spice-devel] [PATCH 1/1] channel: add optional tcp keepalive timeout to channels

[David Jaša]

Hi,

On Pá, 2015-11-27 at 14:45 +0900, Sunny Shin wrote:
> 
> 
> Hi,
> 
> 
> With firewall running between spice server and client, if idle time is
> larger than firewall session timeout, spice sessions freeze and users
> lose their keyboard and mouse control.
> 
> 
> To workaround this issue, I made a patch to add tcp keepalive timeout
> to spice server. 

cool!

However, I think that your configuration approach won't be accepted, the
proper way to set such a value would be addition of another option=value
pair to -spice parameter of qemu CLI (and associated element under
<graphics> element in libvirt - but I'd leave that to libvirt people
once qemu option gets in).

David

> The timeout can be added to qemu config like below.
>     <domain type='kvm'
> xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
>       <qemu:commandline>
>         <qemu:env name='SPICE_KEEPALIVE_TIMEOUT' value='1800'/>
>       </qemu:commandline>
> 
> 
> I wanted to add this option to spice client, but there was no
> setsockopt() option of TCP_KEEPIDLE for windows platform, So, I ended
> up adding it to spice server. Please review the patch and let me know
> what you think. Thank you.
> 
> 
> ---------------------------------------------------------------------------------------------------
> [PATCH 1/1] channel: add optional tcp keepalive timeout to channels
> 
> 
> Signed-off-by: Sunny Shin <sunny4s.git at gmail.com>
> ---
>  server/reds.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> 
> diff --git a/server/reds.c b/server/reds.c
> index 8b3c3cb..05d0b1d 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2254,6 +2254,9 @@ static RedLinkInfo
> *reds_init_client_connection(int socket)
>      RedLinkInfo *link;
>      int delay_val = 1;
>      int flags;
> +    char *keepalive_timeout_str;
> +    int keepalive_timeout;
> +    int keepalive = 1;
> 
> 
>      if ((flags = fcntl(socket, F_GETFL)) == -1) {
>          spice_warning("accept failed, %s", strerror(errno));
> @@ -2271,6 +2274,31 @@ static RedLinkInfo
> *reds_init_client_connection(int socket)
>          }
>      }
> 
> 
> +    keepalive_timeout_str = getenv("SPICE_KEEPALIVE_TIMEOUT");
> +    if (keepalive_timeout_str != NULL) {
> +        errno = 0;
> +        keepalive_timeout = strtol(keepalive_timeout_str, NULL, 10);
> +        if (errno != 0) {
> +            spice_warning("error parsing SPICE_KEEPALIVE_TIMEOUT: %
> s", strerror(errno));
> +            goto error;
> +        }
> +
> +        spice_debug("keepalive timeout %ds", keepalive_timeout);
> +        if (setsockopt(socket, SOL_SOCKET, SO_KEEPALIVE, &keepalive,
> sizeof(keepalive)) == -1) {
> +            if (errno != ENOTSUP) {
> +                spice_printerr("setsockopt for keepalive failed, %s",
> strerror(errno));
> +                goto error;
> +            }
> +        }
> +        if (setsockopt(socket, SOL_TCP, TCP_KEEPIDLE,
> +                       &keepalive_timeout, sizeof(keepalive_timeout))
> == -1) {
> +            if (errno != ENOTSUP) {
> +                spice_printerr("setsockopt for keepalive timeout
> failed, %s", strerror(errno));
> +                goto error;
> +            }
> +        }
> +    }
> +
>      link = spice_new0(RedLinkInfo, 1);
>      link->stream = reds_stream_new(socket);
> 
> 
> --
> 1.8.3.1
> 
> 
> 
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel