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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-03-19 at 13:02 +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@xxxxxxxxxxxxx>
> ---
>  docs/man/xl.cfg.pod.5       |    8 ++++++++
>  tools/libxl/libxl_create.c  |    1 +
>  tools/libxl/libxl_dom.c     |    3 +++
>  tools/libxl/libxl_types.idl |    2 ++
>  tools/libxl/xl_cmdimpl.c    |   14 ++++++++++++++
>  5 files changed, 28 insertions(+), 0 deletions(-)
> 
> Changed since v1:
>   * Add tm_gmtoff, instead subtract, to rtc_timeoffset (Giam Teck Choon)
>   * Add docs for the newly supported options
>   * Include "bool localtime" in libxl API
>   * Remove the unneeded "rtc_timeoffset" output in printf_info_sexp()
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 55f7e75..f93f1d6 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -109,6 +109,14 @@ created online and the remainder will be offline.
>  
>  Start the guest with MBYTES megabytes of RAM.
>  
> +=item B<localtime=BOOLEAN>
> +
> +Set the real time clock to local time or to UTC. UTC by default.

Might be worth making it clearer which corresponds to true and which to
false?

Also both of these options belong in the "Fully-virtualised (HVM) Guest
Specific Options" section of the document.

> +
> +=item B<rtc_timeoffset=SECONDS>
> +
> +Set the real time clock offset in seconds. 0 by default.
> +
>  =item B<on_poweroff="ACTION">
>  
>  Specifies what should be done with the domain if it shuts itself down.
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 8417661..b0e76cb 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -124,6 +124,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>      if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
>          b_info->target_memkb = b_info->max_memkb;
>  
> +    libxl_defbool_setdefault(&b_info->localtime, false);
>      libxl_defbool_setdefault(&b_info->disable_migrate, false);
>  
>      switch (b_info->type) {
> 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..09089b2 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -238,6 +238,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
>      ("target_memkb",    MemKB),
>      ("video_memkb",     MemKB),
>      ("shadow_memkb",    MemKB),
> +    ("rtc_timeoffset",  uint32),
> +    ("localtime",       libxl_defbool),
>      ("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..bcbea48 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -697,6 +697,20 @@ 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;
> +
> +    xlu_cfg_get_defbool(config, "localtime", &b_info->localtime, 0);
> +    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;
> +    }

This "if(...) { ... }" and the logic therein should be done inside
libxl, I think in libxl__domain_build_info_setdefault() just after you
set the default value

BT doesn't the above crash if localtime is not given at all in the
config due to the lack of a specific value (which libxl_defbool_val
requires has been set previously)

Ian.


> +
>      if (!xlu_cfg_get_long (config, "videoram", &l, 0))
>          b_info->video_memkb = l * 1024;
>  



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