From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Libvirt prints an error on startup when it is missing host cpu model
information for any queried qemu binary. On s390 we only have host cpu model
information for kvm enabled qemu instances. So when virt type is not kvm, this
is actually not an error on s390.
This patch adds virt type as a parameter to virQEMUCapsInitCPUModelS390, and a
new return code 2 for virQEMUCapsInitCPUModel and virQEMUCapsInitCPUModelS390.
If the virt type is not kvm then we skip printing the scary error message
and return 2 because this case is actually expected behavior. The new return
code is meant to differentiate between the failure case and the case where we
simply expect the cpu model information to be unattainable.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
src/qemu/qemu_capabilities.c | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3adea66..b073841 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3306,22 +3306,28 @@ virQEMUCapsCPUFilterFeatures(const char *name,
/**
* Returns 0 when host CPU model provided by QEMU was filled in qemuCaps,
* 1 when the caller should fall back to using virCapsPtr->host.cpu,
+ * 2 when cpu model info is not supported for this configuration and
+ * fall back should not be used.
* -1 on error.
*/
static int
virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
qemuMonitorCPUModelInfoPtr modelInfo,
virCPUDefPtr cpu,
- bool migratable)
+ bool migratable,
+ virDomainVirtType type)
{
size_t i;
if (!modelInfo) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("missing host CPU model info from QEMU capabilities "
- "for binary %s"),
- qemuCaps->binary);
- return -1;
+ if (type == VIR_DOMAIN_VIRT_KVM) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing host CPU model info from QEMU "
+ "capabilities for binary %s"),
+ qemuCaps->binary);
+ return -1;
+ }
+ return 2;
}
if (VIR_STRDUP(cpu->model, modelInfo->name) < 0 ||
@@ -3429,6 +3435,8 @@ virQEMUCapsInitCPUModelX86(virQEMUCapsPtr qemuCaps,
/**
* Returns 0 when host CPU model provided by QEMU was filled in qemuCaps,
* 1 when the caller should fall back to other methods
+ * 2 when cpu model info is not supported for this configuration and
+ * fall back should not be used.
* -1 on error.
*/
int
@@ -3445,13 +3453,13 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
if (ARCH_IS_S390(qemuCaps->arch)) {
ret = virQEMUCapsInitCPUModelS390(qemuCaps, cpuData->info,
- cpu, migratable);
+ cpu, migratable, type);
} else if (ARCH_IS_X86(qemuCaps->arch)) {
ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, cpuData->info,
cpu, migratable);
}
- if (ret == 0)
+ if (ret == 0 || ret == 2)
cpu->fallback = VIR_CPU_FALLBACK_FORBID;
return ret;
@@ -3504,6 +3512,12 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
virQEMUCapsCPUFilterFeatures,
&qemuCaps->arch) < 0)
goto error;
+ } else if (rc == 2) {
+ VIR_DEBUG("Qemu does not provide cpu model for arch=%s virttype=%s",
+ virArchToString(qemuCaps->arch),
+ virDomainVirtTypeToString(type));
+ virCPUDefFree(cpu);
+ goto cleanup;
} else if (type == VIR_DOMAIN_VIRT_KVM &&
virCPUGetHostIsSupported(qemuCaps->arch)) {
if (!(fullCPU = virCPUGetHost(qemuCaps->arch, VIR_CPU_TYPE_GUEST,
--
2.9.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Nov 24, 2017 at 09:02:02 +0100, Boris Fiuczynski wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> Libvirt prints an error on startup when it is missing host cpu model
> information for any queried qemu binary. On s390 we only have host cpu model
> information for kvm enabled qemu instances. So when virt type is not kvm, this
> is actually not an error on s390.
>
> This patch adds virt type as a parameter to virQEMUCapsInitCPUModelS390, and a
> new return code 2 for virQEMUCapsInitCPUModel and virQEMUCapsInitCPUModelS390.
> If the virt type is not kvm then we skip printing the scary error message
> and return 2 because this case is actually expected behavior. The new return
> code is meant to differentiate between the failure case and the case where we
> simply expect the cpu model information to be unattainable.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
> ---
> src/qemu/qemu_capabilities.c | 30 ++++++++++++++++++++++--------
> 1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 3adea66..b073841 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -3306,22 +3306,28 @@ virQEMUCapsCPUFilterFeatures(const char *name,
> /**
> * Returns 0 when host CPU model provided by QEMU was filled in qemuCaps,
> * 1 when the caller should fall back to using virCapsPtr->host.cpu,
> + * 2 when cpu model info is not supported for this configuration and
> + * fall back should not be used.
> * -1 on error.
> */
> static int
> virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
> qemuMonitorCPUModelInfoPtr modelInfo,
> virCPUDefPtr cpu,
> - bool migratable)
> + bool migratable,
> + virDomainVirtType type)
virQEMUCapsInitCPUModelX86 is declared with 'type' being the second
parameter. Let's make it consistent.
> {
> size_t i;
>
> if (!modelInfo) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("missing host CPU model info from QEMU capabilities "
> - "for binary %s"),
> - qemuCaps->binary);
> - return -1;
> + if (type == VIR_DOMAIN_VIRT_KVM) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("missing host CPU model info from QEMU "
> + "capabilities for binary %s"),
> + qemuCaps->binary);
> + return -1;
> + }
> + return 2;
> }
>
> if (VIR_STRDUP(cpu->model, modelInfo->name) < 0 ||
> @@ -3429,6 +3435,8 @@ virQEMUCapsInitCPUModelX86(virQEMUCapsPtr qemuCaps,
> /**
> * Returns 0 when host CPU model provided by QEMU was filled in qemuCaps,
> * 1 when the caller should fall back to other methods
> + * 2 when cpu model info is not supported for this configuration and
> + * fall back should not be used.
> * -1 on error.
> */
> int
> @@ -3445,13 +3453,13 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
>
> if (ARCH_IS_S390(qemuCaps->arch)) {
> ret = virQEMUCapsInitCPUModelS390(qemuCaps, cpuData->info,
> - cpu, migratable);
> + cpu, migratable, type);
This would need to be adjusted, of course.
> } else if (ARCH_IS_X86(qemuCaps->arch)) {
> ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, cpuData->info,
> cpu, migratable);
> }
>
> - if (ret == 0)
> + if (ret == 0 || ret == 2)
This does not make sense. The cpu is not filled in by QEMU and thus
fallback should not be set to "forbid". Not to mention the caller will
just free the cpu structure anyway.
> cpu->fallback = VIR_CPU_FALLBACK_FORBID;
>
> return ret;
> @@ -3504,6 +3512,12 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
> virQEMUCapsCPUFilterFeatures,
> &qemuCaps->arch) < 0)
> goto error;
> + } else if (rc == 2) {
> + VIR_DEBUG("Qemu does not provide cpu model for arch=%s virttype=%s",
s/Qemu/QEMU/; s/cpu/CPU/
> + virArchToString(qemuCaps->arch),
> + virDomainVirtTypeToString(type));
> + virCPUDefFree(cpu);
> + goto cleanup;
And I'd just jump to the 'error' label instead of freeing cpu here.
> } else if (type == VIR_DOMAIN_VIRT_KVM &&
> virCPUGetHostIsSupported(qemuCaps->arch)) {
> if (!(fullCPU = virCPUGetHost(qemuCaps->arch, VIR_CPU_TYPE_GUEST,
ACK with the suggested nits fixed. I pushed the patch with the following
diff squashed in.
Thanks.
Jirka
diff --git i/src/qemu/qemu_capabilities.c w/src/qemu/qemu_capabilities.c
index b073841e8b..201e83c45c 100644
--- i/src/qemu/qemu_capabilities.c
+++ w/src/qemu/qemu_capabilities.c
@@ -3312,10 +3312,10 @@ virQEMUCapsCPUFilterFeatures(const char *name,
*/
static int
virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
+ virDomainVirtType type,
qemuMonitorCPUModelInfoPtr modelInfo,
virCPUDefPtr cpu,
- bool migratable,
- virDomainVirtType type)
+ bool migratable)
{
size_t i;
@@ -3452,14 +3452,14 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
return 1;
if (ARCH_IS_S390(qemuCaps->arch)) {
- ret = virQEMUCapsInitCPUModelS390(qemuCaps, cpuData->info,
- cpu, migratable, type);
+ ret = virQEMUCapsInitCPUModelS390(qemuCaps, type, cpuData->info,
+ cpu, migratable);
} else if (ARCH_IS_X86(qemuCaps->arch)) {
ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, cpuData->info,
cpu, migratable);
}
- if (ret == 0 || ret == 2)
+ if (ret == 0)
cpu->fallback = VIR_CPU_FALLBACK_FORBID;
return ret;
@@ -3513,11 +3513,10 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
&qemuCaps->arch) < 0)
goto error;
} else if (rc == 2) {
- VIR_DEBUG("Qemu does not provide cpu model for arch=%s virttype=%s",
+ VIR_DEBUG("QEMU does not provide CPU model for arch=%s virttype=%s",
virArchToString(qemuCaps->arch),
virDomainVirtTypeToString(type));
- virCPUDefFree(cpu);
- goto cleanup;
+ goto error;
} else if (type == VIR_DOMAIN_VIRT_KVM &&
virCPUGetHostIsSupported(qemuCaps->arch)) {
if (!(fullCPU = virCPUGetHost(qemuCaps->arch, VIR_CPU_TYPE_GUEST,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Jiri,
thanks for the ack and pushing.
The nits you fixed need a tiny cleanup in the textual description of the
return values. I will send a patch.
On 11/24/2017 04:56 PM, Jiri Denemark wrote:
> On Fri, Nov 24, 2017 at 09:02:02 +0100, Boris Fiuczynski wrote:
>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>
>> Libvirt prints an error on startup when it is missing host cpu model
>> information for any queried qemu binary. On s390 we only have host cpu model
>> information for kvm enabled qemu instances. So when virt type is not kvm, this
>> is actually not an error on s390.
>>
>> This patch adds virt type as a parameter to virQEMUCapsInitCPUModelS390, and a
>> new return code 2 for virQEMUCapsInitCPUModel and virQEMUCapsInitCPUModelS390.
>> If the virt type is not kvm then we skip printing the scary error message
>> and return 2 because this case is actually expected behavior. The new return
>> code is meant to differentiate between the failure case and the case where we
>> simply expect the cpu model information to be unattainable.
>>
>> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
>> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
>> ---
>> src/qemu/qemu_capabilities.c | 30 ++++++++++++++++++++++--------
>> 1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 3adea66..b073841 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -3306,22 +3306,28 @@ virQEMUCapsCPUFilterFeatures(const char *name,
>> /**
>> * Returns 0 when host CPU model provided by QEMU was filled in qemuCaps,
>> * 1 when the caller should fall back to using virCapsPtr->host.cpu,
>> + * 2 when cpu model info is not supported for this configuration and
>> + * fall back should not be used.
Remove "and fall back should not be used."
>> * -1 on error.
>> */
>> static int
>> virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
>> qemuMonitorCPUModelInfoPtr modelInfo,
>> virCPUDefPtr cpu,
>> - bool migratable)
>> + bool migratable,
>> + virDomainVirtType type)
>
> virQEMUCapsInitCPUModelX86 is declared with 'type' being the second
> parameter. Let's make it consistent.
>
>> {
>> size_t i;
>>
>> if (!modelInfo) {
>> - virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("missing host CPU model info from QEMU capabilities "
>> - "for binary %s"),
>> - qemuCaps->binary);
>> - return -1;
>> + if (type == VIR_DOMAIN_VIRT_KVM) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("missing host CPU model info from QEMU "
>> + "capabilities for binary %s"),
>> + qemuCaps->binary);
>> + return -1;
>> + }
>> + return 2;
>> }
>>
>> if (VIR_STRDUP(cpu->model, modelInfo->name) < 0 ||
>> @@ -3429,6 +3435,8 @@ virQEMUCapsInitCPUModelX86(virQEMUCapsPtr qemuCaps,
>> /**
>> * Returns 0 when host CPU model provided by QEMU was filled in qemuCaps,
>> * 1 when the caller should fall back to other methods
>> + * 2 when cpu model info is not supported for this configuration and
>> + * fall back should not be used.
Remove "and fall back should not be used."
>> * -1 on error.
>> */
>> int
>> @@ -3445,13 +3453,13 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
>>
>> if (ARCH_IS_S390(qemuCaps->arch)) {
>> ret = virQEMUCapsInitCPUModelS390(qemuCaps, cpuData->info,
>> - cpu, migratable);
>> + cpu, migratable, type);
>
> This would need to be adjusted, of course.
>
>> } else if (ARCH_IS_X86(qemuCaps->arch)) {
>> ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, cpuData->info,
>> cpu, migratable);
>> }
>>
>> - if (ret == 0)
>> + if (ret == 0 || ret == 2)
>
> This does not make sense. The cpu is not filled in by QEMU and thus
> fallback should not be set to "forbid". Not to mention the caller will
> just free the cpu structure anyway.
>
>> cpu->fallback = VIR_CPU_FALLBACK_FORBID;
>>
>> return ret;
>> @@ -3504,6 +3512,12 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>> virQEMUCapsCPUFilterFeatures,
>> &qemuCaps->arch) < 0)
>> goto error;
>> + } else if (rc == 2) {
>> + VIR_DEBUG("Qemu does not provide cpu model for arch=%s virttype=%s",
>
> s/Qemu/QEMU/; s/cpu/CPU/
>
>> + virArchToString(qemuCaps->arch),
>> + virDomainVirtTypeToString(type));
>> + virCPUDefFree(cpu);
>> + goto cleanup;
>
> And I'd just jump to the 'error' label instead of freeing cpu here.
>
>> } else if (type == VIR_DOMAIN_VIRT_KVM &&
>> virCPUGetHostIsSupported(qemuCaps->arch)) {
>> if (!(fullCPU = virCPUGetHost(qemuCaps->arch, VIR_CPU_TYPE_GUEST,
>
> ACK with the suggested nits fixed. I pushed the patch with the following
> diff squashed in.
>
> Thanks.
>
> Jirka
>
>
> diff --git i/src/qemu/qemu_capabilities.c w/src/qemu/qemu_capabilities.c
> index b073841e8b..201e83c45c 100644
> --- i/src/qemu/qemu_capabilities.c
> +++ w/src/qemu/qemu_capabilities.c
> @@ -3312,10 +3312,10 @@ virQEMUCapsCPUFilterFeatures(const char *name,
> */
> static int
> virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
> + virDomainVirtType type,
> qemuMonitorCPUModelInfoPtr modelInfo,
> virCPUDefPtr cpu,
> - bool migratable,
> - virDomainVirtType type)
> + bool migratable)
> {
> size_t i;
>
> @@ -3452,14 +3452,14 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
> return 1;
>
> if (ARCH_IS_S390(qemuCaps->arch)) {
> - ret = virQEMUCapsInitCPUModelS390(qemuCaps, cpuData->info,
> - cpu, migratable, type);
> + ret = virQEMUCapsInitCPUModelS390(qemuCaps, type, cpuData->info,
> + cpu, migratable);
> } else if (ARCH_IS_X86(qemuCaps->arch)) {
> ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, cpuData->info,
> cpu, migratable);
> }
>
> - if (ret == 0 || ret == 2)
> + if (ret == 0)
> cpu->fallback = VIR_CPU_FALLBACK_FORBID;
>
> return ret;
> @@ -3513,11 +3513,10 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
> &qemuCaps->arch) < 0)
> goto error;
> } else if (rc == 2) {
> - VIR_DEBUG("Qemu does not provide cpu model for arch=%s virttype=%s",
> + VIR_DEBUG("QEMU does not provide CPU model for arch=%s virttype=%s",
> virArchToString(qemuCaps->arch),
> virDomainVirtTypeToString(type));
> - virCPUDefFree(cpu);
> - goto cleanup;
> + goto error;
> } else if (type == VIR_DOMAIN_VIRT_KVM &&
> virCPUGetHostIsSupported(qemuCaps->arch)) {
> if (!(fullCPU = virCPUGetHost(qemuCaps->arch, VIR_CPU_TYPE_GUEST,
>
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.