I''m currently working with xen-unstable changeset 25260:0f53540494f7
and trying to add a `vncviewer` type to the libxl_types.idl, but
having difficulties understanding the root of a failure that occurs
after I add my changes. Hopefully I enclose enough detail below, but
if not, I''m happy to provide more information.
I''ve created a small patch to add a ''vncviewer'' bool
type to the
configuration parser of xl and based these changes on the "localtime"
implementation part of c/s 25131, see below:
diff -r 0f53540494f7 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Fri May 04 11:18:48 2012 +0100
+++ b/tools/libxl/libxl_create.c Sat May 05 21:27:41 2012 +0000
@@ -156,6 +156,7 @@ int libxl__domain_build_info_setdefault(
if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
b_info->target_memkb = b_info->max_memkb;
+ libxl_defbool_setdefault(&b_info->vncviewer, false);
libxl_defbool_setdefault(&b_info->localtime, false);
libxl_defbool_setdefault(&b_info->disable_migrate, false);
diff -r 0f53540494f7 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Fri May 04 11:18:48 2012 +0100
+++ b/tools/libxl/libxl_types.idl Sat May 05 21:27:41 2012 +0000
@@ -250,6 +250,7 @@ libxl_domain_build_info = Struct("domain
("video_memkb", MemKB),
("shadow_memkb", MemKB),
("rtc_timeoffset", uint32),
+ ("vncviewer", libxl_defbool),
("localtime", libxl_defbool),
("disable_migrate", libxl_defbool),
("cpuid", libxl_cpuid_policy_list),
diff -r 0f53540494f7 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Fri May 04 11:18:48 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Sat May 05 21:27:41 2012 +0000
@@ -728,6 +728,7 @@ static void parse_config_data(const char
if (!xlu_cfg_get_long(config, "rtc_timeoffset", &l, 0))
b_info->rtc_timeoffset = l;
+ xlu_cfg_get_defbool(config, "vncviewer",
&b_info->localtime, 0);
xlu_cfg_get_defbool(config, "localtime",
&b_info->localtime, 0);
if (!xlu_cfg_get_long (config, "videoram", &l, 0))
The outcome of this patch when applied and compiled in is that xl
aborts during the parse_config_data() call in the create_domain().
# ./xl create win7
Parsing config file win7
Aborted
This abort is the `default` case in the switch at xl_cmdimpl.c:736,
which gets triggered from an erroneous b_info->type with a bogus value
of 0x84 (which is neither PV nor HVM.)
For reference, the backtrace is:
(gdb) bt
#0 0xb7fe2416 in __kernel_vsyscall ()
#1 0xb7e28781 in *__GI_raise (sig=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2 0xb7e2bbb2 in *__GI_abort () at abort.c:92
#3 0x0804fe4b in parse_config_data (configfile_filename_report=<value
optimized out>, configfile_data=<value optimized out>,
configfile_len=250, d_config=0xbffff55c) at xl_cmdimpl.c:841
#4 0x080526ba in create_domain (dom_info=<value optimized out>) at
xl_cmdimpl.c:1646
#5 0x0805c098 in main_create (argc=2, argv=0xbffffcc8) at
xl_cmdimpl.c:3470
#6 0x0804cfe4 in main (argc=3, argv=0xbffffcc4) at xl.c:176
I''ve also attached the win7 config file which I''m using, in
case this
is a side effect of something wrong in config file itself (I should
add that it''s mostly copied from an example and it successfully
creates a win7 domain without my patch applied.)
Any guesses or suggestions how to troubleshoot this further would be
greatly appreciated.
Thanks,
Goncalo
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
On Sat, 2012-05-05 at 19:19 -0400, Goncalo Gomes wrote:> diff -r 0f53540494f7 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Fri May 04 11:18:48 2012 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Sat May 05 21:27:41 2012 +0000 > @@ -728,6 +728,7 @@ static void parse_config_data(const char > if (!xlu_cfg_get_long(config, "rtc_timeoffset", &l, 0)) > b_info->rtc_timeoffset = l; > > + xlu_cfg_get_defbool(config, "vncviewer", &b_info->localtime, 0); > xlu_cfg_get_defbool(config, "localtime", &b_info->localtime, 0);You have a small typo here (localtime instead of vncviewer), but that won''t effect the crux of this issue. I''ve tried reproducing using your config file with the patch applied and I can''t.> [...] > This abort is the `default` case in the switch at xl_cmdimpl.c:736, > which gets triggered from an erroneous b_info->type with a bogus value > of 0x84 (which is neither PV nor HVM.)I think it might be useful to sprinkle prints of b_info->type everywhere from the call to libxl_domain_build_info_init_type up until this switch statement to see if you can identify the line which is overwriting this field. I couldn''t spot it by eye but something in there is presumably blowing off the end of a buffer or something similar. You should probably also validate the initial value, which comes from c_info->type, and if that is wrong trace it back to the place which sets that initial value. Ian.
On Sun, 06 May 2012, Ian Campbell wrote:> I''ve tried reproducing using your config file with the patch applied and > I can''t. > > > [...] > > This abort is the `default` case in the switch at xl_cmdimpl.c:736, > > which gets triggered from an erroneous b_info->type with a bogus value > > of 0x84 (which is neither PV nor HVM.) > > I think it might be useful to sprinkle prints of b_info->type everywhere > from the call to libxl_domain_build_info_init_type up until this switch > statement to see if you can identify the line which is overwriting this > field. I couldn''t spot it by eye but something in there is presumably > blowing off the end of a buffer or something similar. > > You should probably also validate the initial value, which comes from > c_info->type, and if that is wrong trace it back to the place which sets > that initial value.Running ''make install'' after various attempts seems to have done it. I was all along under the impression that the dynamic linker would pick the libxenlight in the current dir, which is why I hadn''t run ''make install'' every so often, but did instead occasionally run ''make clean all'' Coming to think of it, if the DLD was to pick the library from the current directory, a world of security bugs and pain would ensue, but for some reason I kept expecting the behaviour to be that. Thanks though! Goncalo
On Sun, 2012-05-06 at 22:23 +0100, Goncalo Gomes wrote:> On Sun, 06 May 2012, Ian Campbell wrote: > > > I''ve tried reproducing using your config file with the patch applied and > > I can''t. > > > > > [...] > > > This abort is the `default` case in the switch at xl_cmdimpl.c:736, > > > which gets triggered from an erroneous b_info->type with a bogus value > > > of 0x84 (which is neither PV nor HVM.) > > > > I think it might be useful to sprinkle prints of b_info->type everywhere > > from the call to libxl_domain_build_info_init_type up until this switch > > statement to see if you can identify the line which is overwriting this > > field. I couldn''t spot it by eye but something in there is presumably > > blowing off the end of a buffer or something similar. > > > > You should probably also validate the initial value, which comes from > > c_info->type, and if that is wrong trace it back to the place which sets > > that initial value. > > Running ''make install'' after various attempts seems to have done it. > > I was all along under the impression that the dynamic linker would > pick the libxenlight in the current dir, which is why I hadn''t run > ''make install'' every so often, but did instead occasionally run ''make > clean all'' Coming to think of it, if the DLD was to pick the library > from the current directory, a world of security bugs and pain would > ensue, but for some reason I kept expecting the behaviour to be that.Yeah, "." is not normally in the default library lookup path for a variety of security etc issues as you suspected.You can either use LD_LIBRARY_PATH=stuff:other-stuff or just run make install. Ian.
On Tue, May 8, 2012 at 11:55 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> Yeah, "." is not normally in the default library lookup path for a > variety of security etc issues as you suspected.You can either use > LD_LIBRARY_PATH=stuff:other-stuff or just run make install....or if you''re on a .deb-based system, "make deb && dpkg -i dist/xen-*.deb". That makes a nice easy way to un-install as well. -George