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

[Lin Ming <mlin@xxxxxxxxxxxxx>]

On Mon, Mar 19, 2012 at 10:09 AM, Teck Choon Giam
<giamteckchoon@xxxxxxxxx> wrote:
> On Mon, Mar 19, 2012 at 9:22 AM, Lin Ming <mlin@xxxxxxxxxxxxx> wrote:
>> On Mon, Mar 19, 2012 at 5:57 AM, Teck Choon Giam
>> <giamteckchoon@xxxxxxxxx> wrote:
>>> Hi Lin Ming,
>>
>> Hi Teck,
>>
>>>
>>> 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;
>>
>> You are right.
>>
>> Python time.altzone/timezone is the offset in seconds "west" of UTC,
>> while tm->tm_gmtoff is "east" of UTC.
>
> I actually tested with the following summary for my backport patch
> based on yours except the "b_info->rtc_timeoffset += tm->tm_gmtoff;"
> to xen-4.1-testing and things look the same.
>
> -----------------------------------------------------------------------------------
> |         Results        |           HVM domU config                |
> -----------------------------------------------------------------------------------
> |     xm     |     xl     |     localtime     |     rtc_timeoffset     |
> -----------------------------------------------------------------------------------
> |   same   |   same  |      Not Set      |         Not Set         |
> -----------------------------------------------------------------------------------
> |   same   |   same  |      Not Set      |              0             |
> -----------------------------------------------------------------------------------
> |   same   |   same  |      Not Set      |          28800          |
> -----------------------------------------------------------------------------------
> |   same   |   same  |           0          |         Not Set         |
> -----------------------------------------------------------------------------------
> |   same   |   same  |           1          |         Not Set         |
> -----------------------------------------------------------------------------------

Great!

"same" means xl localtime/rtc_timeoffset option works same as xm, right?

>
>>
>> I just realized another problem: does ->tm_gmtoff  already check
>> DST(day light saving) zone?
>
> This will need to ask those C experts... ... my testing didn't include
> changing DST/tm_isdst = 1 though.
> I tried to google about it and found
> http://unix.derkeiler.com/Newsgroups/comp.unix.programmer/2004-07/0044.html
> might be useful?

Yes.

I'll set my timezone to some US timezone which actually uses day light saving,
then check tm_gmtoff value.

Thanks,
Lin Ming

>
>>
>> Will check.
>
> Thanks for your patch and prompt reply.
>
> Kindest regards,
> Giam Teck Choon
>
>
>>
>> Thanks,
>> Lin Ming
>>
>>>
>>> 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