Use standard error_setg() mechanism in hyperv_expand_features().
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
target/i386/kvm/kvm.c | 101 +++++++++++++++++++++++++-----------------
1 file changed, 61 insertions(+), 40 deletions(-)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a2ef2dc154a2..f33ba325187f 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1135,7 +1135,7 @@ static bool hyperv_feature_supported(CPUState *cs, int feature)
return true;
}
-static int hv_cpuid_check_and_set(CPUState *cs, int feature)
+static int hv_cpuid_check_and_set(CPUState *cs, int feature, Error **errp)
{
X86CPU *cpu = X86_CPU(cs);
uint64_t deps;
@@ -1149,20 +1149,18 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature)
while (deps) {
dep_feat = ctz64(deps);
if (!(hyperv_feat_enabled(cpu, dep_feat))) {
- fprintf(stderr,
- "Hyper-V %s requires Hyper-V %s\n",
- kvm_hyperv_properties[feature].desc,
- kvm_hyperv_properties[dep_feat].desc);
- return 1;
+ error_setg(errp, "Hyper-V %s requires Hyper-V %s",
+ kvm_hyperv_properties[feature].desc,
+ kvm_hyperv_properties[dep_feat].desc);
+ return 1;
}
deps &= ~(1ull << dep_feat);
}
if (!hyperv_feature_supported(cs, feature)) {
if (hyperv_feat_enabled(cpu, feature)) {
- fprintf(stderr,
- "Hyper-V %s is not supported by kernel\n",
- kvm_hyperv_properties[feature].desc);
+ error_setg(errp, "Hyper-V %s is not supported by kernel",
+ kvm_hyperv_properties[feature].desc);
return 1;
} else {
return 0;
@@ -1209,13 +1207,12 @@ static uint32_t hv_build_cpuid_leaf(CPUState *cs, uint32_t func, int reg)
* of 'hv_passthrough' mode and fills the environment with all supported
* Hyper-V features.
*/
-static int hyperv_expand_features(CPUState *cs)
+static void hyperv_expand_features(CPUState *cs, Error **errp)
{
X86CPU *cpu = X86_CPU(cs);
- int r;
if (!hyperv_enabled(cpu))
- return 0;
+ return;
if (cpu->hyperv_passthrough) {
cpu->hyperv_vendor_id[0] =
@@ -1262,37 +1259,60 @@ static int hyperv_expand_features(CPUState *cs)
}
/* Features */
- r = hv_cpuid_check_and_set(cs, HYPERV_FEAT_RELAXED);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_VAPIC);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_TIME);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_CRASH);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_RESET);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_VPINDEX);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_RUNTIME);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_SYNIC);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_FREQUENCIES);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_REENLIGHTENMENT);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_TLBFLUSH);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_EVMCS);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_IPI);
- r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER_DIRECT);
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RELAXED, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_VAPIC, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_TIME, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_CRASH, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RESET, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_VPINDEX, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RUNTIME, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_SYNIC, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_FREQUENCIES, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_REENLIGHTENMENT, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_TLBFLUSH, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_EVMCS, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_IPI, errp)) {
+ return;
+ }
+ if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER_DIRECT, errp)) {
+ return;
+ }
/* Additional dependencies not covered by kvm_hyperv_properties[] */
if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
!cpu->hyperv_synic_kvm_only &&
!hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
- fprintf(stderr, "Hyper-V %s requires Hyper-V %s\n",
- kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
- kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
- r |= 1;
- }
-
- if (r) {
- return -ENOSYS;
+ error_setg(errp, "Hyper-V %s requires Hyper-V %s",
+ kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
+ kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
}
-
- return 0;
}
/*
@@ -1527,9 +1547,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
env->apic_bus_freq = KVM_APIC_BUS_FREQUENCY;
/* Paravirtualization CPUIDs */
- r = hyperv_expand_features(cs);
- if (r < 0) {
- return r;
+ hyperv_expand_features(cs, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ return -ENOSYS;
}
if (hyperv_enabled(cpu)) {
--
2.30.2
On Thu, Apr 22, 2021 at 06:11:22PM +0200, Vitaly Kuznetsov wrote:
> Use standard error_setg() mechanism in hyperv_expand_features().
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
No objections, but only suggestions below:
> ---
> target/i386/kvm/kvm.c | 101 +++++++++++++++++++++++++-----------------
> 1 file changed, 61 insertions(+), 40 deletions(-)
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index a2ef2dc154a2..f33ba325187f 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -1135,7 +1135,7 @@ static bool hyperv_feature_supported(CPUState *cs, int feature)
> return true;
> }
>
> -static int hv_cpuid_check_and_set(CPUState *cs, int feature)
> +static int hv_cpuid_check_and_set(CPUState *cs, int feature, Error **errp)
If changing the function signature, and the function only returns 0 or 1, maybe
it's a good opportunity to change to a bool return value format?
From include/qapi/error.h:
* - Whenever practical, also return a value that indicates success /
* failure. This can make the error checking more concise, and can
* avoid useless error object creation and destruction. Note that
* we still have many functions returning void. We recommend
* • bool-valued functions return true on success / false on failure,
* • pointer-valued functions return non-null / null pointer, and
* • integer-valued functions return non-negative / negative.
> {
> X86CPU *cpu = X86_CPU(cs);
> uint64_t deps;
> @@ -1149,20 +1149,18 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature)
> while (deps) {
> dep_feat = ctz64(deps);
> if (!(hyperv_feat_enabled(cpu, dep_feat))) {
> - fprintf(stderr,
> - "Hyper-V %s requires Hyper-V %s\n",
> - kvm_hyperv_properties[feature].desc,
> - kvm_hyperv_properties[dep_feat].desc);
> - return 1;
> + error_setg(errp, "Hyper-V %s requires Hyper-V %s",
> + kvm_hyperv_properties[feature].desc,
> + kvm_hyperv_properties[dep_feat].desc);
> + return 1;
> }
> deps &= ~(1ull << dep_feat);
> }
>
> if (!hyperv_feature_supported(cs, feature)) {
> if (hyperv_feat_enabled(cpu, feature)) {
> - fprintf(stderr,
> - "Hyper-V %s is not supported by kernel\n",
> - kvm_hyperv_properties[feature].desc);
> + error_setg(errp, "Hyper-V %s is not supported by kernel",
> + kvm_hyperv_properties[feature].desc);
> return 1;
> } else {
> return 0;
> @@ -1209,13 +1207,12 @@ static uint32_t hv_build_cpuid_leaf(CPUState *cs, uint32_t func, int reg)
> * of 'hv_passthrough' mode and fills the environment with all supported
> * Hyper-V features.
> */
> -static int hyperv_expand_features(CPUState *cs)
> +static void hyperv_expand_features(CPUState *cs, Error **errp)
Same as above: returning a value to indicate error is preferred. If you are no
longer returning an integer error code, I suggest returning bool instead.
> {
> X86CPU *cpu = X86_CPU(cs);
> - int r;
>
> if (!hyperv_enabled(cpu))
> - return 0;
> + return;
>
> if (cpu->hyperv_passthrough) {
> cpu->hyperv_vendor_id[0] =
> @@ -1262,37 +1259,60 @@ static int hyperv_expand_features(CPUState *cs)
> }
>
> /* Features */
> - r = hv_cpuid_check_and_set(cs, HYPERV_FEAT_RELAXED);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_VAPIC);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_TIME);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_CRASH);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_RESET);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_VPINDEX);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_RUNTIME);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_SYNIC);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_FREQUENCIES);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_REENLIGHTENMENT);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_TLBFLUSH);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_EVMCS);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_IPI);
> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER_DIRECT);
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RELAXED, errp)) {
> + return;
> + }
What about a loop?
for (feat = 0; feat < ARRAY_SIZE(kvm_hyperv_properties); feat++) {
if (hv_cpuid_check_and_set(cs, feat, errp)) {
return;
}
}
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_VAPIC, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_TIME, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_CRASH, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RESET, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_VPINDEX, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RUNTIME, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_SYNIC, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_FREQUENCIES, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_REENLIGHTENMENT, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_TLBFLUSH, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_EVMCS, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_IPI, errp)) {
> + return;
> + }
> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER_DIRECT, errp)) {
> + return;
> + }
>
> /* Additional dependencies not covered by kvm_hyperv_properties[] */
> if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
> !cpu->hyperv_synic_kvm_only &&
> !hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
> - fprintf(stderr, "Hyper-V %s requires Hyper-V %s\n",
> - kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
> - kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
> - r |= 1;
> - }
> -
> - if (r) {
> - return -ENOSYS;
> + error_setg(errp, "Hyper-V %s requires Hyper-V %s",
> + kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
> + kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
> }
> -
> - return 0;
> }
>
> /*
> @@ -1527,9 +1547,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
> env->apic_bus_freq = KVM_APIC_BUS_FREQUENCY;
>
> /* Paravirtualization CPUIDs */
> - r = hyperv_expand_features(cs);
> - if (r < 0) {
> - return r;
> + hyperv_expand_features(cs, &local_err);
> + if (local_err) {
> + error_report_err(local_err);
> + return -ENOSYS;
> }
>
> if (hyperv_enabled(cpu)) {
I don't want to block this series because of the suggestions above, so:
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
But I still encourage you to implement those suggestions, anyway.
--
Eduardo
Eduardo Habkost <ehabkost@redhat.com> writes:
> On Thu, Apr 22, 2021 at 06:11:22PM +0200, Vitaly Kuznetsov wrote:
>> Use standard error_setg() mechanism in hyperv_expand_features().
>>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>
> No objections, but only suggestions below:
>
>> ---
>> target/i386/kvm/kvm.c | 101 +++++++++++++++++++++++++-----------------
>> 1 file changed, 61 insertions(+), 40 deletions(-)
>>
>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>> index a2ef2dc154a2..f33ba325187f 100644
>> --- a/target/i386/kvm/kvm.c
>> +++ b/target/i386/kvm/kvm.c
>> @@ -1135,7 +1135,7 @@ static bool hyperv_feature_supported(CPUState *cs, int feature)
>> return true;
>> }
>>
>> -static int hv_cpuid_check_and_set(CPUState *cs, int feature)
>> +static int hv_cpuid_check_and_set(CPUState *cs, int feature, Error **errp)
>
> If changing the function signature, and the function only returns 0 or 1, maybe
> it's a good opportunity to change to a bool return value format?
>
> From include/qapi/error.h:
>
> * - Whenever practical, also return a value that indicates success /
> * failure. This can make the error checking more concise, and can
> * avoid useless error object creation and destruction. Note that
> * we still have many functions returning void. We recommend
> * • bool-valued functions return true on success / false on failure,
> * • pointer-valued functions return non-null / null pointer, and
> * • integer-valued functions return non-negative / negative.
>
>
>> {
>> X86CPU *cpu = X86_CPU(cs);
>> uint64_t deps;
>> @@ -1149,20 +1149,18 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature)
>> while (deps) {
>> dep_feat = ctz64(deps);
>> if (!(hyperv_feat_enabled(cpu, dep_feat))) {
>> - fprintf(stderr,
>> - "Hyper-V %s requires Hyper-V %s\n",
>> - kvm_hyperv_properties[feature].desc,
>> - kvm_hyperv_properties[dep_feat].desc);
>> - return 1;
>> + error_setg(errp, "Hyper-V %s requires Hyper-V %s",
>> + kvm_hyperv_properties[feature].desc,
>> + kvm_hyperv_properties[dep_feat].desc);
>> + return 1;
>> }
>> deps &= ~(1ull << dep_feat);
>> }
>>
>> if (!hyperv_feature_supported(cs, feature)) {
>> if (hyperv_feat_enabled(cpu, feature)) {
>> - fprintf(stderr,
>> - "Hyper-V %s is not supported by kernel\n",
>> - kvm_hyperv_properties[feature].desc);
>> + error_setg(errp, "Hyper-V %s is not supported by kernel",
>> + kvm_hyperv_properties[feature].desc);
>> return 1;
>> } else {
>> return 0;
>> @@ -1209,13 +1207,12 @@ static uint32_t hv_build_cpuid_leaf(CPUState *cs, uint32_t func, int reg)
>> * of 'hv_passthrough' mode and fills the environment with all supported
>> * Hyper-V features.
>> */
>> -static int hyperv_expand_features(CPUState *cs)
>> +static void hyperv_expand_features(CPUState *cs, Error **errp)
>
> Same as above: returning a value to indicate error is preferred. If you are no
> longer returning an integer error code, I suggest returning bool instead.
>
hv_cpuid_check_and_set() is eliminated later in the series but
hyperv_expand_features() stays, I can make it bool.
>> {
>> X86CPU *cpu = X86_CPU(cs);
>> - int r;
>>
>> if (!hyperv_enabled(cpu))
>> - return 0;
>> + return;
>>
>> if (cpu->hyperv_passthrough) {
>> cpu->hyperv_vendor_id[0] =
>> @@ -1262,37 +1259,60 @@ static int hyperv_expand_features(CPUState *cs)
>> }
>>
>> /* Features */
>> - r = hv_cpuid_check_and_set(cs, HYPERV_FEAT_RELAXED);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_VAPIC);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_TIME);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_CRASH);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_RESET);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_VPINDEX);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_RUNTIME);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_SYNIC);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_FREQUENCIES);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_REENLIGHTENMENT);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_TLBFLUSH);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_EVMCS);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_IPI);
>> - r |= hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER_DIRECT);
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RELAXED, errp)) {
>> + return;
>> + }
>
> What about a loop?
>
> for (feat = 0; feat < ARRAY_SIZE(kvm_hyperv_properties); feat++) {
> if (hv_cpuid_check_and_set(cs, feat, errp)) {
> return;
> }
> }
>
This is done later in the series ("i386: kill off hv_cpuid_check_and_set()").
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_VAPIC, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_TIME, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_CRASH, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RESET, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_VPINDEX, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_RUNTIME, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_SYNIC, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_FREQUENCIES, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_REENLIGHTENMENT, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_TLBFLUSH, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_EVMCS, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_IPI, errp)) {
>> + return;
>> + }
>> + if (hv_cpuid_check_and_set(cs, HYPERV_FEAT_STIMER_DIRECT, errp)) {
>> + return;
>> + }
>>
>> /* Additional dependencies not covered by kvm_hyperv_properties[] */
>> if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
>> !cpu->hyperv_synic_kvm_only &&
>> !hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
>> - fprintf(stderr, "Hyper-V %s requires Hyper-V %s\n",
>> - kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
>> - kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
>> - r |= 1;
>> - }
>> -
>> - if (r) {
>> - return -ENOSYS;
>> + error_setg(errp, "Hyper-V %s requires Hyper-V %s",
>> + kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
>> + kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
>> }
>> -
>> - return 0;
>> }
>>
>> /*
>> @@ -1527,9 +1547,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
>> env->apic_bus_freq = KVM_APIC_BUS_FREQUENCY;
>>
>> /* Paravirtualization CPUIDs */
>> - r = hyperv_expand_features(cs);
>> - if (r < 0) {
>> - return r;
>> + hyperv_expand_features(cs, &local_err);
>> + if (local_err) {
>> + error_report_err(local_err);
>> + return -ENOSYS;
>> }
>>
>> if (hyperv_enabled(cpu)) {
>
> I don't want to block this series because of the suggestions above, so:
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>
> But I still encourage you to implement those suggestions, anyway.
'Loop' idea is already implemented and hv_cpuid_check_and_set() is gone
but I'll remember to make hyperv_expand_features() bool. Thanks!
--
Vitaly
© 2016 - 2026 Red Hat, Inc.