Re: [Xen-devel] [PATCH] libxl: support for "rtc_timeoffset" and "localtime"

[Teck Choon Giam <giamteckchoon@xxxxxxxxx>]

Hi Lin Ming,

Thanks for this patch.  I am actually working to backport this patch
to xen-4.1-testing for self-testing.  I have a question as below about
one of your code.

On Sun, Mar 18, 2012 at 7:50 PM, Lin Ming <mlin@xxxxxxxxxxxxx> wrote:
> Implement "rtc_timeoffset" and "localtime" options compatible as xm.
>
> rtc_timeoffset is the offset between host time and guest time.
> localtime means to specify whether the emulted RTC appears as UTC or is
> offset by the host.
>
> Signed-off-by: Lin Ming <mlin@xxxxxxxxxxxxx>
> ---
>  tools/libxl/libxl_dom.c     |    3 +++
>  tools/libxl/libxl_types.idl |    1 +
>  tools/libxl/xl_cmdimpl.c    |   13 +++++++++++++
>  tools/libxl/xl_sxp.c        |    1 +
>  4 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 9b33267..0bdd654 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -91,6 +91,9 @@ 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);
> +
>     if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
>         unsigned long shadow;
>         shadow = (info->shadow_memkb + 1023) / 1024;
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 413a1a6..56f760c 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -238,6 +238,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>     ("target_memkb",    MemKB),
>     ("video_memkb",     MemKB),
>     ("shadow_memkb",    MemKB),
> +    ("rtc_timeoffset",  uint32),
>     ("disable_migrate", libxl_defbool),
>     ("cpuid",           libxl_cpuid_policy_list),
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 1d59b89..b080a2b 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -697,6 +697,19 @@ static void parse_config_data(const char 
> *configfile_filename_report,
>         }
>     }
>
> +    if (!xlu_cfg_get_long(config, "rtc_timeoffset", &l, 0))
> +        b_info->rtc_timeoffset = l;
> +
> +    if (!xlu_cfg_get_long(config, "localtime", &l, 0) && l) {
> +        time_t t;
> +        struct tm *tm;
> +
> +        t = time(NULL);
> +        tm = localtime(&t);
> +
> +        b_info->rtc_timeoffset -= tm->tm_gmtoff;

If tm->tm_gmtoff is 28800 for example (GMT +8) tested with
Asia/Singapore and Asia/Hong_Kong, then what is the
b_info->rct_timeoffset?
Is it b_info->rtc_timeoffset = b_info->rtc_timeoffset - tm->tm_gmtoff?
 If this is so, then I guess it is not right.  Shouldn't this be as
below instead?

b_info->rtc_timeoffset += tm->tm_gmtoff;

In python, time.altzone and time.timezone will return the offset with
negative for Asia or non-US/most of Western Europe depending it is
time.altzone or time.timezone according to
http://docs.python.org/library/time.html... someone correct me if I am
wrong :p

Tested with python in Scientific Linux 6:

Python 2.6.6 (r266:84292, Jan  4 2012, 16:09:22)
[GCC 4.4.6 20110731 (Red Hat 4.4.6-3)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> print time.altzone
-28800
>>> print time.timezone
-28800
>>> print time.tzname
('SGT', 'SGT')


So unless tm->tm_gmtoff return the same value including negative as
python time function, then I am totally wrong and sorry about it.

Thanks.

Kindest regards,
Giam Teck Choon


> +    }
> +
>     if (!xlu_cfg_get_long (config, "videoram", &l, 0))
>         b_info->video_memkb = l * 1024;
>
> diff --git a/tools/libxl/xl_sxp.c b/tools/libxl/xl_sxp.c
> index c68b6df..bb1f3da 100644
> --- a/tools/libxl/xl_sxp.c
> +++ b/tools/libxl/xl_sxp.c
> @@ -92,6 +92,7 @@ void printf_info_sexp(int domid, libxl_domain_config 
> *d_config)
>         printf("\t\t\t(firmware %s)\n", b_info->u.hvm.firmware);
>         printf("\t\t\t(video_memkb %"PRId64")\n", b_info->video_memkb);
>         printf("\t\t\t(shadow_memkb %"PRId64")\n", b_info->shadow_memkb);
> +        printf("\t\t\t(rtc_timeoffset %"PRId32")\n", b_info->rtc_timeoffset);
>         printf("\t\t\t(pae %s)\n", 
> libxl_defbool_to_string(b_info->u.hvm.pae));
>         printf("\t\t\t(apic %s)\n",
>                libxl_defbool_to_string(b_info->u.hvm.apic));
> --
> 1.7.2.5
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel