All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libxl: support for "rtc_timeoffset" and "localtime"
@ 2012-03-18 11:50 Lin Ming
  2012-03-18 21:57 ` Teck Choon Giam
  2012-03-19  8:58 ` Ian Campbell
  0 siblings, 2 replies; 9+ messages in thread
From: Lin Ming @ 2012-03-18 11:50 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian.Campbell

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@ss.pku.edu.cn>
---
 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 (!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

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH] libxl: support for "rtc_timeoffset" and "localtime"
  2012-03-18 11:50 [PATCH] libxl: support for "rtc_timeoffset" and "localtime" Lin Ming
@ 2012-03-18 21:57 ` Teck Choon Giam
  2012-03-19  1:22   ` Lin Ming
  2012-03-19  8:58 ` Ian Campbell
  1 sibling, 1 reply; 9+ messages in thread
From: Teck Choon Giam @ 2012-03-18 21:57 UTC (permalink / raw)
  To: Lin Ming; +Cc: Ian.Campbell, xen-devel

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@ss.pku.edu.cn> 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@ss.pku.edu.cn>
> ---
>  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@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] libxl: support for "rtc_timeoffset" and "localtime"
  2012-03-18 21:57 ` Teck Choon Giam
@ 2012-03-19  1:22   ` Lin Ming
  2012-03-19  2:09     ` Teck Choon Giam
  0 siblings, 1 reply; 9+ messages in thread
From: Lin Ming @ 2012-03-19  1:22 UTC (permalink / raw)
  To: Teck Choon Giam; +Cc: Ian.Campbell, xen-devel

On Mon, Mar 19, 2012 at 5:57 AM, Teck Choon Giam
<giamteckchoon@gmail.com> 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@ss.pku.edu.cn> 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@ss.pku.edu.cn>
>> ---
>>  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 just realized another problem: does ->tm_gmtoff  already check
DST(day light saving) zone?

Will check.

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@lists.xen.org
>> http://lists.xen.org/xen-devel
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] libxl: support for "rtc_timeoffset" and "localtime"
  2012-03-19  1:22   ` Lin Ming
@ 2012-03-19  2:09     ` Teck Choon Giam
  2012-03-19  2:24       ` Lin Ming
  0 siblings, 1 reply; 9+ messages in thread
From: Teck Choon Giam @ 2012-03-19  2:09 UTC (permalink / raw)
  To: Lin Ming; +Cc: Ian.Campbell, xen-devel

On Mon, Mar 19, 2012 at 9:22 AM, Lin Ming <mlin@ss.pku.edu.cn> wrote:
> On Mon, Mar 19, 2012 at 5:57 AM, Teck Choon Giam
> <giamteckchoon@gmail.com> 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@ss.pku.edu.cn> 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@ss.pku.edu.cn>
>>> ---
>>>  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         |
-----------------------------------------------------------------------------------

>
> 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?

>
> 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@lists.xen.org
>>> http://lists.xen.org/xen-devel
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] libxl: support for "rtc_timeoffset" and "localtime"
  2012-03-19  2:09     ` Teck Choon Giam
@ 2012-03-19  2:24       ` Lin Ming
  2012-03-19  2:50         ` Teck Choon Giam
  0 siblings, 1 reply; 9+ messages in thread
From: Lin Ming @ 2012-03-19  2:24 UTC (permalink / raw)
  To: Teck Choon Giam; +Cc: Ian.Campbell, xen-devel

On Mon, Mar 19, 2012 at 10:09 AM, Teck Choon Giam
<giamteckchoon@gmail.com> wrote:
> On Mon, Mar 19, 2012 at 9:22 AM, Lin Ming <mlin@ss.pku.edu.cn> wrote:
>> On Mon, Mar 19, 2012 at 5:57 AM, Teck Choon Giam
>> <giamteckchoon@gmail.com> 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@ss.pku.edu.cn> 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@ss.pku.edu.cn>
>>>> ---
>>>>  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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] libxl: support for "rtc_timeoffset" and "localtime"
  2012-03-19  2:24       ` Lin Ming
@ 2012-03-19  2:50         ` Teck Choon Giam
  2012-03-19  3:10           ` Lin Ming
  0 siblings, 1 reply; 9+ messages in thread
From: Teck Choon Giam @ 2012-03-19  2:50 UTC (permalink / raw)
  To: Lin Ming; +Cc: Ian.Campbell, xen-devel

On Mon, Mar 19, 2012 at 10:24 AM, Lin Ming <mlin@ss.pku.edu.cn> wrote:
> On Mon, Mar 19, 2012 at 10:09 AM, Teck Choon Giam
> <giamteckchoon@gmail.com> wrote:
>> On Mon, Mar 19, 2012 at 9:22 AM, Lin Ming <mlin@ss.pku.edu.cn> wrote:
>>> On Mon, Mar 19, 2012 at 5:57 AM, Teck Choon Giam
>>> <giamteckchoon@gmail.com> 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@ss.pku.edu.cn> 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@ss.pku.edu.cn>
>>>>> ---
>>>>>  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?

Yes, same here means the results of xm and xl are the same.

>
>>
>>>
>>> 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.

Hopefully it will be like the link I posted... it gives you the
current offset but that is FreeBSD... ...

I tested in Linux and one result as below:

[x58.localdomain:/home/choon]# cp -pf /usr/share/zoneinfo/US/Eastern
/etc/localtime
cp: overwrite `/etc/localtime'? y
[x58.localdomain:/home/choon]# date
Sun Mar 18 22:45:09 EDT 2012
[x58.localdomain:/home/choon]# ./checktimeoffset
tm_isdst = 1
tm_gmtoff = -14400

So it is UTC-4 right?  If so, from http://en.wikipedia.org/wiki/UTC-4
the offset does include DST?

Thanks.

Kindest regards,
Giam Teck Choon



>
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] libxl: support for "rtc_timeoffset" and "localtime"
  2012-03-19  2:50         ` Teck Choon Giam
@ 2012-03-19  3:10           ` Lin Ming
  0 siblings, 0 replies; 9+ messages in thread
From: Lin Ming @ 2012-03-19  3:10 UTC (permalink / raw)
  To: Teck Choon Giam; +Cc: Ian.Campbell, xen-devel

On Mon, Mar 19, 2012 at 10:50 AM, Teck Choon Giam
<giamteckchoon@gmail.com> wrote:
[...]
>>
>> Great!
>>
>> "same" means xl localtime/rtc_timeoffset option works same as xm, right?
>
> Yes, same here means the results of xm and xl are the same.

Nice!

>
>>
>>>
>>>>
>>>> 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.
>
> Hopefully it will be like the link I posted... it gives you the
> current offset but that is FreeBSD... ...
>
> I tested in Linux and one result as below:
>
> [x58.localdomain:/home/choon]# cp -pf /usr/share/zoneinfo/US/Eastern
> /etc/localtime
> cp: overwrite `/etc/localtime'? y
> [x58.localdomain:/home/choon]# date
> Sun Mar 18 22:45:09 EDT 2012
> [x58.localdomain:/home/choon]# ./checktimeoffset
> tm_isdst = 1
> tm_gmtoff = -14400
>
> So it is UTC-4 right?  If so, from http://en.wikipedia.org/wiki/UTC-4
> the offset does include DST?

I think so.

I'll post a v2 patch with your change added.

Thanks.

>
> Thanks.
>
> Kindest regards,
> Giam Teck Choon
>

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] libxl: support for "rtc_timeoffset" and "localtime"
  2012-03-18 11:50 [PATCH] libxl: support for "rtc_timeoffset" and "localtime" Lin Ming
  2012-03-18 21:57 ` Teck Choon Giam
@ 2012-03-19  8:58 ` Ian Campbell
  2012-03-19 12:09   ` Lin Ming
  1 sibling, 1 reply; 9+ messages in thread
From: Ian Campbell @ 2012-03-19  8:58 UTC (permalink / raw)
  To: Lin Ming; +Cc: xen-devel

On Sun, 2012-03-18 at 11:50 +0000, Lin Ming 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@ss.pku.edu.cn

Thanks, this look good!

A few comments:

> ---
>  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(-)

Please also patch docs/man/xl.cfg.pod.5 to document the newly supported
options

> [...]

> 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) {

I wonder if we ought to include a "bool localtime" in the libxl API and
move this stuff there. If we think this logic would be generally useful
to other toolstack then we should do this.

> +        time_t t;
> +        struct tm *tm;
> +
> +        t = time(NULL);
> +        tm = localtime(&t);
> +
> +        b_info->rtc_timeoffset -= tm->tm_gmtoff;
> +    }
> +
>      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));

Is this change compatible with xm's output?

In general there is no need to update this file unless it has been shown
to cause a compatibility issue with xm.

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] libxl: support for "rtc_timeoffset" and "localtime"
  2012-03-19  8:58 ` Ian Campbell
@ 2012-03-19 12:09   ` Lin Ming
  0 siblings, 0 replies; 9+ messages in thread
From: Lin Ming @ 2012-03-19 12:09 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

On Mon, Mar 19, 2012 at 4:58 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Sun, 2012-03-18 at 11:50 +0000, Lin Ming 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@ss.pku.edu.cn
>
> Thanks, this look good!
>
> A few comments:
>
>> ---
>>  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(-)
>
> Please also patch docs/man/xl.cfg.pod.5 to document the newly supported
> options

OK.

>
>> [...]
>
>> 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) {
>
> I wonder if we ought to include a "bool localtime" in the libxl API and
> move this stuff there. If we think this logic would be generally useful
> to other toolstack then we should do this.

I think it's useful.
Will add it.

>
>> +        time_t t;
>> +        struct tm *tm;
>> +
>> +        t = time(NULL);
>> +        tm = localtime(&t);
>> +
>> +        b_info->rtc_timeoffset -= tm->tm_gmtoff;
>> +    }
>> +
>>      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));
>
> Is this change compatible with xm's output?

No, will remove it.

>
> In general there is no need to update this file unless it has been shown
> to cause a compatibility issue with xm.

Thanks for the comments.
Lin Ming

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-03-19 12:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-18 11:50 [PATCH] libxl: support for "rtc_timeoffset" and "localtime" Lin Ming
2012-03-18 21:57 ` Teck Choon Giam
2012-03-19  1:22   ` Lin Ming
2012-03-19  2:09     ` Teck Choon Giam
2012-03-19  2:24       ` Lin Ming
2012-03-19  2:50         ` Teck Choon Giam
2012-03-19  3:10           ` Lin Ming
2012-03-19  8:58 ` Ian Campbell
2012-03-19 12:09   ` Lin Ming

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.