[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Do not fail virConnectCompareCPU if host CPU is not known



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]