On 02/21/2018 02:20 AM, Daniel P. Berrangé wrote:
On Tue, Feb 20, 2018 at 05:28:57PM -0700, Jim Fehlig wrote:
libxl supports setting the domain real time clock to local time or UTC via the localtime field of libxl_domain_build_info. Adjustment of the clock is also supported via the rtc_timeoffset field. The libvirt libxl driver has never supported these settings, instead relying on libxl's default of a UTC real time clock with adjustment set to 0.
There is at least one user that would like the ability to change the defaults
https://www.redhat.com/archives/libvirt-users/2018-February/msg00059.html
Add support for specifying a local time clock and for specifying an adjustment for both local time and UTC clocks. Add a test case to verify the XML to libxl_domain_config conversion.
Local time clock and clock adjustment is already supported by the XML <-> xl.cfg converter. What is missing is an explicit test for the conversion. There are plenty of existing tests that all use UTC with 0 adjustment. Hijack test-fullvirt-tsc-timer to test a local time clock with 1 hour adjustment.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 35 +++++++-- .../libxlxml2domconfigdata/variable-clock-hvm.json | 91 ++++++++++++++++++++++ .../libxlxml2domconfigdata/variable-clock-hvm.xml | 36 +++++++++ tests/libxlxml2domconfigtest.c | 1 + tests/xlconfigdata/test-fullvirt-tsc-timer.cfg | 4 +- tests/xlconfigdata/test-fullvirt-tsc-timer.xml | 2 +- 6 files changed, 160 insertions(+), 9 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 0c5de344d..39ae709c7 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -274,6 +274,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, virCapsPtr caps, libxl_domain_config *d_config) { + virDomainClockDef clock = def->clock; libxl_domain_build_info *b_info = &d_config->b_info; int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; size_t i; @@ -293,10 +294,32 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, for (i = 0; i < virDomainDefGetVcpus(def); i++) libxl_bitmap_set((&b_info->avail_vcpus), i);
- for (i = 0; i < def->clock.ntimers; i++) { - switch ((virDomainTimerNameType) def->clock.timers[i]->name) { + switch (clock.offset) {
Can you cast that to virDomainClockOffset to get enum checking
+ case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: + if (clock.data.variable.basis == VIR_DOMAIN_CLOCK_BASIS_LOCALTIME) + libxl_defbool_set(&b_info->localtime, true); + b_info->rtc_timeoffset = clock.data.variable.adjustment; + break; + + case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: + libxl_defbool_set(&b_info->localtime, true); + break; + + /* Nothing to do since UTC is the default in libxl */ + case VIR_DOMAIN_CLOCK_OFFSET_UTC: + break; +
Put case VIR_DOMAIN_CLOCK_OFFSET_LAST: right here
+ default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported clock offset '%s'"), + virDomainClockOffsetTypeToString(clock.offset));
You shouldn't use the ToString macros in a default: or _LAST: case because it will return empty string for invalid values. Just print out the decimal value, and use the word "Unexpected" rather than "unsupported"
+ return -1; + }
Assuming those simple tweaks are done, then
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Thanks. I also had to account for the unsupported case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE. Along with your suggestions, I squashed in the below diff and pushed. Regards, Jim diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 95cdee4fa..01d9b82da 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -309,6 +309,12 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, case VIR_DOMAIN_CLOCK_OFFSET_UTC: break; + case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported clock offset '%s'"), + virDomainClockOffsetTypeToString(clock.offset)); + return -1; + case VIR_DOMAIN_CLOCK_OFFSET_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED,