[libvirt] [PATCH] qemu: Do not fail virConnectCompareCPU if host CPU is not known
When host CPU could not be properly detected, virConnectCompareCPU will just report that any CPU is incompatible with host CPU instead of failing. --- src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc04d13..6d3b8d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9419,8 +9419,8 @@ qemuCPUCompare(virConnectPtr conn, qemuDriverLock(driver); if (!driver->caps || !driver->caps->host.cpu) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot get host CPU capabilities")); + VIR_WARN("cannot get host CPU capabilities"); + ret = VIR_CPU_COMPARE_INCOMPATIBLE; } else { ret = cpuCompareXML(driver->caps->host.cpu, xmlDesc); } -- 1.7.11.1
On 07/12/2012 05:06 AM, Jiri Denemark wrote:
When host CPU could not be properly detected, virConnectCompareCPU will just report that any CPU is incompatible with host CPU instead of failing. --- src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc04d13..6d3b8d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9419,8 +9419,8 @@ qemuCPUCompare(virConnectPtr conn, qemuDriverLock(driver);
if (!driver->caps || !driver->caps->host.cpu) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot get host CPU capabilities")); + VIR_WARN("cannot get host CPU capabilities"); + ret = VIR_CPU_COMPARE_INCOMPATIBLE;
Seems simple enough. And not detecting the host CPU should be relatively rare. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Jul 12, 2012 at 01:06:08PM +0200, Jiri Denemark wrote:
When host CPU could not be properly detected, virConnectCompareCPU will just report that any CPU is incompatible with host CPU instead of failing. --- src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc04d13..6d3b8d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9419,8 +9419,8 @@ qemuCPUCompare(virConnectPtr conn, qemuDriverLock(driver);
if (!driver->caps || !driver->caps->host.cpu) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot get host CPU capabilities")); + VIR_WARN("cannot get host CPU capabilities"); + ret = VIR_CPU_COMPARE_INCOMPATIBLE; } else { ret = cpuCompareXML(driver->caps->host.cpu, xmlDesc); }
Jiri, I think that I've changed my own taste about this, too. I don't know what can lead driver->caps to be NULL, but I support that many things that are unrelated to host CPU can. If this is the case, the caller of cpuCompareXML may receive a misleading VIR_CPU_COMPARE_INCOMPATIBLE. Limiting the new ret to the case of driver->caps->host.cpu == NULL would have been better. But again: is there a chance that driver->caps->host.cpu is NULL due to lack of memory during host probing? I'm still wondering why libvirt must detect a known hardware cpu on the host. In the age of nested virt, it may find more bizarre combinations, that it may be interesting to support. Dan.
On Sat, Jul 14, 2012 at 23:31:52 +0300, Dan Kenigsberg wrote:
On Thu, Jul 12, 2012 at 01:06:08PM +0200, Jiri Denemark wrote:
When host CPU could not be properly detected, virConnectCompareCPU will just report that any CPU is incompatible with host CPU instead of failing. --- src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc04d13..6d3b8d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9419,8 +9419,8 @@ qemuCPUCompare(virConnectPtr conn, qemuDriverLock(driver);
if (!driver->caps || !driver->caps->host.cpu) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot get host CPU capabilities")); + VIR_WARN("cannot get host CPU capabilities"); + ret = VIR_CPU_COMPARE_INCOMPATIBLE; } else { ret = cpuCompareXML(driver->caps->host.cpu, xmlDesc); }
Jiri, I think that I've changed my own taste about this, too.
I don't know what can lead driver->caps to be NULL, but I support that many things that are unrelated to host CPU can.
If this is the case, the caller of cpuCompareXML may receive a misleading VIR_CPU_COMPARE_INCOMPATIBLE. Limiting the new ret to the case of driver->caps->host.cpu == NULL would have been better.
Oh, right. And it matches commit message. So I squashed the following hunk in @@ -9423,7 +9423,10 @@ qemuCPUCompare(virConnectPtr conn, qemuDriverLock(driver); - if (!driver->caps || !driver->caps->host.cpu) { + if (!driver->caps) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot get host capabilities")); + } else if (!driver->caps->host.cpu) { VIR_WARN("cannot get host CPU capabilities"); ret = VIR_CPU_COMPARE_INCOMPATIBLE; } else { and pushed based on Eric's previous ACK.
But again: is there a chance that driver->caps->host.cpu is NULL due to lack of memory during host probing?
Not really. If there's a lack of memory when qemu driver probes for host CPU, something else further in the initialization path will most likely fail as well and the qemu driver will completely fail to initialize.
I'm still wondering why libvirt must detect a known hardware cpu on the host. In the age of nested virt, it may find more bizarre combinations, that it may be interesting to support.
Yeah, it may be worth it. Jirka
participants (3)
-
Dan Kenigsberg -
Eric Blake -
Jiri Denemark