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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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

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.


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