[libvirt] [PATCH] libxl: add support for specifying clock offset and adjustment
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) { + 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; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported clock offset '%s'"), + virDomainClockOffsetTypeToString(clock.offset)); + return -1; + } + + for (i = 0; i < clock.ntimers; i++) { + switch ((virDomainTimerNameType) clock.timers[i]->name) { case VIR_DOMAIN_TIMER_NAME_TSC: - switch (def->clock.timers[i]->mode) { + switch (clock.timers[i]->mode) { case VIR_DOMAIN_TIMER_MODE_NATIVE: b_info->tsc_mode = LIBXL_TSC_MODE_NATIVE; break; @@ -315,10 +338,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, if (!hvm) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported timer type (name) '%s'"), - virDomainTimerNameTypeToString(def->clock.timers[i]->name)); + virDomainTimerNameTypeToString(clock.timers[i]->name)); return -1; } - if (def->clock.timers[i]->present == 1) + if (clock.timers[i]->present == 1) libxl_defbool_set(&b_info->u.hvm.hpet, 1); break; @@ -329,7 +352,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, case VIR_DOMAIN_TIMER_NAME_PIT: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported timer type (name) '%s'"), - virDomainTimerNameTypeToString(def->clock.timers[i]->name)); + virDomainTimerNameTypeToString(clock.timers[i]->name)); return -1; case VIR_DOMAIN_TIMER_NAME_LAST: diff --git a/tests/libxlxml2domconfigdata/variable-clock-hvm.json b/tests/libxlxml2domconfigdata/variable-clock-hvm.json new file mode 100644 index 000000000..49d0a4330 --- /dev/null +++ b/tests/libxlxml2domconfigdata/variable-clock-hvm.json @@ -0,0 +1,91 @@ +{ + "c_info": { + "type": "hvm", + "name": "test-hvm", + "uuid": "2147d599-9cc6-c0dc-92ab-4064b5446e9b" + }, + "b_info": { + "max_vcpus": 4, + "avail_vcpus": [ + 0, + 1, + 2, + 3 + ], + "max_memkb": 1048576, + "target_memkb": 1048576, + "video_memkb": 8192, + "shadow_memkb": 12288, + "rtc_timeoffset": 3600, + "localtime": "True", + "device_model_version": "qemu_xen", + "device_model": "/bin/true", + "sched_params": { + "weight": 1000 + }, + "type.hvm": { + "pae": "True", + "apic": "True", + "acpi": "True", + "vga": { + "kind": "cirrus" + }, + "vnc": { + "enable": "True", + "listen": "0.0.0.0", + "findunused": "False" + }, + "sdl": { + "enable": "False" + }, + "spice": { + + }, + "boot": "c", + "rdm": { + + } + }, + "arch_arm": { + + } + }, + "disks": [ + { + "pdev_path": "/var/lib/xen/images/test-hvm.img", + "vdev": "hda", + "backend": "qdisk", + "format": "raw", + "removable": 1, + "readwrite": 1 + } + ], + "nics": [ + { + "devid": 0, + "mac": "00:16:3e:66:12:b4", + "bridge": "br0", + "script": "/etc/xen/scripts/vif-bridge", + "nictype": "vif_ioemu" + } + ], + "vfbs": [ + { + "devid": -1, + "vnc": { + "enable": "True", + "listen": "0.0.0.0", + "findunused": "False" + }, + "sdl": { + "enable": "False" + } + } + ], + "vkbs": [ + { + "devid": -1 + } + ], + "on_reboot": "restart" +} diff --git a/tests/libxlxml2domconfigdata/variable-clock-hvm.xml b/tests/libxlxml2domconfigdata/variable-clock-hvm.xml new file mode 100644 index 000000000..608788779 --- /dev/null +++ b/tests/libxlxml2domconfigdata/variable-clock-hvm.xml @@ -0,0 +1,36 @@ +<domain type='xen'> + <name>test-hvm</name> + <description>None</description> + <uuid>2147d599-9cc6-c0dc-92ab-4064b5446e9b</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>4</vcpu> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <clock offset='localtime' adjustment='3600'/> + <os> + <type>hvm</type> + <loader>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='hd'/> + </os> + <features> + <apic/> + <acpi/> + <pae/> + </features> + <devices> + <emulator>/bin/true</emulator> + <disk type='file' device='disk'> + <driver name='qemu'/> + <source file='/var/lib/xen/images/test-hvm.img'/> + <target dev='hda'/> + </disk> + <interface type='bridge'> + <source bridge='br0'/> + <mac address='00:16:3e:66:12:b4'/> + <script path='/etc/xen/scripts/vif-bridge'/> + </interface> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'/> + </devices> +</domain> diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index bd4c3af5a..be3e34944 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -188,6 +188,7 @@ mymain(void) DO_TEST("basic-pv"); DO_TEST("basic-hvm"); + DO_TEST("variable-clock-hvm"); DO_TEST("moredevs-hvm"); DO_TEST("vnuma-hvm"); DO_TEST("multiple-ip"); diff --git a/tests/xlconfigdata/test-fullvirt-tsc-timer.cfg b/tests/xlconfigdata/test-fullvirt-tsc-timer.cfg index 609c0fd5a..587d3461f 100644 --- a/tests/xlconfigdata/test-fullvirt-tsc-timer.cfg +++ b/tests/xlconfigdata/test-fullvirt-tsc-timer.cfg @@ -9,8 +9,8 @@ apic = 1 hap = 0 viridian = 0 tsc_mode = "native" -rtc_timeoffset = 0 -localtime = 0 +rtc_timeoffset = 3600 +localtime = 1 on_poweroff = "destroy" on_reboot = "restart" on_crash = "restart" diff --git a/tests/xlconfigdata/test-fullvirt-tsc-timer.xml b/tests/xlconfigdata/test-fullvirt-tsc-timer.xml index bedbe8e0e..0816f96f3 100644 --- a/tests/xlconfigdata/test-fullvirt-tsc-timer.xml +++ b/tests/xlconfigdata/test-fullvirt-tsc-timer.xml @@ -15,7 +15,7 @@ <pae/> <hap state='off'/> </features> - <clock offset='variable' adjustment='0' basis='utc'> + <clock offset='variable' adjustment='3600' basis='localtime'> <timer name='tsc' present='yes' mode='native'/> </clock> <on_poweroff>destroy</on_poweroff> -- 2.16.1
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> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
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,
participants (2)
-
Daniel P. Berrangé -
Jim Fehlig