Hi, This is just my experience about issues I encountered when upgrade from xen-4.1-testing changeset 23277:80130491806f to xen-unstable changeset 25191:a95fc7decc83. 1. Immediately after upgrade, xl list show such error: # xl list libxl: error: libxl.c:506:libxl_list_domain: geting domain info list: Permission denied libxl_domain_infolist failed. After a reboot, it is fine. Any idea why such behaviour? Imagine if there are running domUs... this might cause issues to shutdown? I will downgrade and repeat such test to confirm. Might be worth a note in upgrading note about this if this is intended? 2. localtime setting not working. Set to localtime=1 doesn''t seems to work whereby setting rtc_timeoffset works. Any idea? 3. xl trigger hvmdomain power still never remove those related iptables forward rules. This is the same problem when I test in xen-4.1-testing. Refer to http://lists.xen.org/archives/html/xen-devel/2012-02/msg00771.html about past report. When test in xen-4.1-testing, one funny behaviour is if I start hvmdomain using xm then use xl trigger hvmdomain power does remove those iptables related rule chain. I shall do more testing when time permit and sorry if this report is useless to anyone. Thanks. Kindest regards, Giam Teck Choon
Ian Campbell
2012-Apr-12 07:08 UTC
Re: Upgrade from xen-4.1-testing to xen-unstable report
On Wed, 2012-04-11 at 22:23 +0100, Teck Choon Giam wrote:> Hi, > > This is just my experience about issues I encountered when upgrade > from xen-4.1-testing changeset 23277:80130491806f to xen-unstable > changeset 25191:a95fc7decc83. > > 1. Immediately after upgrade, xl list show such error: > > # xl list > libxl: error: libxl.c:506:libxl_list_domain: geting domain info list: > Permission denied > libxl_domain_infolist failed. > > After a reboot, it is fine. Any idea why such behaviour? Imagine if > there are running domUs... this might cause issues to shutdown? I > will downgrade and repeat such test to confirm. Might be worth a note > in upgrading note about this if this is intended?The tools and the hypervisor are a matched pair so you would need to reboot the system in order to use the new tools. This has always been the case with Xen upgrades.> 2. localtime setting not working. Set to localtime=1 doesn''t seems to > work whereby setting rtc_timeoffset works. Any idea?I''ve CC''d Lin Ming who implemented both of those.> 3. xl trigger hvmdomain power still never remove those related > iptables forward rules. This is the same problem when I test in > xen-4.1-testing. Refer to > http://lists.xen.org/archives/html/xen-devel/2012-02/msg00771.html > about past report. When test in xen-4.1-testing, one funny behaviour > is if I start hvmdomain using xm then use xl trigger hvmdomain power > does remove those iptables related rule chain.I think this is an issue with the hotplug scripts not being run at the right time. There are some improvements to this in the pipeline which I hope will help here.> I shall do more testing when time permit and sorry if this report is > useless to anyone.On the contrary it is always useful to hear about these things, thank you. Ian.
Teck Choon Giam
2012-Apr-12 11:44 UTC
Re: Upgrade from xen-4.1-testing to xen-unstable report
On Thu, Apr 12, 2012 at 8:08 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Wed, 2012-04-11 at 22:23 +0100, Teck Choon Giam wrote: >> Hi, >> >> This is just my experience about issues I encountered when upgrade >> from xen-4.1-testing changeset 23277:80130491806f to xen-unstable >> changeset 25191:a95fc7decc83. >> >> 1. Immediately after upgrade, xl list show such error: >> >> # xl list >> libxl: error: libxl.c:506:libxl_list_domain: geting domain info list: >> Permission denied >> libxl_domain_infolist failed. >> >> After a reboot, it is fine. Any idea why such behaviour? Imagine if >> there are running domUs... this might cause issues to shutdown? I >> will downgrade and repeat such test to confirm. Might be worth a note >> in upgrading note about this if this is intended? > > The tools and the hypervisor are a matched pair so you would need to > reboot the system in order to use the new tools. This has always been > the case with Xen upgrades.I think you mean minor version upgrade is fine but not for major version upgrade which I can understand ;)> >> 2. localtime setting not working. Set to localtime=1 doesn''t seems to >> work whereby setting rtc_timeoffset works. Any idea? > > I''ve CC''d Lin Ming who implemented both of those.Thanks. I know Lin Ming implemented this.> >> 3. xl trigger hvmdomain power still never remove those related >> iptables forward rules. This is the same problem when I test in >> xen-4.1-testing. Refer to >> http://lists.xen.org/archives/html/xen-devel/2012-02/msg00771.html >> about past report. When test in xen-4.1-testing, one funny behaviour >> is if I start hvmdomain using xm then use xl trigger hvmdomain power >> does remove those iptables related rule chain. > > I think this is an issue with the hotplug scripts not being run at the > right time. There are some improvements to this in the pipeline which I > hope will help here.Hope so :) Anyway, same problem if hvmdomain shutdown itself.> >> I shall do more testing when time permit and sorry if this report is >> useless to anyone. > > On the contrary it is always useful to hear about these things, thank > you.My main frequent testing and production rely on xen-4.1-testing not xen-unstable but I will try my best to do such testing in xen-unstable once in a while until xen v4.2.0 officially released.> > Ian. >Thanks Ian for taking time to response ;) Kindest regards, Giam Teck Choon
On Thu, 2012-04-12 at 07:08 +0000, Ian Campbell wrote:> On Wed, 2012-04-11 at 22:23 +0100, Teck Choon Giam wrote: > > Hi, > > > > This is just my experience about issues I encountered when upgrade > > from xen-4.1-testing changeset 23277:80130491806f to xen-unstable > > changeset 25191:a95fc7decc83. > > > > 1. Immediately after upgrade, xl list show such error: > > > > # xl list > > libxl: error: libxl.c:506:libxl_list_domain: geting domain info list: > > Permission denied > > libxl_domain_infolist failed. > > > > After a reboot, it is fine. Any idea why such behaviour? Imagine if > > there are running domUs... this might cause issues to shutdown? I > > will downgrade and repeat such test to confirm. Might be worth a note > > in upgrading note about this if this is intended? > > The tools and the hypervisor are a matched pair so you would need to > reboot the system in order to use the new tools. This has always been > the case with Xen upgrades. > > > 2. localtime setting not working. Set to localtime=1 doesn''t seems to > > work whereby setting rtc_timeoffset works. Any idea? > > I''ve CC''d Lin Ming who implemented both of those.Just did a quick gdb debug, seems libxl__domain_build_info_setdefault was called 3 times. So below statement was executed 3 times too. b_info->rtc_timeoffset += tm->tm_gmtoff; Will look at this issue later. (gdb) bt #0 libxl__domain_build_info_setdefault (gc=0x7fffffffdc40, b_info=0x7fffffffdd90) at libxl_create.c:70 #1 0x00007ffff7991fd4 in libxl_domain_need_memory (ctx=0x626050, b_info=0x7fffffffdd90, need_memkb=0x7fffffffdc84) at libxl.c:2696 #2 0x000000000040b0ca in freemem (b_info=0x7fffffffdd90) at xl_cmdimpl.c:1399 #3 0x000000000040bd6d in create_domain (dom_info=0x7fffffffe040) at xl_cmdimpl.c:1656 #4 0x000000000041142c in main_create (argc=1, argv=0x7fffffffe638) at xl_cmdimpl.c:3446 #5 0x00000000004069d8 in main (argc=2, argv=0x7fffffffe630) at xl.c:165 (gdb) (gdb) bt #0 libxl__domain_build_info_setdefault (gc=0x7fffffffdc70, b_info=0x7fffffffdd90) at libxl_create.c:70 #1 0x00007ffff799739c in do_domain_create (gc=0x7fffffffdc70, d_config=0x7fffffffdd50, cb=0, priv=0x7fffffffdd34, domid_out=0x625dc8, restore_fd=-1) at libxl_create.c:571 #2 0x00007ffff7997cdd in libxl_domain_create_new (ctx=0x626050, d_config=0x7fffffffdd50, cb=0, priv=0x7fffffffdd34, domid=0x625dc8) at libxl_create.c:738 #3 0x000000000040be4a in create_domain (dom_info=0x7fffffffe040) at xl_cmdimpl.c:1679 #4 0x000000000041142c in main_create (argc=1, argv=0x7fffffffe638) at xl_cmdimpl.c:3446 #5 0x00000000004069d8 in main (argc=2, argv=0x7fffffffe630) at xl.c:165 (gdb) (gdb) bt #0 libxl__domain_build_info_setdefault (gc=0x7fffffffdaa0, b_info=0x7fffffffdd90) at libxl_create.c:70 #1 0x00007ffff79b2146 in libxl_run_bootloader (ctx=0x626050, info=0x7fffffffdd90, disk=0x626b20, domid=9) at libxl_bootloader.c:349 #2 0x00007ffff7997464 in do_domain_create (gc=0x7fffffffdc70, d_config=0x7fffffffdd50, cb=0, priv=0x7fffffffdd34, domid_out=0x625dc8, restore_fd=-1) at libxl_create.c:580 #3 0x00007ffff7997cdd in libxl_domain_create_new (ctx=0x626050, d_config=0x7fffffffdd50, cb=0, priv=0x7fffffffdd34, domid=0x625dc8) at libxl_create.c:738 #4 0x000000000040be4a in create_domain (dom_info=0x7fffffffe040) at xl_cmdimpl.c:1679 #5 0x000000000041142c in main_create (argc=1, argv=0x7fffffffe638) at xl_cmdimpl.c:3446 #6 0x00000000004069d8 in main (argc=2, argv=0x7fffffffe630) at xl.c:165 (gdb)
Ian Campbell
2012-Apr-12 14:03 UTC
Re: Upgrade from xen-4.1-testing to xen-unstable report
On Thu, 2012-04-12 at 13:50 +0100, Lin Ming wrote:> On Thu, 2012-04-12 at 07:08 +0000, Ian Campbell wrote: > > On Wed, 2012-04-11 at 22:23 +0100, Teck Choon Giam wrote: > > > Hi, > > > > > > This is just my experience about issues I encountered when upgrade > > > from xen-4.1-testing changeset 23277:80130491806f to xen-unstable > > > changeset 25191:a95fc7decc83. > > > > > > 1. Immediately after upgrade, xl list show such error: > > > > > > # xl list > > > libxl: error: libxl.c:506:libxl_list_domain: geting domain info list: > > > Permission denied > > > libxl_domain_infolist failed. > > > > > > After a reboot, it is fine. Any idea why such behaviour? Imagine if > > > there are running domUs... this might cause issues to shutdown? I > > > will downgrade and repeat such test to confirm. Might be worth a note > > > in upgrading note about this if this is intended? > > > > The tools and the hypervisor are a matched pair so you would need to > > reboot the system in order to use the new tools. This has always been > > the case with Xen upgrades. > > > > > 2. localtime setting not working. Set to localtime=1 doesn''t seems to > > > work whereby setting rtc_timeoffset works. Any idea? > > > > I''ve CC''d Lin Ming who implemented both of those. > > Just did a quick gdb debug, seems libxl__domain_build_info_setdefault > was called 3 times. So below statement was executed 3 times too. > > b_info->rtc_timeoffset += tm->tm_gmtoff;Oh, that sounds like the problem. The setdefault functions are supposed to be idempotent, IOW calling it two or more times should give the same result as calling it just once. I think you need to move this logic from setdefault if (libxl_defbool_val(b_info->localtime)) { time_t t; struct tm *tm; t = time(NULL); tm = localtime(&t); b_info->rtc_timeoffset += tm->tm_gmtoff; } into libxl__build_pre and do that calculation on a temporary variable whjich you pass to xc_domain_set_time_offset rather than modifying b_info. Ian.
On Thu, 2012-04-12 at 15:03 +0100, Ian Campbell wrote:> On Thu, 2012-04-12 at 13:50 +0100, Lin Ming wrote: > > On Thu, 2012-04-12 at 07:08 +0000, Ian Campbell wrote: > > > On Wed, 2012-04-11 at 22:23 +0100, Teck Choon Giam wrote: > > > > Hi, > > > > > > > > This is just my experience about issues I encountered when upgrade > > > > from xen-4.1-testing changeset 23277:80130491806f to xen-unstable > > > > changeset 25191:a95fc7decc83. > > > > > > > > 1. Immediately after upgrade, xl list show such error: > > > > > > > > # xl list > > > > libxl: error: libxl.c:506:libxl_list_domain: geting domain info list: > > > > Permission denied > > > > libxl_domain_infolist failed. > > > > > > > > After a reboot, it is fine. Any idea why such behaviour? Imagine if > > > > there are running domUs... this might cause issues to shutdown? I > > > > will downgrade and repeat such test to confirm. Might be worth a note > > > > in upgrading note about this if this is intended? > > > > > > The tools and the hypervisor are a matched pair so you would need to > > > reboot the system in order to use the new tools. This has always been > > > the case with Xen upgrades. > > > > > > > 2. localtime setting not working. Set to localtime=1 doesn''t seems to > > > > work whereby setting rtc_timeoffset works. Any idea? > > > > > > I''ve CC''d Lin Ming who implemented both of those. > > > > Just did a quick gdb debug, seems libxl__domain_build_info_setdefault > > was called 3 times. So below statement was executed 3 times too. > > > > b_info->rtc_timeoffset += tm->tm_gmtoff; > > Oh, that sounds like the problem. The setdefault functions are supposed > to be idempotent, IOW calling it two or more times should give the same > result as calling it just once. > > I think you need to move this logic from setdefault > if (libxl_defbool_val(b_info->localtime)) { > time_t t; > struct tm *tm; > > t = time(NULL); > tm = localtime(&t); > > b_info->rtc_timeoffset += tm->tm_gmtoff; > } > > into libxl__build_pre and do that calculation on a temporary variable > whjich you pass to xc_domain_set_time_offset rather than modifying > b_info.How about below patch? From e06589cb5f1efd885f9589405d0f4ecc97427ad6 Mon Sep 17 00:00:00 2001 From: Lin Ming <mlin@ss.pku.edu.cn> Date: Thu, 12 Apr 2012 22:36:24 +0800 Subject: [PATCH] libxl: fix rtc_timeoffset setting libxl__domain_build_info_setdefault may be called several times, so rtc_timeoffset can''t be set in it. Move rtc_timeoffset setting logic to libxl__build_pre. Reported-by: Teck Choon Giam <giamteckchoon@gmail.com> Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn> --- tools/libxl/libxl_create.c | 9 --------- tools/libxl/libxl_dom.c | 17 +++++++++++++++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index e63c7bd..e706124 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -127,15 +127,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, b_info->target_memkb = b_info->max_memkb; libxl_defbool_setdefault(&b_info->localtime, false); - if (libxl_defbool_val(b_info->localtime)) { - time_t t; - struct tm *tm; - - t = time(NULL); - tm = localtime(&t); - - b_info->rtc_timeoffset += tm->tm_gmtoff; - } libxl_defbool_setdefault(&b_info->disable_migrate, false); diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 0bdd654..91c4bd8 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -65,6 +65,8 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, libxl_ctx *ctx = libxl__gc_owner(gc); int tsc_mode; char *xs_domid, *con_domid; + uint32_t rtc_timeoffset; + xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus); libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, &info->cpumap); xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT); @@ -91,8 +93,19 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, if (libxl_defbool_val(info->disable_migrate)) xc_domain_disable_migrate(ctx->xch, domid); - if (info->rtc_timeoffset) - xc_domain_set_time_offset(ctx->xch, domid, info->rtc_timeoffset); + rtc_timeoffset = info->rtc_timeoffset; + if (libxl_defbool_val(info->localtime)) { + time_t t; + struct tm *tm; + + t = time(NULL); + tm = localtime(&t); + + rtc_timeoffset += tm->tm_gmtoff; + } + + if (rtc_timeoffset) + xc_domain_set_time_offset(ctx->xch, domid, rtc_timeoffset); if (info->type == LIBXL_DOMAIN_TYPE_HVM) { unsigned long shadow; -- 1.7.2.5> > Ian. > >
Ian Campbell
2012-Apr-12 14:50 UTC
Re: Upgrade from xen-4.1-testing to xen-unstable report
On Thu, 2012-04-12 at 15:42 +0100, Lin Ming wrote:> On Thu, 2012-04-12 at 15:03 +0100, Ian Campbell wrote: > > On Thu, 2012-04-12 at 13:50 +0100, Lin Ming wrote: > > > On Thu, 2012-04-12 at 07:08 +0000, Ian Campbell wrote: > > > > On Wed, 2012-04-11 at 22:23 +0100, Teck Choon Giam wrote: > > > > > Hi, > > > > > > > > > > This is just my experience about issues I encountered when upgrade > > > > > from xen-4.1-testing changeset 23277:80130491806f to xen-unstable > > > > > changeset 25191:a95fc7decc83. > > > > > > > > > > 1. Immediately after upgrade, xl list show such error: > > > > > > > > > > # xl list > > > > > libxl: error: libxl.c:506:libxl_list_domain: geting domain info list: > > > > > Permission denied > > > > > libxl_domain_infolist failed. > > > > > > > > > > After a reboot, it is fine. Any idea why such behaviour? Imagine if > > > > > there are running domUs... this might cause issues to shutdown? I > > > > > will downgrade and repeat such test to confirm. Might be worth a note > > > > > in upgrading note about this if this is intended? > > > > > > > > The tools and the hypervisor are a matched pair so you would need to > > > > reboot the system in order to use the new tools. This has always been > > > > the case with Xen upgrades. > > > > > > > > > 2. localtime setting not working. Set to localtime=1 doesn''t seems to > > > > > work whereby setting rtc_timeoffset works. Any idea? > > > > > > > > I''ve CC''d Lin Ming who implemented both of those. > > > > > > Just did a quick gdb debug, seems libxl__domain_build_info_setdefault > > > was called 3 times. So below statement was executed 3 times too. > > > > > > b_info->rtc_timeoffset += tm->tm_gmtoff; > > > > Oh, that sounds like the problem. The setdefault functions are supposed > > to be idempotent, IOW calling it two or more times should give the same > > result as calling it just once. > > > > I think you need to move this logic from setdefault > > if (libxl_defbool_val(b_info->localtime)) { > > time_t t; > > struct tm *tm; > > > > t = time(NULL); > > tm = localtime(&t); > > > > b_info->rtc_timeoffset += tm->tm_gmtoff; > > } > > > > into libxl__build_pre and do that calculation on a temporary variable > > whjich you pass to xc_domain_set_time_offset rather than modifying > > b_info. > > How about below patch? > > From e06589cb5f1efd885f9589405d0f4ecc97427ad6 Mon Sep 17 00:00:00 2001 > From: Lin Ming <mlin@ss.pku.edu.cn> > Date: Thu, 12 Apr 2012 22:36:24 +0800 > Subject: [PATCH] libxl: fix rtc_timeoffset setting > > libxl__domain_build_info_setdefault may be called several times, so > rtc_timeoffset can''t be set in it. > > Move rtc_timeoffset setting logic to libxl__build_pre. > > Reported-by: Teck Choon Giam <giamteckchoon@gmail.com> > Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>Looks good to me. Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > tools/libxl/libxl_create.c | 9 --------- > tools/libxl/libxl_dom.c | 17 +++++++++++++++-- > 2 files changed, 15 insertions(+), 11 deletions(-) > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index e63c7bd..e706124 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -127,15 +127,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, > b_info->target_memkb = b_info->max_memkb; > > libxl_defbool_setdefault(&b_info->localtime, false); > - if (libxl_defbool_val(b_info->localtime)) { > - time_t t; > - struct tm *tm; > - > - t = time(NULL); > - tm = localtime(&t); > - > - b_info->rtc_timeoffset += tm->tm_gmtoff; > - } > > libxl_defbool_setdefault(&b_info->disable_migrate, false); > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index 0bdd654..91c4bd8 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -65,6 +65,8 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > libxl_ctx *ctx = libxl__gc_owner(gc); > int tsc_mode; > char *xs_domid, *con_domid; > + uint32_t rtc_timeoffset; > + > xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus); > libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, &info->cpumap); > xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT); > @@ -91,8 +93,19 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > if (libxl_defbool_val(info->disable_migrate)) > xc_domain_disable_migrate(ctx->xch, domid); > > - if (info->rtc_timeoffset) > - xc_domain_set_time_offset(ctx->xch, domid, info->rtc_timeoffset); > + rtc_timeoffset = info->rtc_timeoffset; > + if (libxl_defbool_val(info->localtime)) { > + time_t t; > + struct tm *tm; > + > + t = time(NULL); > + tm = localtime(&t); > + > + rtc_timeoffset += tm->tm_gmtoff; > + } > + > + if (rtc_timeoffset) > + xc_domain_set_time_offset(ctx->xch, domid, rtc_timeoffset); > > if (info->type == LIBXL_DOMAIN_TYPE_HVM) { > unsigned long shadow;
Teck Choon Giam
2012-Apr-12 15:25 UTC
Re: Upgrade from xen-4.1-testing to xen-unstable report
On Thu, Apr 12, 2012 at 10:50 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Thu, 2012-04-12 at 15:42 +0100, Lin Ming wrote: >> On Thu, 2012-04-12 at 15:03 +0100, Ian Campbell wrote: >> > On Thu, 2012-04-12 at 13:50 +0100, Lin Ming wrote: >> > > On Thu, 2012-04-12 at 07:08 +0000, Ian Campbell wrote: >> > > > On Wed, 2012-04-11 at 22:23 +0100, Teck Choon Giam wrote: >> > > > > Hi, >> > > > > >> > > > > This is just my experience about issues I encountered when upgrade >> > > > > from xen-4.1-testing changeset 23277:80130491806f to xen-unstable >> > > > > changeset 25191:a95fc7decc83. >> > > > > >> > > > > 1. Immediately after upgrade, xl list show such error: >> > > > > >> > > > > # xl list >> > > > > libxl: error: libxl.c:506:libxl_list_domain: geting domain info list: >> > > > > Permission denied >> > > > > libxl_domain_infolist failed. >> > > > > >> > > > > After a reboot, it is fine. Any idea why such behaviour? Imagine if >> > > > > there are running domUs... this might cause issues to shutdown? I >> > > > > will downgrade and repeat such test to confirm. Might be worth a note >> > > > > in upgrading note about this if this is intended? >> > > > >> > > > The tools and the hypervisor are a matched pair so you would need to >> > > > reboot the system in order to use the new tools. This has always been >> > > > the case with Xen upgrades. >> > > > >> > > > > 2. localtime setting not working. Set to localtime=1 doesn''t seems to >> > > > > work whereby setting rtc_timeoffset works. Any idea? >> > > > >> > > > I''ve CC''d Lin Ming who implemented both of those. >> > > >> > > Just did a quick gdb debug, seems libxl__domain_build_info_setdefault >> > > was called 3 times. So below statement was executed 3 times too. >> > > >> > > b_info->rtc_timeoffset += tm->tm_gmtoff; >> > >> > Oh, that sounds like the problem. The setdefault functions are supposed >> > to be idempotent, IOW calling it two or more times should give the same >> > result as calling it just once. >> > >> > I think you need to move this logic from setdefault >> > if (libxl_defbool_val(b_info->localtime)) { >> > time_t t; >> > struct tm *tm; >> > >> > t = time(NULL); >> > tm = localtime(&t); >> > >> > b_info->rtc_timeoffset += tm->tm_gmtoff; >> > } >> > >> > into libxl__build_pre and do that calculation on a temporary variable >> > whjich you pass to xc_domain_set_time_offset rather than modifying >> > b_info. >> >> How about below patch?I am currently compiling xen-unstable changeset 25193:c6bde42c8845 with this patch. Will test then report back. Thanks. Kindest regards, Giam Teck Choon>> >> From e06589cb5f1efd885f9589405d0f4ecc97427ad6 Mon Sep 17 00:00:00 2001 >> From: Lin Ming <mlin@ss.pku.edu.cn> >> Date: Thu, 12 Apr 2012 22:36:24 +0800 >> Subject: [PATCH] libxl: fix rtc_timeoffset setting >> >> libxl__domain_build_info_setdefault may be called several times, so >> rtc_timeoffset can''t be set in it. >> >> Move rtc_timeoffset setting logic to libxl__build_pre. >> >> Reported-by: Teck Choon Giam <giamteckchoon@gmail.com> >> Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn> > > Looks good to me. > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > >> --- >> tools/libxl/libxl_create.c | 9 --------- >> tools/libxl/libxl_dom.c | 17 +++++++++++++++-- >> 2 files changed, 15 insertions(+), 11 deletions(-) >> >> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c >> index e63c7bd..e706124 100644 >> --- a/tools/libxl/libxl_create.c >> +++ b/tools/libxl/libxl_create.c >> @@ -127,15 +127,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, >> b_info->target_memkb = b_info->max_memkb; >> >> libxl_defbool_setdefault(&b_info->localtime, false); >> - if (libxl_defbool_val(b_info->localtime)) { >> - time_t t; >> - struct tm *tm; >> - >> - t = time(NULL); >> - tm = localtime(&t); >> - >> - b_info->rtc_timeoffset += tm->tm_gmtoff; >> - } >> >> libxl_defbool_setdefault(&b_info->disable_migrate, false); >> >> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c >> index 0bdd654..91c4bd8 100644 >> --- a/tools/libxl/libxl_dom.c >> +++ b/tools/libxl/libxl_dom.c >> @@ -65,6 +65,8 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, >> libxl_ctx *ctx = libxl__gc_owner(gc); >> int tsc_mode; >> char *xs_domid, *con_domid; >> + uint32_t rtc_timeoffset; >> + >> xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus); >> libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, &info->cpumap); >> xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT); >> @@ -91,8 +93,19 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, >> if (libxl_defbool_val(info->disable_migrate)) >> xc_domain_disable_migrate(ctx->xch, domid); >> >> - if (info->rtc_timeoffset) >> - xc_domain_set_time_offset(ctx->xch, domid, info->rtc_timeoffset); >> + rtc_timeoffset = info->rtc_timeoffset; >> + if (libxl_defbool_val(info->localtime)) { >> + time_t t; >> + struct tm *tm; >> + >> + t = time(NULL); >> + tm = localtime(&t); >> + >> + rtc_timeoffset += tm->tm_gmtoff; >> + } >> + >> + if (rtc_timeoffset) >> + xc_domain_set_time_offset(ctx->xch, domid, rtc_timeoffset); >> >> if (info->type == LIBXL_DOMAIN_TYPE_HVM) { >> unsigned long shadow; > >
Teck Choon Giam
2012-Apr-12 16:27 UTC
Re: Upgrade from xen-4.1-testing to xen-unstable report
On Thu, Apr 12, 2012 at 11:25 PM, Teck Choon Giam <giamteckchoon@gmail.com> wrote:> On Thu, Apr 12, 2012 at 10:50 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> On Thu, 2012-04-12 at 15:42 +0100, Lin Ming wrote: >>> On Thu, 2012-04-12 at 15:03 +0100, Ian Campbell wrote: >>> > On Thu, 2012-04-12 at 13:50 +0100, Lin Ming wrote: >>> > > On Thu, 2012-04-12 at 07:08 +0000, Ian Campbell wrote: >>> > > > On Wed, 2012-04-11 at 22:23 +0100, Teck Choon Giam wrote: >>> > > > > Hi, >>> > > > > >>> > > > > This is just my experience about issues I encountered when upgrade >>> > > > > from xen-4.1-testing changeset 23277:80130491806f to xen-unstable >>> > > > > changeset 25191:a95fc7decc83. >>> > > > > >>> > > > > 1. Immediately after upgrade, xl list show such error: >>> > > > > >>> > > > > # xl list >>> > > > > libxl: error: libxl.c:506:libxl_list_domain: geting domain info list: >>> > > > > Permission denied >>> > > > > libxl_domain_infolist failed. >>> > > > > >>> > > > > After a reboot, it is fine. Any idea why such behaviour? Imagine if >>> > > > > there are running domUs... this might cause issues to shutdown? I >>> > > > > will downgrade and repeat such test to confirm. Might be worth a note >>> > > > > in upgrading note about this if this is intended? >>> > > > >>> > > > The tools and the hypervisor are a matched pair so you would need to >>> > > > reboot the system in order to use the new tools. This has always been >>> > > > the case with Xen upgrades. >>> > > > >>> > > > > 2. localtime setting not working. Set to localtime=1 doesn''t seems to >>> > > > > work whereby setting rtc_timeoffset works. Any idea? >>> > > > >>> > > > I''ve CC''d Lin Ming who implemented both of those. >>> > > >>> > > Just did a quick gdb debug, seems libxl__domain_build_info_setdefault >>> > > was called 3 times. So below statement was executed 3 times too. >>> > > >>> > > b_info->rtc_timeoffset += tm->tm_gmtoff; >>> > >>> > Oh, that sounds like the problem. The setdefault functions are supposed >>> > to be idempotent, IOW calling it two or more times should give the same >>> > result as calling it just once. >>> > >>> > I think you need to move this logic from setdefault >>> > if (libxl_defbool_val(b_info->localtime)) { >>> > time_t t; >>> > struct tm *tm; >>> > >>> > t = time(NULL); >>> > tm = localtime(&t); >>> > >>> > b_info->rtc_timeoffset += tm->tm_gmtoff; >>> > } >>> > >>> > into libxl__build_pre and do that calculation on a temporary variable >>> > whjich you pass to xc_domain_set_time_offset rather than modifying >>> > b_info. >>> >>> How about below patch? > > I am currently compiling xen-unstable changeset 25193:c6bde42c8845 > with this patch. Will test then report back.Tested and it works! Thanks. Kindest regards, Giam Teck Choon