Re: [PATCH] Play with spice for xen-upstream-qemu on upstream Xen

[ZhouPeng <zpengxen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 8480 bytes --]

> I was just about to apply this when I noticed a couple of
> oddities:
Fixed in this patch.

>> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
>> +                       ": at least one of the spiceport or tls_port must be provided");
>
> punctuation and I think if you run this it will produce output like:
>  libxl: something: : at least one of the spiceport ....
>
> Did you add the ": " after testing ?  If so then perhaps the existing
> logging functions are wrong.
The output is like this,
libxl: something : at least one of the spiceport ....

It is
void libxl__logv(...)
    libxl__logv
        xtl_log(ctx->lg, msglevel, errnoval, "libxl",
                "%s%s%s%s" "%s",
                fileline, func&&file?":":"", func?func:"",
func||file?" ":"", // here

output the msg.
It use a space not ': ' between func name and log msg.
So no bug
But I feel the format like below is more clear
void libxl__logv(...)
    libxl__logv
        xtl_log(ctx->lg, msglevel, errnoval, "libxl",
                "%s%s%s%s" "%s",
                fileline, func&&file?":":"", func?func:"",
func||file?": ":"",   // here

If you reply to agree to use ': ' instead of ' ',
I will send a little patch for this.

Any way, I agree with you to  trim the header ':' like below
": at least one of the spiceport or tls_port must be provided" to
"at least one of the spiceport or tls_port must be provided"

> Secondly, your patch has a lot of rather long lines in new code.  Can
> you please try to keep your lines down to 75 characters (or 80 if you
> absolutely must) ?
Fixed in this patch.

There are many lines up to 80 characters in libxl.idl
and even in libxl_dm.c, that's why I turn a blind eye
to libxl.idl in my last patch.

I think I will send a patch for libxl.idl to cut down lines to 80,
after this patch applied.

Thanks.
-----
Signed-off-by: Zhou Peng <zhoupeng@nfs.iscas.ac.cn>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

This patch allows you to use spice for
xen-upstream-qemu on upstream Xen or released Xen-4.1.0.

Nothing need to be modified in xen-upstream-qemu,
because qemu has include spice's code as a new feature since qemu-0.14.

= Usage =

Add spice fields in VM cfg file.

e.g.

spice=1
spiceport=6000
spicehost='192.168.1.187'
spicedisable_ticketing = 0 # default is 0
spicepasswd = 'password'
spiceagent_mouse = 1 # default is 1
------------------------------------------------------------------------------

diff -r fb517cc27ade -r a19b590873e6 tools/libxl/libxl.idl
--- a/tools/libxl/libxl.idl	Fri May 20 18:20:09 2011 +0100
+++ b/tools/libxl/libxl.idl	Wed May 25 21:21:55 2011 +0800
@@ -210,6 +210,22 @@ libxl_device_model_info = Struct("device
     ("keymap",           string,            False, "set keyboard
layout, default is en-us keyboard"),
     ("sdl",              bool,              False, "sdl enabled or disabled"),
     ("opengl",           bool,              False, "opengl enabled or
disabled (if enabled requires sdl enabled)"),
+    ("spice",            bool,              False,
+    "spice enabled or disabled"),
+    ("spiceport",        integer,           False,
+    "the port that should be listened on for the spice server"),
+    ("spicetls_port",    integer,           False, """the tls port
+that should be listened on for the spice server,
+at least one of the port or tls port must be given"""),
+    ("spicehost",        string,            False, """the interface
+that should be listened on if given otherwise any interface"""),
+    ("spicedisable_ticketing", bool,        False,
+    "enable client connection with no password"),
+    ("spicepasswd",      string,            False, """set ticket password
+witch must be used by a client for connection.
+The password never expires"""),
+    ("spiceagent_mouse", bool,              False,
+    "Whether spice agent is used for client mouse mode(default is on)"),
     ("nographic",        bool,              False, "no graphics, use
serial port"),
     ("gfx_passthru",     bool,              False, "graphics
passthrough enabled or disabled"),
     ("serial",           string,            False, "serial port
re-direct to pty deivce"),
diff -r fb517cc27ade -r a19b590873e6 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Fri May 20 18:20:09 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Wed May 25 21:21:55 2011 +0800
@@ -281,6 +281,43 @@ static char ** libxl__build_device_model
     }
     if (info->sdl) {
         flexarray_append(dm_args, "-sdl");
+    }
+    if (info->spice) {
+        char *spiceoptions = NULL;
+        if (!info->spiceport && !info->spicetls_port) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                "at least one of the spiceport or tls_port must be provided");
+            return NULL;
+        }
+
+        if (!info->spicedisable_ticketing) {
+            if (!info->spicepasswd) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                    "spice ticketing is enabled but missing password");
+                return NULL;
+            }
+            else if (!info->spicepasswd[0]) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                    "spice password can't be empty");
+                return NULL;
+            }
+        }
+        spiceoptions = libxl__sprintf(gc, "port=%d,tls-port=%d",
+                       info->spiceport, info->spicetls_port);
+        if (info->spicehost)
+            spiceoptions = libxl__sprintf(gc,
+                    "%s,addr=%s", spiceoptions, info->spicehost);
+        if (info->spicedisable_ticketing)
+            spiceoptions = libxl__sprintf(gc, "%s,disable-ticketing",
+                                               spiceoptions);
+        else
+            spiceoptions = libxl__sprintf(gc,
+                    "%s,password=%s", spiceoptions, info->spicepasswd);
+        spiceoptions = libxl__sprintf(gc, "%s,agent-mouse=%s", spiceoptions,
+                                      info->spiceagent_mouse ? "on" : "off");
+
+        flexarray_append(dm_args, "-spice");
+        flexarray_append(dm_args, spiceoptions);
     }

     if (info->type == LIBXL_DOMAIN_TYPE_PV && !info->nographic) {
diff -r fb517cc27ade -r a19b590873e6 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Fri May 20 18:20:09 2011 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Wed May 25 21:21:55 2011 +0800
@@ -367,6 +367,13 @@ static void printf_info(int domid,
         printf("\t\t\t(usb %d)\n", dm_info->usb);
         printf("\t\t\t(usbdevice %s)\n", dm_info->usbdevice);
         printf("\t\t\t(acpi %d)\n", dm_info->acpi);
+        printf("\t\t\t(spice %d)\n", dm_info->spice);
+        printf("\t\t\t(spiceport %d)\n", dm_info->spiceport);
+        printf("\t\t\t(spicetls_port %d)\n", dm_info->spicetls_port);
+        printf("\t\t\t(spicehost %s)\n", dm_info->spicehost);
+        printf("\t\t\t(spicedisable_ticketing %d)\n",
+                    dm_info->spicedisable_ticketing);
+        printf("\t\t\t(spiceagent_mouse %d)\n", dm_info->spiceagent_mouse);
         printf("\t\t)\n");
     } else {
         printf("\t\t(linux %d)\n", b_info->hvm);
@@ -1124,6 +1131,20 @@ skip_vfb:
             dm_info->sdl = l;
         if (!xlu_cfg_get_long (config, "opengl", &l))
             dm_info->opengl = l;
+        if (!xlu_cfg_get_long (config, "spice", &l))
+            dm_info->spice = l;
+        if (!xlu_cfg_get_long (config, "spiceport", &l))
+            dm_info->spiceport = l;
+        if (!xlu_cfg_get_long (config, "spicetls_port", &l))
+            dm_info->spicetls_port = l;
+        xlu_cfg_replace_string (config, "spicehost", &dm_info->spicehost);
+        if (!xlu_cfg_get_long (config, "spicedisable_ticketing", &l))
+            dm_info->spicedisable_ticketing = l;
+        xlu_cfg_replace_string (config, "spicepasswd", &dm_info->spicepasswd);
+        if (!xlu_cfg_get_long (config, "spiceagent_mouse", &l))
+            dm_info->spiceagent_mouse = l;
+        else
+            dm_info->spiceagent_mouse = 1;
         if (!xlu_cfg_get_long (config, "nographic", &l))
             dm_info->nographic = l;
         if (!xlu_cfg_get_long (config, "gfx_passthru", &l))

-- 
Zhou Peng
Operating System Technology Group
Institute of Software, the Chinese Academy of Sciences (ISCAS)

[-- Attachment #2: spice-upstreamXen-upstreamQemu.diff --]
[-- Type: text/x-patch, Size: 6349 bytes --]

Signed-off-by: Zhou Peng <zhoupeng@nfs.iscas.ac.cn>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

This patch allows you to use spice for
xen-upstream-qemu on upstream Xen or released Xen-4.1.0.

Nothing need to be modified in xen-upstream-qemu, 
because qemu has include spice's code as a new feature since qemu-0.14.

= Usage =

Add spice fields in VM cfg file.

e.g.

spice=1
spiceport=6000
spicehost='192.168.1.187'
spicedisable_ticketing = 0 # default is 0
spicepasswd = 'password'
spiceagent_mouse = 1 # default is 1
------------------------------------------------------------------------------

diff -r fb517cc27ade -r a19b590873e6 tools/libxl/libxl.idl
--- a/tools/libxl/libxl.idl	Fri May 20 18:20:09 2011 +0100
+++ b/tools/libxl/libxl.idl	Wed May 25 21:21:55 2011 +0800
@@ -210,6 +210,22 @@ libxl_device_model_info = Struct("device
     ("keymap",           string,            False, "set keyboard layout, default is en-us keyboard"),
     ("sdl",              bool,              False, "sdl enabled or disabled"),
     ("opengl",           bool,              False, "opengl enabled or disabled (if enabled requires sdl enabled)"),
+    ("spice",            bool,              False,
+    "spice enabled or disabled"),
+    ("spiceport",        integer,           False, 
+    "the port that should be listened on for the spice server"),
+    ("spicetls_port",    integer,           False, """the tls port
+that should be listened on for the spice server, 
+at least one of the port or tls port must be given"""),
+    ("spicehost",        string,            False, """the interface
+that should be listened on if given otherwise any interface"""),
+    ("spicedisable_ticketing", bool,        False, 
+    "enable client connection with no password"),
+    ("spicepasswd",      string,            False, """set ticket password
+witch must be used by a client for connection.
+The password never expires"""),
+    ("spiceagent_mouse", bool,              False, 
+    "Whether spice agent is used for client mouse mode(default is on)"),
     ("nographic",        bool,              False, "no graphics, use serial port"),
     ("gfx_passthru",     bool,              False, "graphics passthrough enabled or disabled"),
     ("serial",           string,            False, "serial port re-direct to pty deivce"),
diff -r fb517cc27ade -r a19b590873e6 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Fri May 20 18:20:09 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Wed May 25 21:21:55 2011 +0800
@@ -281,6 +281,43 @@ static char ** libxl__build_device_model
     }
     if (info->sdl) {
         flexarray_append(dm_args, "-sdl");
+    }
+    if (info->spice) {
+        char *spiceoptions = NULL;
+        if (!info->spiceport && !info->spicetls_port) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                "at least one of the spiceport or tls_port must be provided");
+            return NULL;
+        }
+
+        if (!info->spicedisable_ticketing) {
+            if (!info->spicepasswd) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                    "spice ticketing is enabled but missing password");
+                return NULL;
+            }
+            else if (!info->spicepasswd[0]) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                    "spice password can't be empty");
+                return NULL;
+            }
+        }
+        spiceoptions = libxl__sprintf(gc, "port=%d,tls-port=%d",
+                       info->spiceport, info->spicetls_port);
+        if (info->spicehost)
+            spiceoptions = libxl__sprintf(gc,
+                    "%s,addr=%s", spiceoptions, info->spicehost);
+        if (info->spicedisable_ticketing)
+            spiceoptions = libxl__sprintf(gc, "%s,disable-ticketing",
+                                               spiceoptions);
+        else
+            spiceoptions = libxl__sprintf(gc,
+                    "%s,password=%s", spiceoptions, info->spicepasswd);
+        spiceoptions = libxl__sprintf(gc, "%s,agent-mouse=%s", spiceoptions,
+                                      info->spiceagent_mouse ? "on" : "off");
+
+        flexarray_append(dm_args, "-spice");
+        flexarray_append(dm_args, spiceoptions);
     }
 
     if (info->type == LIBXL_DOMAIN_TYPE_PV && !info->nographic) {
diff -r fb517cc27ade -r a19b590873e6 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Fri May 20 18:20:09 2011 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Wed May 25 21:21:55 2011 +0800
@@ -367,6 +367,13 @@ static void printf_info(int domid,
         printf("\t\t\t(usb %d)\n", dm_info->usb);
         printf("\t\t\t(usbdevice %s)\n", dm_info->usbdevice);
         printf("\t\t\t(acpi %d)\n", dm_info->acpi);
+        printf("\t\t\t(spice %d)\n", dm_info->spice);
+        printf("\t\t\t(spiceport %d)\n", dm_info->spiceport);
+        printf("\t\t\t(spicetls_port %d)\n", dm_info->spicetls_port);
+        printf("\t\t\t(spicehost %s)\n", dm_info->spicehost);
+        printf("\t\t\t(spicedisable_ticketing %d)\n",
+                    dm_info->spicedisable_ticketing);
+        printf("\t\t\t(spiceagent_mouse %d)\n", dm_info->spiceagent_mouse);
         printf("\t\t)\n");
     } else {
         printf("\t\t(linux %d)\n", b_info->hvm);
@@ -1124,6 +1131,20 @@ skip_vfb:
             dm_info->sdl = l;
         if (!xlu_cfg_get_long (config, "opengl", &l))
             dm_info->opengl = l;
+        if (!xlu_cfg_get_long (config, "spice", &l))
+            dm_info->spice = l;
+        if (!xlu_cfg_get_long (config, "spiceport", &l))
+            dm_info->spiceport = l;
+        if (!xlu_cfg_get_long (config, "spicetls_port", &l))
+            dm_info->spicetls_port = l;
+        xlu_cfg_replace_string (config, "spicehost", &dm_info->spicehost);
+        if (!xlu_cfg_get_long (config, "spicedisable_ticketing", &l))
+            dm_info->spicedisable_ticketing = l;
+        xlu_cfg_replace_string (config, "spicepasswd", &dm_info->spicepasswd);
+        if (!xlu_cfg_get_long (config, "spiceagent_mouse", &l))
+            dm_info->spiceagent_mouse = l;
+        else
+            dm_info->spiceagent_mouse = 1;
         if (!xlu_cfg_get_long (config, "nographic", &l))
             dm_info->nographic = l;
         if (!xlu_cfg_get_long (config, "gfx_passthru", &l))

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel