* Upgrade from xen-4.1-testing to xen-unstable report
@ 2012-04-11 21:23 Teck Choon Giam
2012-04-12 7:08 ` Ian Campbell
0 siblings, 1 reply; 9+ messages in thread
From: Teck Choon Giam @ 2012-04-11 21:23 UTC (permalink / raw)
To: xen-devel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Upgrade from xen-4.1-testing to xen-unstable report
2012-04-11 21:23 Upgrade from xen-4.1-testing to xen-unstable report Teck Choon Giam
@ 2012-04-12 7:08 ` Ian Campbell
2012-04-12 11:44 ` Teck Choon Giam
2012-04-12 12:50 ` Lin Ming
0 siblings, 2 replies; 9+ messages in thread
From: Ian Campbell @ 2012-04-12 7:08 UTC (permalink / raw)
To: Teck Choon Giam; +Cc: xen-devel, Lin Ming
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Upgrade from xen-4.1-testing to xen-unstable report
2012-04-12 7:08 ` Ian Campbell
@ 2012-04-12 11:44 ` Teck Choon Giam
2012-04-12 12:50 ` Lin Ming
1 sibling, 0 replies; 9+ messages in thread
From: Teck Choon Giam @ 2012-04-12 11:44 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Lin Ming
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Upgrade from xen-4.1-testing to xen-unstable report
2012-04-12 7:08 ` Ian Campbell
2012-04-12 11:44 ` Teck Choon Giam
@ 2012-04-12 12:50 ` Lin Ming
2012-04-12 14:03 ` Ian Campbell
1 sibling, 1 reply; 9+ messages in thread
From: Lin Ming @ 2012-04-12 12:50 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Teck Choon Giam
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)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Upgrade from xen-4.1-testing to xen-unstable report
2012-04-12 12:50 ` Lin Ming
@ 2012-04-12 14:03 ` Ian Campbell
2012-04-12 14:42 ` Lin Ming
0 siblings, 1 reply; 9+ messages in thread
From: Ian Campbell @ 2012-04-12 14:03 UTC (permalink / raw)
To: Lin Ming; +Cc: xen-devel, Teck Choon Giam
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Upgrade from xen-4.1-testing to xen-unstable report
2012-04-12 14:03 ` Ian Campbell
@ 2012-04-12 14:42 ` Lin Ming
2012-04-12 14:50 ` Ian Campbell
0 siblings, 1 reply; 9+ messages in thread
From: Lin Ming @ 2012-04-12 14:42 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Teck Choon Giam
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.
>
>
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Upgrade from xen-4.1-testing to xen-unstable report
2012-04-12 14:42 ` Lin Ming
@ 2012-04-12 14:50 ` Ian Campbell
2012-04-12 15:25 ` Teck Choon Giam
0 siblings, 1 reply; 9+ messages in thread
From: Ian Campbell @ 2012-04-12 14:50 UTC (permalink / raw)
To: Lin Ming, Ian Jackson; +Cc: xen-devel, Teck Choon Giam
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;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Upgrade from xen-4.1-testing to xen-unstable report
2012-04-12 14:50 ` Ian Campbell
@ 2012-04-12 15:25 ` Teck Choon Giam
2012-04-12 16:27 ` Teck Choon Giam
0 siblings, 1 reply; 9+ messages in thread
From: Teck Choon Giam @ 2012-04-12 15:25 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Ian Jackson, Lin Ming
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;
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Upgrade from xen-4.1-testing to xen-unstable report
2012-04-12 15:25 ` Teck Choon Giam
@ 2012-04-12 16:27 ` Teck Choon Giam
0 siblings, 0 replies; 9+ messages in thread
From: Teck Choon Giam @ 2012-04-12 16:27 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Ian Jackson, Lin Ming
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-04-12 16:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-11 21:23 Upgrade from xen-4.1-testing to xen-unstable report Teck Choon Giam
2012-04-12 7:08 ` Ian Campbell
2012-04-12 11:44 ` Teck Choon Giam
2012-04-12 12:50 ` Lin Ming
2012-04-12 14:03 ` Ian Campbell
2012-04-12 14:42 ` Lin Ming
2012-04-12 14:50 ` Ian Campbell
2012-04-12 15:25 ` Teck Choon Giam
2012-04-12 16:27 ` Teck Choon Giam
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.