[libvirt] [PATCH] s390: qemu-capabilities: Avoid error message when missing non-kvm host cpu info
Boris Fiuczynski
fiuczy at linux.vnet.ibm.com
Mon Nov 27 12:12:04 UTC 2017
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 at 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 at linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
>> Reviewed-by: Marc Hartmayer <mhartmay at 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
More information about the libvir-list
mailing list