[libvirt] [PATCH 3/3] qemu_process: add check for hyperv features

[John Ferlan]

On 03/29/2016 09:31 AM, Pavel Hrdina wrote:
> Commit 7068b56c introduced several hyperv features.  Not all hyperv
> features are supported by old enough kernels and we shouldn't allow to
> start a guest if kernel doesn't support any of the hyperv feature.
> 
> There is one exception, for backward compatibility we cannot error out
> if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
> reason we ignore invtsc, to not break restoring saved domains with older
> libvirt.

>From yesterday's dialog, there's also commit id '59fc0d06' which adds
"hv_crash" and commit id '600bca59' which adds "hv_time".

Neither is handled via these bits, but wouldn't both fall into the same
trap since both were added after commit '2e8f9080'?

> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  src/cpu/cpu_x86.c       |  8 ++++++++
>  src/cpu/cpu_x86_data.h  |  8 ++++++++
>  src/qemu/qemu_process.c | 31 +++++++++++++++++++++++++++++++
>  3 files changed, 47 insertions(+)
> 

Now I was going to ask about a capability bit for this, but seeing none
in previous commits, I thought I was safe... I guess not.

The rest looks OK to me, although I wonder if there's a way to avoid
missing this for future changes

John

> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> index 90949f6..b7f1690 100644
> --- a/src/cpu/cpu_x86.c
> +++ b/src/cpu/cpu_x86.c
> @@ -75,6 +75,14 @@ static const struct x86_kvm_feature x86_kvm_features[] =
>      {VIR_CPU_x86_KVM_PV_UNHALT,    { .function = 0x40000001, .eax = 0x00000080 }},
>      {VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT,
>                                     { .function = 0x40000001, .eax = 0x01000000 }},
> +    {VIR_CPU_x86_KVM_HV_RUNTIME,   { .function = 0x40000003, .eax = 0x00000001 }},
> +    {VIR_CPU_x86_KVM_HV_SYNIC,     { .function = 0x40000003, .eax = 0x00000004 }},
> +    {VIR_CPU_x86_KVM_HV_STIMER,    { .function = 0x40000003, .eax = 0x00000008 }},
> +    {VIR_CPU_x86_KVM_HV_RELAXED,   { .function = 0x40000003, .eax = 0x00000020 }},
> +    {VIR_CPU_x86_KVM_HV_SPINLOCK,  { .function = 0x40000003, .eax = 0x00000022 }},
> +    {VIR_CPU_x86_KVM_HV_VAPIC,     { .function = 0x40000003, .eax = 0x00000030 }},
> +    {VIR_CPU_x86_KVM_HV_VPINDEX,   { .function = 0x40000003, .eax = 0x00000040 }},
> +    {VIR_CPU_x86_KVM_HV_RESET,     { .function = 0x40000003, .eax = 0x00000080 }},
>  };
>  
>  struct x86_model {
> diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
> index 88dccf6..777cc8d 100644
> --- a/src/cpu/cpu_x86_data.h
> +++ b/src/cpu/cpu_x86_data.h
> @@ -48,6 +48,14 @@ struct _virCPUx86CPUID {
>  # define VIR_CPU_x86_KVM_PV_EOI       "__kvm_pv_eoi"
>  # define VIR_CPU_x86_KVM_PV_UNHALT    "__kvm_pv_unhalt"
>  # define VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT "__kvm_clocksource_stable"
> +# define VIR_CPU_x86_KVM_HV_RUNTIME   "__kvm_hv_runtime"
> +# define VIR_CPU_x86_KVM_HV_SYNIC     "__kvm_hv_synic"
> +# define VIR_CPU_x86_KVM_HV_STIMER    "__kvm_hv_stimer"
> +# define VIR_CPU_x86_KVM_HV_RELAXED   "__kvm_hv_relaxed"
> +# define VIR_CPU_x86_KVM_HV_SPINLOCK  "__kvm_hv_spinlock"
> +# define VIR_CPU_x86_KVM_HV_VAPIC     "__kvm_hv_vapic"
> +# define VIR_CPU_x86_KVM_HV_VPINDEX   "__kvm_hv_vpindex"
> +# define VIR_CPU_x86_KVM_HV_RESET     "__kvm_hv_reset"
>  
>  
>  typedef struct _virCPUx86Data virCPUx86Data;
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 9334a75..07b9df2 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3928,6 +3928,37 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
>              }
>          }
>  
> +        for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
> +            if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) {
> +                char *cpuFeature;
> +                if (virAsprintf(&cpuFeature, "__kvm_hv_%s",
> +                                virDomainHypervTypeToString(i)) < 0)
> +                    goto cleanup;
> +                if (!cpuHasFeature(guestcpu, cpuFeature)) {
> +                    switch ((virDomainHyperv) i) {
> +                    case VIR_DOMAIN_HYPERV_RELAXED:
> +                    case VIR_DOMAIN_HYPERV_VAPIC:
> +                    case VIR_DOMAIN_HYPERV_SPINLOCKS:
> +                        VIR_WARN("host doesn't support hyperv '%s' feature",
> +                                 virDomainHypervTypeToString(i));
> +                        break;
> +                    case VIR_DOMAIN_HYPERV_VPINDEX:
> +                    case VIR_DOMAIN_HYPERV_RUNTIME:
> +                    case VIR_DOMAIN_HYPERV_SYNIC:
> +                    case VIR_DOMAIN_HYPERV_STIMER:
> +                    case VIR_DOMAIN_HYPERV_RESET:
> +                    case VIR_DOMAIN_HYPERV_VENDOR_ID:
> +                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                       _("host doesn't support hyperv '%s' feature"),
> +                                       virDomainHypervTypeToString(i));
> +                        goto cleanup;
> +                        break;
> +                    case VIR_DOMAIN_HYPERV_LAST:
> +                        break;
> +                    }
> +                }
> +            }
> +        }
>  
>          if (def->cpu && def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
>              for (i = 0; i < def->cpu->nfeatures; i++) {
>