[libvirt] [PATCH 6/7] qemu: Probe QEMU binary for host CPU

[Jiri Denemark]

On Tue, Jul 23, 2013 at 17:19:03 +0100, Daniel Berrange wrote:
> On Tue, Jul 23, 2013 at 06:11:35PM +0200, Jiri Denemark wrote:
> > Since QEMU and kvm may filter some host CPU features or add efficiently
> > emulated features, asking QEMU binary for host CPU data provides
> > better results when we later use the data for building guest CPUs.
> > ---
> >  src/qemu/qemu_capabilities.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
> >  src/qemu/qemu_capabilities.h |  2 ++
> >  2 files changed, 45 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 9440396..d46a059 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -253,6 +253,7 @@ struct _virQEMUCaps {
> >  
> >      size_t ncpuDefinitions;
> >      char **cpuDefinitions;
> > +    virCPUDefPtr hostCPU;
> >  
> >      size_t nmachineTypes;
> >      char **machineTypes;
> > @@ -1757,6 +1758,9 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
> >              goto error;
> >      }
> >  
> > +    if (!(ret->hostCPU = virCPUDefCopy(qemuCaps->hostCPU)))
> > +        goto error;
> > +
> >      if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0)
> >          goto error;
> >      if (VIR_ALLOC_N(ret->machineAliases, qemuCaps->nmachineTypes) < 0)
> > @@ -1796,6 +1800,7 @@ void virQEMUCapsDispose(void *obj)
> >          VIR_FREE(qemuCaps->cpuDefinitions[i]);
> >      }
> >      VIR_FREE(qemuCaps->cpuDefinitions);
> > +    virCPUDefFree(qemuCaps->hostCPU);
> >  
> >      virBitmapFree(qemuCaps->flags);
> >  
> > @@ -2485,7 +2490,6 @@ virQEMUCapsInitQMPCommandNew(const char *binary,
> >                                 "-no-user-config",
> >                                 "-nodefaults",
> >                                 "-nographic",
> > -                               "-M", "none",
> >                                 "-qmp", monitor,
> >                                 "-pidfile", pidfile,
> >                                 "-daemonize",
> > @@ -2617,6 +2621,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
> >  
> >      cmd = virQEMUCapsInitQMPCommandNew(qemuCaps->binary, monarg, pidfile,
> >                                         runUid, runGid);
> > +    virCommandAddArgList(cmd, "-M", "none", NULL);
> >  
> >      if ((ret = virQEMUCapsInitQMPCommandRun(cmd, qemuCaps->binary, pidfile,
> >                                              &config, &mon, &pid)) < 0) {
> > @@ -2679,6 +2684,37 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
> >      if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon) < 0)
> >          goto cleanup;
> >  
> > +    if ((qemuCaps->arch == VIR_ARCH_I686 ||
> > +         qemuCaps->arch == VIR_ARCH_X86_64) &&
> > +        (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) ||
> > +         virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KVM)) &&
> > +        virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_HOST) &&
> > +        qemuCaps->nmachineTypes) {
> > +        virQEMUCapsInitQMPCommandAbort(&cmd, &mon, &pid, pidfile);
> > +
> > +        VIR_DEBUG("Checking host CPU data provided by %s", qemuCaps->binary);
> > +        cmd = virQEMUCapsInitQMPCommandNew(qemuCaps->binary, monarg, pidfile,
> > +                                           runUid, runGid);
> > +        virCommandAddArgList(cmd, "-cpu", "host", NULL);
> > +        /* -cpu host gives the same CPU for all machine types so we just
> > +         * use the first one when probing
> > +         */
> > +        virCommandAddArg(cmd, "-machine");
> > +        virCommandAddArgFormat(cmd, "%s,accel=kvm",
> > +                               qemuCaps->machineTypes[0]);
> > +
> > +        if (virQEMUCapsInitQMPCommandRun(cmd, qemuCaps->binary, pidfile,
> > +                                         &config, &mon, &pid) < 0)
> > +            goto cleanup;
> > +
> > +        qemuCaps->hostCPU = qemuMonitorGetCPU(mon, qemuCaps->arch);
> > +        if (qemuCaps->hostCPU) {
> > +            char *cpu = virCPUDefFormat(qemuCaps->hostCPU, 0);
> > +            VIR_DEBUG("Host CPU reported by %s: %s", qemuCaps->binary, cpu);
> > +            VIR_FREE(cpu);
> > +        }
> > +    }
> 
> 
> This code is causing us to invoke the QEMU binary multiple times,
> which is something we worked really hard to get away from. I really,
> really don't like this as an approach. QEMU needs to be able to
> give us the data we need here without multiple invocations.
> 
> eg, by allowing the monitor command to specify 'kvm' vs 'qemu'
> when asking for data, so you can interrogate it without having
> to re-launch it with different accel=XXX args.

Yeah, it's not an ideal solution but it's all we can get now. For giving
us host CPU specification QEMU needs to initialize a complete machine,
that is, it needs to be started with machine type != none, and it needs
to be started with -cpu host, which cannot be used in tcg, only kvm
supports "host" CPU. So the reasons for us invoking QEMU twice are very
deep inside QEMU architecture. Moreover, we only run QEMU binary twice
for i686 and x86_64 archs and when the binary supports kvm, although
this probably covers most real-life cases :/ In any case, it's all
behind our capabilities cache so this should only happen once in libvirt
life-time.

Jirka