Re: [PATCH qemu 1/2] add the ability to specify Spice keepalive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey,

On Tue, Dec 01, 2015 at 01:45:07PM +0900, Sunny Shin wrote:
> add the ability to specify Spice keepalive
> 
> Signed-off-by: Sunny Shin <sunny4s.git@xxxxxxxxx>
> ---
>  qemu-options.hx |  4 ++++
>  ui/spice-core.c | 15 +++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0eea4ee..bc8fe42 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -995,6 +995,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
>      "       [,streaming-video=[off|all|filter]][,disable-copy-paste]\n"
>      "       [,disable-agent-file-xfer][,agent-mouse=[on|off]]\n"
>      "
> [,playback-compression=[on|off]][,seamless-migration=[on|off]]\n"
> +    "       [,keepalive-timeout=<timeout>]\n"
>      "   enable spice\n"
>      "   at least one of {port, tls-port} is mandatory\n",
>      QEMU_ARCH_ALL)
> @@ -1086,6 +1087,9 @@ Enable/disable audio stream compression (using celt
> 0.5.1).  Default is on.
>  @item seamless-migration=[on|off]
>  Enable/disable spice seamless migration. Default is off.
> 
> +@item keepalive-timeout=[<timeout>]
> +Enable tcp keepalive and set timeout as specified value.
> +
>  @end table
>  ETEXI
> 
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 6a62d71..1ffff9b 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -496,6 +496,9 @@ static QemuOptsList qemu_spice_opts = {
>          }, {
>              .name = "seamless-migration",
>              .type = QEMU_OPT_BOOL,
> +        }, {
> +            .name = "keepalive-timeout",
> +            .type = QEMU_OPT_NUMBER,
>          },
>          { /* end of list */ }
>      },
> @@ -644,6 +647,7 @@ void qemu_spice_init(void)
>      spice_image_compression_t compression;
>      spice_wan_compression_t wan_compr;
>      bool seamless_migration;
> +    int keepalive_timeout;
> 
>      qemu_thread_get_self(&me);
> 
> @@ -794,6 +798,17 @@ void qemu_spice_init(void)
> 
>      seamless_migration = qemu_opt_get_bool(opts, "seamless-migration", 0);
>      spice_server_set_seamless_migration(spice_server, seamless_migration);
> +
> +    keepalive_timeout = qemu_opt_get_number(opts, "keepalive-timeout", 0);
> +    if (keepalive_timeout > 0) {
> +#if SPICE_SERVER_VERSION >= 0x000c06
> +        spice_server_set_keepalive_timeout(spice_server,
> keepalive_timeout);

This will need to be 0x000c07 as 0x000c06 is already released and does
not have this function.

> +#else
> +        error_report("this qemu build does not support the "
> +                     "\"keepalive-timeout\" option");
> +#endif

This is similar to what is done for the disabling of file transfer, but
I'm not sure this is good enough for libvirt support. Maybe they will
prefer that -spice keepalive-timeout does not appear in qemu -help when
support was not compiled in spice-server, and that it appears when it's
fully supported. I've asked for their feedback, and I've already
prepared a patch achieving that, see below. Patch looks good to me otherwise.

Thanks again for working on that!

Christophe

commit 6ed2fefc186c4a2e49ed1fb952a26eb4727588c5
Author: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Date:   Tue Dec 8 18:16:27 2015 +0100

    spice: Don't show keepalive option in --help when not supported
    
    Only newer spice-server supports the keepalive option. QEMU detects at
    compile-time whether it's going to be able to set the keepalive option
    or not. For libvirt integration, it's better if keepalive-timeout does
    not appear in help output when the option is non-functional because
    spice-server was too old at compile-time.

diff --git a/configure b/configure
index b9552fd..524bf48 100755
--- a/configure
+++ b/configure
@@ -3971,6 +3971,9 @@ EOF
     QEMU_CFLAGS="$QEMU_CFLAGS $spice_cflags"
     spice_protocol_version=$($pkg_config --modversion spice-protocol)
     spice_server_version=$($pkg_config --modversion spice-server)
+    if $pkg_config --atleast-version=0.12.7 spice-server; then
+      spice_supports_keepalive="yes"
+    fi
   else
     if test "$spice" = "yes" ; then
       feature_not_found "spice" \
@@ -5202,6 +5205,9 @@ fi
 if test "$spice" = "yes" ; then
   echo "CONFIG_SPICE=y" >> $config_host_mak
 fi
+if test "$spice_supports_keepalive" = "yes" ; then
+  echo "CONFIG_SPICE_KEEPALIVE=y" >> $config_host_mak
+fi
 
 if test "$smartcard" = "yes" ; then
   echo "CONFIG_SMARTCARD=y" >> $config_host_mak
diff --git a/qemu-options.hx b/qemu-options.hx
index bc8fe42..bb8ee62 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -995,7 +995,9 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
     "       [,streaming-video=[off|all|filter]][,disable-copy-paste]\n"
     "       [,disable-agent-file-xfer][,agent-mouse=[on|off]]\n"
     "       [,playback-compression=[on|off]][,seamless-migration=[on|off]]\n"
+#ifdef CONFIG_SPICE_KEEPALIVE
     "       [,keepalive-timeout=<timeout>]\n"
+#endif
     "   enable spice\n"
     "   at least one of {port, tls-port} is mandatory\n",
     QEMU_ARCH_ALL)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index a8a6ee2..437278a 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -647,7 +647,9 @@ void qemu_spice_init(void)
     spice_image_compression_t compression;
     spice_wan_compression_t wan_compr;
     bool seamless_migration;
+#if SPICE_SERVER_VERSION >= 0x000c07
     int keepalive_timeout;
+#endif
 
     qemu_thread_get_self(&me);
 
@@ -728,15 +730,12 @@ void qemu_spice_init(void)
                              tls_ciphers);
     }
 
+#if SPICE_SERVER_VERSION >= 0x000c07
     keepalive_timeout = qemu_opt_get_number(opts, "keepalive-timeout", 0);
     if (keepalive_timeout > 0) {
-#if SPICE_SERVER_VERSION >= 0x000c06
         spice_server_set_keepalive_timeout(spice_server, keepalive_timeout);
-#else
-        error_report("this qemu build does not support the "
-                     "\"keepalive-timeout\" option");
-#endif
     }
+#endif
 
     if (password) {
         qemu_spice_set_passwd(password, false, false);

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]