[libvirt PATCH v2 07/20] cpu_x86: Implement virCPUDataIsIdentical for x86

[Michal Prívozník]

On 11/4/21 5:27 PM, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh at redhat.com>
> ---
>  src/cpu/cpu_x86.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> index a08ac225ef..5ce193e693 100644
> --- a/src/cpu/cpu_x86.c
> +++ b/src/cpu/cpu_x86.c
> @@ -3332,6 +3332,74 @@ virCPUx86DataAddFeature(virCPUData *cpuData,
>  }
>  
>  
> +static bool
> +virCPUx86DataItemIsIdentical(const virCPUx86DataItem *a,
> +                             const virCPUx86DataItem *b)
> +{
> +    if (a->type != b->type)
> +        return false;
> +
> +    switch (a->type) {
> +    case VIR_CPU_X86_DATA_NONE:
> +        break;
> +
> +    case VIR_CPU_X86_DATA_CPUID:
> +        return a->data.cpuid.eax_in == b->data.cpuid.eax_in &&
> +               a->data.cpuid.ecx_in == b->data.cpuid.ecx_in &&
> +               a->data.cpuid.eax == b->data.cpuid.eax &&
> +               a->data.cpuid.ebx == b->data.cpuid.ebx &&
> +               a->data.cpuid.ecx == b->data.cpuid.ecx &&
> +               a->data.cpuid.edx == b->data.cpuid.edx;

So this can be replaced with memcmp(), but the moment we will want to
store a pointer in .cpuid we will have to rewrite the code to this
explicit variant. So I guess keep it as is?

> +
> +    case VIR_CPU_X86_DATA_MSR:
> +        return a->data.msr.index == b->data.msr.index &&
> +               a->data.msr.eax == b->data.msr.eax &&
> +               a->data.msr.edx == b->data.msr.edx;
> +    }
> +
> +    return true;
> +}
> +
> +static virCPUCompareResult
> +virCPUx86DataIsIdentical(const virCPUData *a,
> +                         const virCPUData *b)
> +{
> +    const virCPUx86Data *adata;
> +    const virCPUx86Data *bdata;
> +    size_t i;
> +    size_t j;
> +
> +    if (!a || !b)
> +        return VIR_CPU_COMPARE_ERROR;
> +
> +    if (a->arch != b->arch)
> +        return VIR_CPU_COMPARE_INCOMPATIBLE;
> +
> +    if (!((adata = &a->data.x86) && (bdata = &b->data.x86)))
> +        return VIR_CPU_COMPARE_ERROR;
> +
> +    if (adata->len != bdata->len)
> +        return VIR_CPU_COMPARE_INCOMPATIBLE;
> +
> +    for (i = 0; i < adata->len; ++i) {
> +        bool found = false;
> +
> +        for (j = 0; j < bdata->len; ++j) {
> +            if (!virCPUx86DataItemIsIdentical(&adata->items[i],
> +                                              &bdata->items[j]))
> +                continue;
> +
> +            found = true;
> +            break;
> +        }
> +
> +        if (!found)
> +            return VIR_CPU_COMPARE_INCOMPATIBLE;

Do we need this? I mean, couldn't we replace 'found = true' with ;return
VIR_CPU_COMPARE_IDENTICAL;' Or even better, drop the negation in if and
return the identical value instead of continue.

> +    }
> +
> +    return VIR_CPU_COMPARE_IDENTICAL;

This can then be INCOMPATIBLE.

> +}
> +
>  static bool
>  virCPUx86FeatureIsMSR(const char *name)
>  {
> @@ -3415,4 +3483,5 @@ struct cpuArchDriver cpuDriverX86 = {
>      .copyMigratable = virCPUx86CopyMigratable,
>      .validateFeatures = virCPUx86ValidateFeatures,
>      .dataAddFeature = virCPUx86DataAddFeature,
> +    .dataIsIdentical = virCPUx86DataIsIdentical,
>  };
> 

Michal