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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 18 Apr 2011, ZhouPeng wrote:
> Signed-off-by: Zhou Peng <zhoupeng@xxxxxxxxxxxxxxx>
> 
> This patch allows you to play with 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 configuration file.
> #spice
> spice=1
> spiceport=6000
> spicehost='192.168.1.187'
> spicedisable_ticketing = 0 # default is 0
> spicepasswd = 'password'
> 
> apic=0 # disable acpi, but if you used the appended patch, set acpi=0
> 
> You may need to disable acpi(I'm not sure),
> but if you want to disable acpi, you may need to set
> apic = 0, (Yes, It is apic not acpi, pls don't ask me why, because I am also 
> confused with it).
> If you feel uncomfortable by setting apic = 0, you can try an additional 
> patch appended,
> then you can use acpi=0 in vm cfg file to give "no-acpi" argument to qemu.
> 
> For detailed:
> http://code.google.com/p/spice4xen/wiki/Using_Upstream_Qemu

Cool! Does it mean that it works right now?


 
> diff -r 3f00c5faa12a tools/libxl/libxl.idl
> --- a/tools/libxl/libxl.idlÂÂÂ Wed Apr 13 16:10:26 2011 +0100
> +++ b/tools/libxl/libxl.idlÂÂÂ Mon Apr 18 10:52:09 2011 +0800
> @@ -153,6 +153,13 @@ 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 passwords never expires"),
> +ÂÂÂ ("spiceagent_mouset",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 3f00c5faa12a tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.cÂÂÂ Wed Apr 13 16:10:26 2011 +0100
> +++ b/tools/libxl/libxl_dm.cÂÂÂ Mon Apr 18 10:52:09 2011 +0800
> @@ -225,15 +225,44 @@ static char ** libxl__build_device_model
> Â
> ÂÂÂÂÂÂÂÂ if (strchr(listen, ':') != NULL)
> ÂÂÂÂÂÂÂÂÂÂÂÂ flexarray_append(dm_args,
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ libxl__sprintf(gc, "%s%s", listen,
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ info->vncunused ? ",to=99" : ""));
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ libxl__sprintf(gc, "%s%s,%s", listen,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ info->vncunused ? ",to=99" : "", info->vncpasswd));
> ÂÂÂÂÂÂÂÂ else
> ÂÂÂÂÂÂÂÂÂÂÂÂ flexarray_append(dm_args,
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ libxl__sprintf(gc, "%s:%d%s", listen, display,
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ info->vncunused ? ",to=99" : ""));
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ libxl__sprintf(gc, "%s:%d%s,%s", listen, display,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ info->vncunused ? ",to=99" : "", info->vncpasswd));

This is not actually part of the spice support patch to libxl, is it?
It looks like a generic bug fix to the vncpasswd handling.
Could you please send it as a separate patch?


> ÂÂÂÂ }
> ÂÂÂÂ if (info->sdl) {
> ÂÂÂÂÂÂÂÂ flexarray_append(dm_args, "-sdl");
> +ÂÂÂ }
> +ÂÂÂ if (info->spice) {
> +ÂÂÂÂÂÂÂ char *spiceoptions = NULL;
> +ÂÂÂÂÂÂÂ if (!info->spiceport && !info->spicetls_port) {
> +ÂÂÂÂÂÂÂÂÂÂÂ assert(!"at least one of the spiceport or tls_port must be 
> provided");
> +ÂÂÂÂÂÂÂ }

please return error here and let the caller handle the failure


> +
> +ÂÂÂÂÂÂÂ if (!info->spicedisable_ticketing) {
> +ÂÂÂÂÂÂÂÂÂÂÂ if (!info->spicepasswd)
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ assert(!"spice ticketing is enabled but missing password");
> +ÂÂÂÂÂÂÂÂÂÂÂ else if (!info->spicepasswd[0])
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ assert(!"missing code for supplying spice password");
> +ÂÂÂÂÂÂÂ }

same here: replace the asserts with return errors


> +ÂÂÂÂÂÂÂ spiceoptions = libxl__sprintf(gc, "port=%d,tls-port=%d",
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ info->spiceport, info->spicetls_port);
> +ÂÂÂÂÂÂÂ if (!info->spicehost)
> +ÂÂÂÂÂÂÂÂÂÂÂ spiceoptions = libxl__sprintf(gc,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "%s,host=%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_mouset ? "on" : 
> "off");
> +
> +ÂÂÂÂÂÂÂ flexarray_append(dm_args, "-spice");
> +ÂÂÂÂÂÂÂ flexarray_append(dm_args, spiceoptions);
> + printf("SPICE Options:\n -spice %s\n", spiceoptions);
> ÂÂÂÂ }
> Â
> ÂÂÂÂ if (info->type == XENPV && !info->nographic) {
> diff -r 3f00c5faa12a tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.cÂÂÂ Wed Apr 13 16:10:26 2011 +0100
> +++ b/tools/libxl/xl_cmdimpl.cÂÂÂ Mon Apr 18 10:52:10 2011 +0800
> @@ -1089,6 +1089,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_mouset = l;
> +ÂÂÂÂÂÂÂ else
> +ÂÂÂÂÂÂÂÂÂÂÂ dm_info->spiceagent_mouset = 1;
> ÂÂÂÂÂÂÂÂ if (!xlu_cfg_get_long (config, "nographic", &l))
> ÂÂÂÂÂÂÂÂÂÂÂÂ dm_info->nographic = l;
> ÂÂÂÂÂÂÂÂ if (!xlu_cfg_get_long (config, "gfx_passthru", &l))
> 
> 
> ==============================Appended patch======================

thanks for the patches, next time could you please send the patches
inline as plain text, each patch as a separate email?
See this document on how to send patches to the LKML as a reference:

http://www.mjmwired.net/kernel/Documentation/email-clients.txt


> Signed-off-by: Zhou Peng <zhoupeng@xxxxxxxxxxxxxxx>
> 
> tool/libxl: mistake apic for acpi in libxl__build_device_model_args_old/new
> It may be advisedly coded for some reason, then it can be a mistake of my 
> understanding.
> 

Thanks for the patch, I found the first version that you sent and reply
to it now.

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