[libvirt] [PATCH] cpuCompare: Fix comparison of two host CPUs
When a CPU to be compared with host CPU describes a host CPU instead of a guest CPU, the result is incorrect. This is because instead of treating additional features in host CPU description as required, they were treated as if they were mentioned with all possible policies at the same time. --- src/cpu/cpu_x86.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 114235c..ab7c8cc 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -826,8 +826,7 @@ x86ModelFromCPU(const virCPUDefPtr cpu, struct x86_model *model = NULL; int i; - if (cpu->type == VIR_CPU_TYPE_HOST - || policy == VIR_CPU_FEATURE_REQUIRE) { + if (policy == VIR_CPU_FEATURE_REQUIRE) { if ((model = x86ModelFind(map, cpu->model)) == NULL) { virCPUReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU model %s"), cpu->model); @@ -839,6 +838,8 @@ x86ModelFromCPU(const virCPUDefPtr cpu, } else if (VIR_ALLOC(model) < 0) goto no_memory; + else if (cpu->type == VIR_CPU_TYPE_HOST) + return model; for (i = 0; i < cpu->nfeatures; i++) { const struct x86_feature *feature; @@ -1181,7 +1182,7 @@ x86Compute(virCPUDefPtr host, } if (!(map = x86LoadMap()) || - !(host_model = x86ModelFromCPU(host, map, 0)) || + !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE)) || !(cpu_force = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_FORCE)) || !(cpu_require = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_REQUIRE)) || !(cpu_optional = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_OPTIONAL)) || @@ -1611,7 +1612,7 @@ x86Baseline(virCPUDefPtr *cpus, if (!(map = x86LoadMap())) goto error; - if (!(base_model = x86ModelFromCPU(cpus[0], map, 0))) + if (!(base_model = x86ModelFromCPU(cpus[0], map, VIR_CPU_FEATURE_REQUIRE))) goto error; if (VIR_ALLOC(cpu) < 0 || @@ -1630,7 +1631,7 @@ x86Baseline(virCPUDefPtr *cpus, for (i = 1; i < ncpus; i++) { const char *vn = NULL; - if (!(model = x86ModelFromCPU(cpus[i], map, 0))) + if (!(model = x86ModelFromCPU(cpus[i], map, VIR_CPU_FEATURE_REQUIRE))) goto error; if (cpus[i]->vendor && model->vendor && @@ -1710,7 +1711,7 @@ x86Update(virCPUDefPtr guest, union cpuData *data = NULL; if (!(map = x86LoadMap()) || - !(host_model = x86ModelFromCPU(host, map, 0))) + !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE))) goto cleanup; for (i = 0; i < guest->nfeatures; i++) { -- 1.7.1.1
On Mon, Jul 12, 2010 at 05:49:36PM +0200, Jiri Denemark wrote:
When a CPU to be compared with host CPU describes a host CPU instead of a guest CPU, the result is incorrect. This is because instead of treating additional features in host CPU description as required, they were treated as if they were mentioned with all possible policies at the same time.
Looks fine, ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
When a CPU to be compared with host CPU describes a host CPU instead of a guest CPU, the result is incorrect. This is because instead of treating additional features in host CPU description as required, they were treated as if they were mentioned with all possible policies at the same time.
Looks fine, ACK
Thanks, pushed. Jirka
Jiri, I have an unrelated concern regarding the semantics of comparison of host and guest CPUs. I do not compare CPUs for fun, but rather to know if a guest can be run on a specific host. However, this is not exactly what virConnectCompareCPU gives me: A vmx-enabled host cpu is a superset of itself, but it would not run itself as a guest since we do not have nested vxm yet. Similarly, if we ever have svm-emulation-by-kvm, we could be running svm guests on a vmx host. Is it only me? Or should libvirt expose the more interesting meaning? Dan. On Mon, Jul 12, 2010 at 05:49:36PM +0200, Jiri Denemark wrote:
When a CPU to be compared with host CPU describes a host CPU instead of a guest CPU, the result is incorrect. This is because instead of treating additional features in host CPU description as required, they were treated as if they were mentioned with all possible policies at the same time.
Hi Dan,
I do not compare CPUs for fun, but rather to know if a guest can be run on a specific host. However, this is not exactly what virConnectCompareCPU gives me: A vmx-enabled host cpu is a superset of itself, but it would not run itself as a guest since we do not have nested vxm yet. Similarly, if we ever have svm-emulation-by-kvm, we could be running svm guests on a vmx host.
Is it only me? Or should libvirt expose the more interesting meaning?
Absolutely, that would be just perfect. However, I believe libvirt does the best it can. Only hypervisor itself knows that it can emulate some features which are not present in host CPU or that some features cannot work in guests even though host CPU provides them. For such advanced comparison, qemu would need to provide a way for libvirt to ask for this stuff. Jirka
On Tue, Jul 13, 2010 at 12:03:43PM +0200, Jiri Denemark wrote:
Hi Dan,
I do not compare CPUs for fun, but rather to know if a guest can be run on a specific host. However, this is not exactly what virConnectCompareCPU gives me: A vmx-enabled host cpu is a superset of itself, but it would not run itself as a guest since we do not have nested vxm yet. Similarly, if we ever have svm-emulation-by-kvm, we could be running svm guests on a vmx host.
Is it only me? Or should libvirt expose the more interesting meaning?
Absolutely, that would be just perfect. However, I believe libvirt does the best it can. Only hypervisor itself knows that it can emulate some features which are not present in host CPU or that some features cannot work in guests even though host CPU provides them. For such advanced comparison, qemu would need to provide a way for libvirt to ask for this stuff.
Yep, we would need a different API for this, because we would need to be able to pass in more data than we curently do. eg the machine type, the emulator binary path, virtualization type (kvm/qemu/xen) etc. It might be easiest to just have an API that takes the full domain XML instead of just the CPU XML snipoet Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Tue, Jul 13, 2010 at 11:14:13AM +0100, Daniel P. Berrange wrote:
On Tue, Jul 13, 2010 at 12:03:43PM +0200, Jiri Denemark wrote:
Hi Dan,
I do not compare CPUs for fun, but rather to know if a guest can be run on a specific host. However, this is not exactly what virConnectCompareCPU gives me: A vmx-enabled host cpu is a superset of itself, but it would not run itself as a guest since we do not have nested vxm yet. Similarly, if we ever have svm-emulation-by-kvm, we could be running svm guests on a vmx host.
Is it only me? Or should libvirt expose the more interesting meaning?
Absolutely, that would be just perfect. However, I believe libvirt does the best it can. Only hypervisor itself knows that it can emulate some features which are not present in host CPU or that some features cannot work in guests even though host CPU provides them. For such advanced comparison, qemu would need to provide a way for libvirt to ask for this stuff.
Yep, we would need a different API for this, because we would need to be able to pass in more data than we curently do. eg the machine type, the emulator binary path, virtualization type (kvm/qemu/xen) etc. It might be easiest to just have an API that takes the full domain XML instead of just the CPU XML snipoet
Would it make sense to implement this by a VIR_DOMAIN_CHECK_COMPATIBILITY flag to virDomainCreateXML?
participants (4)
-
Dan Kenigsberg -
Daniel P. Berrange -
Daniel Veillard -
Jiri Denemark