Re: [libvirt PATCH v2 07/20] cpu_x86: Implement virCPUDataIsIdentical for x86
On 11/4/21 5:27 PM, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)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