On 01/20/2017 01:59 AM, Jim Fehlig wrote:
> The current logic around configuring timers in libxl based on
> virDomainDef object is a bit brain dead. Unsupported timers are
> silently ignored and tsc is only recognized if it is the first
> timer specified.
>
> Change the logic to reject unsupported timers and honor the tsc
> timer regardless of its order when multiple timers are specified.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
> src/libxl/libxl_conf.c | 40 +++++++++++++++++++++++++++++++---------
> 1 file changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index a24f9e0..3e6d623 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -313,9 +313,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> for (i = 0; i < virDomainDefGetVcpus(def); i++)
> libxl_bitmap_set((&b_info->avail_vcpus), i);
>
> - if (def->clock.ntimers > 0 &&
> - def->clock.timers[0]->name == VIR_DOMAIN_TIMER_NAME_TSC) {
> - switch (def->clock.timers[0]->mode) {
> + for (i = 0; i < def->clock.ntimers; i++) {
> + switch ((virDomainTimerNameType) def->clock.timers[i]->name) {
> + case VIR_DOMAIN_TIMER_NAME_TSC:
> + switch (def->clock.timers[i]->mode) {
> case VIR_DOMAIN_TIMER_MODE_NATIVE:
> b_info->tsc_mode = 2;
> break;
> @@ -324,8 +325,35 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> break;
> default:
> b_info->tsc_mode = 1;
> + }
> + break;
> +
> + case VIR_DOMAIN_TIMER_NAME_HPET:
> + if (!hvm) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unsupported timer type (name) '%s'"),
> + virDomainTimerNameTypeToString(def->clock.timers[i]->name));
> + return -1;
> + }
> + if (def->clock.timers[i]->present == 1)
> + libxl_defbool_set(&b_info->u.hvm.hpet, 1);
> + break;
> +
> + case VIR_DOMAIN_TIMER_NAME_PLATFORM:
> + case VIR_DOMAIN_TIMER_NAME_KVMCLOCK:
> + case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK:
Interestingly, and looking at the code [toolstack:
tools/libxl/libxl_dom.c:hvm_set_viridian_features(), hypervisor:
xen/arch/x86/hvm/viridian.c:update_reference_tsc()] it looks like that to some
extent we support the Hyper-V Reference TSC page (Hyper-V Clock?) but it is kept
disabled in the viridian base features in libxl. Even if we wanted to enable it
would have to be more specific than adding <viridian /> feature like with:
libxl_bitmap_set(&b_info->u.hvm.viridian_enable,
LIBXL_VIRIDIAN_ENLIGHTENMENT_REFERENCE_BASE);
libxl_bitmap_set(&b_info->u.hvm.viridian_enable,
LIBXL_VIRIDIAN_ENLIGHTENMENT_REFERENCE_TSC);
and perhaps behind "#ifdef LIBXL_VIRIDIAN_ENLIGHTENMENT_REFERENCE_TSC" as this
is only supported since Xen 4.6.0? And I think it would work with tsc_mode not
being emulated? Relevant commit on Xen is dd94caca5 ("x86/viridian: add
Partition Reference Time enlightenment") Anyhow this is probably not in the
context of this series, just food for thought in case we want to have in the future.
> + case VIR_DOMAIN_TIMER_NAME_RTC:
> + case VIR_DOMAIN_TIMER_NAME_PIT:
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unsupported timer type (name) '%s'"),
> + virDomainTimerNameTypeToString(def->clock.timers[i]->name));
> + return -1;
> +
> + case VIR_DOMAIN_TIMER_NAME_LAST:
> + break;
> }
> }
> +
Spurious whitespace? Or perhaps it was purposely added?
> b_info->sched_params.weight = 1000;
> b_info->max_memkb = virDomainDefGetMemoryInitial(def);
> b_info->target_memkb = def->mem.cur_balloon;
> @@ -341,12 +369,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> libxl_defbool_set(&b_info->u.hvm.acpi,
> def->features[VIR_DOMAIN_FEATURE_ACPI] ==
> VIR_TRISTATE_SWITCH_ON);
> - for (i = 0; i < def->clock.ntimers; i++) {
> - if (def->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_HPET &&
> - def->clock.timers[i]->present == 1) {
> - libxl_defbool_set(&b_info->u.hvm.hpet, 1);
> - }
> - }
>
> if (def->nsounds > 0) {
> /*
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
