On Tue, Jul 23, 2013 at 05:19:03PM +0100, Daniel P. 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.
The specific information libvirt requires here depend on KVM being
initialized, and QEMU code/interfaces currently assume that: 1) there's
only 1 machine being initialized, and it is initialized very early; 2)
there's only one accelerator being initialized, and it is initialized
very early.
I have no idea how long it will take for QEMU to provide a QMP interface
for late/multiple initialization of machines/accelerators. In the
meantime, the way libvirt queries for host CPU capabilities without
taking QEMU and kernel capabilities into account is a serious bug we
need to solve.
Maybe if we are lucky we can find a workaround in the meantime that
won't require running QEMU multiple times, but I am not sure. Maybe it
will be a hack that will be worse than simply running QEMU twice.
--
Eduardo
