[libvirt] [PATCH] libxl: add support for specifying clock offset and adjustment

Jim Fehlig jfehlig at suse.com
Wed Feb 21 19:48:29 UTC 2018


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




More information about the libvir-list mailing list