Xen devel - Apr 2011 - Fwd:[PATCH-may be a bug ignored] tools/libxl/libxl__build_device_model_args_new/old: The condition is so rigorous that user has no chance to disable the vnc

In my spice testing,
I find user have no way to disable vnc in qemu-old and new using xl,
no matter how to set
"vnc vncunused vncpasswd vncdisplay vnclisten" in vm cfg file.

I think this may be a bug ignored.

Signed-off-by: Zhou Peng <zhoupeng@nfs.iscas.ac.cn>

tools/libxl/libxl__build_device_model_args_new/old: The condition is
so rigorous that user has no chance to disable the vnc,
considering what has been done in parse_config_data() by default,
which is not resonable with vnc option in vm-cfg file.

I think, If user explicitly set "vnc=0", vnc should be disabled.
User should have the chance to only use sdl, other remote
display(spice) and even nothing.

diff -r 41879bb8d174 -r 6871474a2a09 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Fri Apr 15 09:51:14 2011 +0800
+++ b/tools/libxl/libxl_dm.c	Fri Apr 15 10:06:59 2011 +0800
@@ -55,7 +55,7 @@ static char ** libxl__build_device_model
     if (info->dom_name)
         flexarray_vappend(dm_args, "-domain-name", info->dom_name,
NULL);

-    if (info->vnc || info->vncdisplay || info->vnclisten ||
info->vncunused) {
+    if (info->vnc) {
         char *vncarg;
         if (info->vncdisplay) {
             if (info->vnclisten && strchr(info->vnclisten,
'':'') == NULL) {
@@ -205,7 +205,7 @@ static char ** libxl__build_device_model
     if (info->dom_name) {
         flexarray_vappend(dm_args, "-name", info->dom_name, NULL);
     }
-    if (info->vnc || info->vncdisplay || info->vnclisten ||
info->vncunused) {
+    if (info->vnc) {
         int display = 0;
         const char *listen = "127.0.0.1";
-- 
Zhou Peng
Operating System Technology Group
Institute of Software, the Chinese Academy of Sciences (ISCAS)


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

On Wed, 20 Apr 2011, ZhouPeng wrote:
> In my spice testing,
> I find user have no way to disable vnc in qemu-old and new using xl,
> no matter how to set
> "vnc vncunused vncpasswd vncdisplay vnclisten" in vm cfg file.
> 
> I think this may be a bug ignored.
> 
> Signed-off-by: Zhou Peng <zhoupeng@nfs.iscas.ac.cn>
> 
> tools/libxl/libxl__build_device_model_args_new/old: The condition is
> so rigorous that user has no chance to disable the vnc,
> considering what has been done in parse_config_data() by default,
> which is not resonable with vnc option in vm-cfg file.
> 
> I think, If user explicitly set "vnc=0", vnc should be disabled.
> User should have the chance to only use sdl, other remote
> display(spice) and even nothing.
> 

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


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

Thanks.

2011/4/20 Stefano Stabellini
<stefano.stabellini@eu.citrix.com>:
> On Wed, 20 Apr 2011, ZhouPeng wrote:
>> In my spice testing,
>> I find user have no way to disable vnc in qemu-old and new using xl,
>> no matter how to set
>> "vnc vncunused vncpasswd vncdisplay vnclisten" in vm cfg
file.
>>
>> I think this may be a bug ignored.
>>
>> Signed-off-by: Zhou Peng <zhoupeng@nfs.iscas.ac.cn>
>>
>> tools/libxl/libxl__build_device_model_args_new/old: The condition is
>> so rigorous that user has no chance to disable the vnc,
>> considering what has been done in parse_config_data() by default,
>> which is not resonable with vnc option in vm-cfg file.
>>
>> I think, If user explicitly set "vnc=0", vnc should be
disabled.
>> User should have the chance to only use sdl, other remote
>> display(spice) and even nothing.
>>
>
>
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
>


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

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

ZhouPeng writes ("[Xen-devel] Fwd:[PATCH-may be a bug ignored]
tools/libxl/libxl__build_device_model_args_new/old: The condition is so rigorous
that user has no chance to disable the vnc"):
> In my spice testing,
> I find user have no way to disable vnc in qemu-old and new using xl,
> no matter how to set
> "vnc vncunused vncpasswd vncdisplay vnclisten" in vm cfg file.
Does your patch not have the effect of turning vnc off by default ?  I
don''t think that''s correct.

Ian.

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

2011/5/9 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> ZhouPeng writes ("[Xen-devel] Fwd:[PATCH-may be a bug ignored]
tools/libxl/libxl__build_device_model_args_new/old: The condition is so rigorous
that user has no chance to disable the vnc"):
>> In my spice testing,
>> I find user have no way to disable vnc in qemu-old and new using xl,
>> no matter how to set
>> "vnc vncunused vncpasswd vncdisplay vnclisten" in vm cfg
file.
>
> Does your patch not have the effect of turning vnc off by default ?  I
> don''t think that''s correct.
pls see below:
parse_config_data(...)  < xl_cmdimpl.c >
       libxl_init_dm_info(...) set dm_info->vnc = 1
       ...
       if (!xlu_cfg_get_long(config, "vnc", &l))
                dm_info->vnc = l

So, The patch will turn vnc off  by default,
Because dm_info->vnc = 1 in libxl_init_dm_info(...) turn on vnc in default,
My patch turns off vnc only when user ''set vnc = 0'' explicitly
in vm cfg file.

And testing in xen-unstable, it works.

Thanks,
> Ian.
>


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

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

2011/5/10 ZhouPeng <zpengxen@gmail.com>:
> 2011/5/9 Ian Jackson <Ian.Jackson@eu.citrix.com>:
>> ZhouPeng writes ("[Xen-devel] Fwd:[PATCH-may be a bug ignored]
tools/libxl/libxl__build_device_model_args_new/old: The condition is so rigorous
that user has no chance to disable the vnc"):
>>> In my spice testing,
>>> I find user have no way to disable vnc in qemu-old and new using
xl,
>>> no matter how to set
>>> "vnc vncunused vncpasswd vncdisplay vnclisten" in vm cfg
file.
>>
>> Does your patch not have the effect of turning vnc off by default ?  I
>> don''t think that''s correct.
>
> pls see below:
> parse_config_data(...)  < xl_cmdimpl.c >
>       libxl_init_dm_info(...) set dm_info->vnc = 1
>       ...
>       if (!xlu_cfg_get_long(config, "vnc", &l))
>                dm_info->vnc = l
>
> So, The patch will not turn vnc off  by default,
So, The patch will not turn vnc off  by default. ...typo mistake
:)
> Because dm_info->vnc = 1 in libxl_init_dm_info(...) turn on vnc in
default,
> My patch turns off vnc only when user ''set vnc = 0''
explicitly in vm cfg file.
>
> And testing in xen-unstable, it works.
>
> Thanks,
>> Ian.
>>
>
>
>
> --
> Zhou Peng
> Operating System Technology Group
> Institute of Software, the Chinese Academy of Sciences (ISCAS)
>


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

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

2011/5/9 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> ZhouPeng writes ("[Xen-devel] Fwd:[PATCH-may be a bug ignored]
tools/libxl/libxl__build_device_model_args_new/old: The condition is so rigorous
that user has no chance to disable the vnc"):
>> In my spice testing,
>> I find user have no way to disable vnc in qemu-old and new using xl,
>> no matter how to set
>> "vnc vncunused vncpasswd vncdisplay vnclisten" in vm cfg
file.
>
> Does your patch not have the effect of turning vnc off by default ?  I
> don''t think that''s correct.
Reply again because I suspect I misunderstood you.
Do you mean the vnc should be turned off by default or not?

In my patch, I assume the vnc should be turned on by default.
-- 
Zhou Peng
Operating System Technology Group
Institute of Software, the Chinese Academy of Sciences (ISCAS)

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

Stefano Stabellini writes ("[Xen-devel] Re: Fwd:[PATCH-may be a bug
ignored] tools/libxl/libxl__build_device_model_args_new/old: The condition is so
rigorous that user has no chance to disable the
vnc"):
> On Wed, 20 Apr 2011, ZhouPeng wrote:
> > tools/libxl/libxl__build_device_model_args_new/old: The condition is
> > so rigorous that user has no chance to disable the vnc,
> > considering what has been done in parse_config_data() by default,
> > which is not resonable with vnc option in vm-cfg file.
> > 
> > I think, If user explicitly set "vnc=0", vnc should be
disabled.
> > User should have the chance to only use sdl, other remote
> > display(spice) and even nothing.
...
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Thanks.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

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