Re: [Qemu-devel] [libvirt] [PATCH 6/7] qemu: Probe QEMU binary for host CPU

Re: [Qemu-devel] [libvirt] [PATCH 6/7] qemu: Probe QEMU binary for host CPU

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]

[adding qemu]

On 07/23/2013 10:19 AM, 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.
>> ---

>> +        virCommandAddArg(cmd, "-machine");
>> +        virCommandAddArgFormat(cmd, "%s,accel=kvm",
>> +                               qemuCaps->machineTypes[0]);
>> +

> 
> 
> 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.

We don't know if -machine accel=kvm is a valid option until after
issuing some QMP commands, but now we are forced to reinvoke a new qemu
with -machine accel=kvm enabled in order to get the query to give us
accurate answers.  I agree that this is less than desirable; hopefully
the qemu folks can help us figure out a solution, now that we are
bringing attention to the issue.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [libvirt] [PATCH 6/7] qemu: Probe QEMU binary for host CPU

[Eduardo Habkost]

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

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Eduardo Habkost]

On Tue, Jul 23, 2013 at 07:32:46PM +0200, Jiri Denemark wrote:
> On Tue, Jul 23, 2013 at 19:28:38 +0200, Jiri Denemark wrote:
> > On Tue, Jul 23, 2013 at 17:32:42 +0100, Daniel Berrange wrote:
> > > On Tue, Jul 23, 2013 at 06:11:33PM +0200, Jiri Denemark wrote:
> > > > ---
> > > >  src/qemu/qemu_monitor.c                            |  21 +++
> > > >  src/qemu/qemu_monitor.h                            |   3 +
> > > >  src/qemu/qemu_monitor_json.c                       | 162 +++++++++++++++++++++
> > > >  src/qemu/qemu_monitor_json.h                       |   6 +
> > > >  tests/Makefile.am                                  |   1 +
> > > >  .../qemumonitorjson-getcpu-empty.data              |   2 +
> > > >  .../qemumonitorjson-getcpu-empty.json              |  46 ++++++
> > > >  .../qemumonitorjson-getcpu-filtered.data           |   4 +
> > > >  .../qemumonitorjson-getcpu-filtered.json           |  46 ++++++
> > > >  .../qemumonitorjson-getcpu-full.data               |   4 +
> > > >  .../qemumonitorjson-getcpu-full.json               |  46 ++++++
> > > >  .../qemumonitorjson-getcpu-host.data               |   5 +
> > > >  .../qemumonitorjson-getcpu-host.json               |  45 ++++++
> > > >  tests/qemumonitorjsontest.c                        |  74 ++++++++++
> > > >  14 files changed, 465 insertions(+)
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json
> > > 
> > > ACK, though I believe the design of this monitor API is flawed
> > > because it requires you to re-launch QEMU with different accel
> > > args
> > 
> > Not really, this can be used in tcg too. It's just when we want to get
> > the data for "host" CPU, we need to enable kvm as tcg knows nothing
> > about that CPU. Which makes sense as kvm (the kernel module) influences
> > how the "host" CPU will look like.
> 
> However, you need to have a CPU to be able to ask for his properties
> (which kinda makes sense too) and for that you also need a machine with
> type != none. Which makes sense too, as the CPU may differ depending on
> machine type (which, however, does not happen for "host" CPU).

In addition to the "-cpu host" KVM initialization problem, this is an
additional problem with the current interfaces provided by QEMU:

1) libvirt needs to query data that depend on chosen machine-type and
   CPU model
2) Some machine-type behavior is code and not introspectable data
   * Luckily most of the data we need in this case should/will be
     encoded in the compat_props tables.
   * In either case, we don't have an API to query for machine-type
     compat_props information yet.
3) CPU model behavior will be modelled as CPU class behavior. Like
   on the machine-type case, some of the CPU-model-specific behavior may
   be modelled as code, and not introspectable data.
   * However, e may be able to eventually encode most or all of
     CPU-model-specific behavior simply as different per-CPU-class
     property defaults.
   * In either case, we don't have an API for QOM class introspection,
     yet.

But there's something important in this case: the resulting CPUID data
for a specific machine-type + CPU-model combination must be always the
same, forever. This means libvirt may even use a static table, or cache
this information indefinitely.

(Note that I am not talking about "-cpu host", here, but about all the
other CPU models)

-- 
Eduardo

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Eduardo Habkost]

On Wed, Jul 24, 2013 at 11:03:03AM +0100, Daniel P. Berrange wrote:
> On Tue, Jul 23, 2013 at 07:28:38PM +0200, Jiri Denemark wrote:
> > On Tue, Jul 23, 2013 at 17:32:42 +0100, Daniel Berrange wrote:
> > > On Tue, Jul 23, 2013 at 06:11:33PM +0200, Jiri Denemark wrote:
> > > > ---
> > > >  src/qemu/qemu_monitor.c                            |  21 +++
> > > >  src/qemu/qemu_monitor.h                            |   3 +
> > > >  src/qemu/qemu_monitor_json.c                       | 162 +++++++++++++++++++++
> > > >  src/qemu/qemu_monitor_json.h                       |   6 +
> > > >  tests/Makefile.am                                  |   1 +
> > > >  .../qemumonitorjson-getcpu-empty.data              |   2 +
> > > >  .../qemumonitorjson-getcpu-empty.json              |  46 ++++++
> > > >  .../qemumonitorjson-getcpu-filtered.data           |   4 +
> > > >  .../qemumonitorjson-getcpu-filtered.json           |  46 ++++++
> > > >  .../qemumonitorjson-getcpu-full.data               |   4 +
> > > >  .../qemumonitorjson-getcpu-full.json               |  46 ++++++
> > > >  .../qemumonitorjson-getcpu-host.data               |   5 +
> > > >  .../qemumonitorjson-getcpu-host.json               |  45 ++++++
> > > >  tests/qemumonitorjsontest.c                        |  74 ++++++++++
> > > >  14 files changed, 465 insertions(+)
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data
> > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json
> > > 
> > > ACK, though I believe the design of this monitor API is flawed
> > > because it requires you to re-launch QEMU with different accel
> > > args
> > 
> > Not really, this can be used in tcg too. It's just when we want to get
> > the data for "host" CPU, we need to enable kvm as tcg knows nothing
> > about that CPU. Which makes sense as kvm (the kernel module) influences
> > how the "host" CPU will look like.
> 
> Is there no ioctl() for the KVM module we can just invoke directly to
> discover what CPU flag filtering it will perform. Presumably QEMU is
> using some ioctl to discover this, so libvirt ought to be able to
> too.
> 

Yes, there is: GET_SUPPORTED_CPUID. But availability of some features
may depend on QEMU capabilities as well. On those cases libvirt may need
to combine the GET_SUPPORTED_CPUID results with what it knows about QEMU
capabilities. But this should work as long as we report and document
QEMU capabilities/options that affect CPU features very clearly.

That may be an appropriate way to go to, if you don't mind having
low-level KVM ioctl() code inside libvirt, that duplicates some QEMU
logic.

(But we still have the problem of querying/reporting CPU feature details
that depend on machine-type+CPU-model [that is not addressed by this
series yet]. See my previous message about it)

-- 
Eduardo

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Andreas Färber]

Am 24.07.2013 20:25, schrieb Eduardo Habkost:
> In addition to the "-cpu host" KVM initialization problem, this is an
> additional problem with the current interfaces provided by QEMU:
> 
> 1) libvirt needs to query data that depend on chosen machine-type and
>    CPU model
> 2) Some machine-type behavior is code and not introspectable data
>    * Luckily most of the data we need in this case should/will be
>      encoded in the compat_props tables.
>    * In either case, we don't have an API to query for machine-type
>      compat_props information yet.

... and I don't think we should add such a thing. It is an internal
implementation detail, whose results should be inspected instead.

> 3) CPU model behavior will be modelled as CPU class behavior. Like
>    on the machine-type case, some of the CPU-model-specific behavior may
>    be modelled as code, and not introspectable data.
>    * However, e may be able to eventually encode most or all of
>      CPU-model-specific behavior simply as different per-CPU-class
>      property defaults.
>    * In either case, we don't have an API for QOM class introspection,
>      yet.

If I understood Anthony correctly on the previous call then that is by
design. We have a query-cpu-definitions QMP API to obtain CPU models
though. And qom-list-types to discover QOM types. The CPU can then be
instantiated via -cpu (the type via -device/-object on other targets),
inspected with qom-list/qom-get API and modified with qom-set.

The problem with the latter is that devices/CPUs get realized in code
rather than shortly before emulation/virtualization starts - that's what
my recursive QOM realization series prepared to address, but Paolo
veto'ed that and hasn't provided sufficient feedback on what exactly his
concerns are founded on and what he proposes instead. In particular:
Would walking the qdev bus tree instead of the QOM composition tree
address the concerns?

Depending on what libvirt is actually trying to do, the above combined
with Igor's feature properties and -S might do the job. For x86 the CPUs
are easily locatable in the QOM tree via ICC.

Regards,
Andreas

> But there's something important in this case: the resulting CPUID data
> for a specific machine-type + CPU-model combination must be always the
> same, forever. This means libvirt may even use a static table, or cache
> this information indefinitely.
> 
> (Note that I am not talking about "-cpu host", here, but about all the
> other CPU models)

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Daniel P. Berrange]

On Wed, Jul 24, 2013 at 03:25:19PM -0300, Eduardo Habkost wrote:
> On Tue, Jul 23, 2013 at 07:32:46PM +0200, Jiri Denemark wrote:
> > On Tue, Jul 23, 2013 at 19:28:38 +0200, Jiri Denemark wrote:
> > > On Tue, Jul 23, 2013 at 17:32:42 +0100, Daniel Berrange wrote:
> > > > On Tue, Jul 23, 2013 at 06:11:33PM +0200, Jiri Denemark wrote:
> > > > > ---
> > > > >  src/qemu/qemu_monitor.c                            |  21 +++
> > > > >  src/qemu/qemu_monitor.h                            |   3 +
> > > > >  src/qemu/qemu_monitor_json.c                       | 162 +++++++++++++++++++++
> > > > >  src/qemu/qemu_monitor_json.h                       |   6 +
> > > > >  tests/Makefile.am                                  |   1 +
> > > > >  .../qemumonitorjson-getcpu-empty.data              |   2 +
> > > > >  .../qemumonitorjson-getcpu-empty.json              |  46 ++++++
> > > > >  .../qemumonitorjson-getcpu-filtered.data           |   4 +
> > > > >  .../qemumonitorjson-getcpu-filtered.json           |  46 ++++++
> > > > >  .../qemumonitorjson-getcpu-full.data               |   4 +
> > > > >  .../qemumonitorjson-getcpu-full.json               |  46 ++++++
> > > > >  .../qemumonitorjson-getcpu-host.data               |   5 +
> > > > >  .../qemumonitorjson-getcpu-host.json               |  45 ++++++
> > > > >  tests/qemumonitorjsontest.c                        |  74 ++++++++++
> > > > >  14 files changed, 465 insertions(+)
> > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data
> > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json
> > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data
> > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json
> > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data
> > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json
> > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data
> > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json
> > > > 
> > > > ACK, though I believe the design of this monitor API is flawed
> > > > because it requires you to re-launch QEMU with different accel
> > > > args
> > > 
> > > Not really, this can be used in tcg too. It's just when we want to get
> > > the data for "host" CPU, we need to enable kvm as tcg knows nothing
> > > about that CPU. Which makes sense as kvm (the kernel module) influences
> > > how the "host" CPU will look like.
> > 
> > However, you need to have a CPU to be able to ask for his properties
> > (which kinda makes sense too) and for that you also need a machine with
> > type != none. Which makes sense too, as the CPU may differ depending on
> > machine type (which, however, does not happen for "host" CPU).
> 
> In addition to the "-cpu host" KVM initialization problem, this is an
> additional problem with the current interfaces provided by QEMU:
> 
> 1) libvirt needs to query data that depend on chosen machine-type and
>    CPU model
> 2) Some machine-type behavior is code and not introspectable data
>    * Luckily most of the data we need in this case should/will be
>      encoded in the compat_props tables.
>    * In either case, we don't have an API to query for machine-type
>      compat_props information yet.
> 3) CPU model behavior will be modelled as CPU class behavior. Like
>    on the machine-type case, some of the CPU-model-specific behavior may
>    be modelled as code, and not introspectable data.
>    * However, e may be able to eventually encode most or all of
>      CPU-model-specific behavior simply as different per-CPU-class
>      property defaults.
>    * In either case, we don't have an API for QOM class introspection,
>      yet.
> 
> But there's something important in this case: the resulting CPUID data
> for a specific machine-type + CPU-model combination must be always the
> same, forever. This means libvirt may even use a static table, or cache
> this information indefinitely.
> 
> (Note that I am not talking about "-cpu host", here, but about all the
> other CPU models)

Hmm, so if the CPU filtering can vary per every single individual
machine type, then the approach Jiri started here, of invoking QEMU
with machine type set to query the CPU after it was created, is
definitely not something we can follow. It is just far too inefficient.

I understand that the QEMU code isn't currently structured in a way
that lets it easily expose information that varies per machine type,
but I don't think we need to solve the entire problem space in a
perfectly generic fashion here. Perfect is the enemy of good.

If we can get all the CPU feature flag filtering information to be
in statically defined data structures, then it seems that it would
be pretty straightforward to add a monitor API that takes a CPU
model name and machine type name, and returns the list of feature
flags, without actually having to initialize the machine type or
CPU. It can even just open /dev/kvm & issue the neccessary ioctl,
without having to initialize the entire KVM CPU subsystem in QEMU.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Eduardo Habkost]

On Thu, Jul 25, 2013 at 10:45:10AM +0100, Daniel P. Berrange wrote:
> On Wed, Jul 24, 2013 at 03:25:19PM -0300, Eduardo Habkost wrote:
> > On Tue, Jul 23, 2013 at 07:32:46PM +0200, Jiri Denemark wrote:
> > > On Tue, Jul 23, 2013 at 19:28:38 +0200, Jiri Denemark wrote:
> > > > On Tue, Jul 23, 2013 at 17:32:42 +0100, Daniel Berrange wrote:
> > > > > On Tue, Jul 23, 2013 at 06:11:33PM +0200, Jiri Denemark wrote:
> > > > > > ---
> > > > > >  src/qemu/qemu_monitor.c                            |  21 +++
> > > > > >  src/qemu/qemu_monitor.h                            |   3 +
> > > > > >  src/qemu/qemu_monitor_json.c                       | 162 +++++++++++++++++++++
> > > > > >  src/qemu/qemu_monitor_json.h                       |   6 +
> > > > > >  tests/Makefile.am                                  |   1 +
> > > > > >  .../qemumonitorjson-getcpu-empty.data              |   2 +
> > > > > >  .../qemumonitorjson-getcpu-empty.json              |  46 ++++++
> > > > > >  .../qemumonitorjson-getcpu-filtered.data           |   4 +
> > > > > >  .../qemumonitorjson-getcpu-filtered.json           |  46 ++++++
> > > > > >  .../qemumonitorjson-getcpu-full.data               |   4 +
> > > > > >  .../qemumonitorjson-getcpu-full.json               |  46 ++++++
> > > > > >  .../qemumonitorjson-getcpu-host.data               |   5 +
> > > > > >  .../qemumonitorjson-getcpu-host.json               |  45 ++++++
> > > > > >  tests/qemumonitorjsontest.c                        |  74 ++++++++++
> > > > > >  14 files changed, 465 insertions(+)
> > > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data
> > > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json
> > > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data
> > > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json
> > > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data
> > > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json
> > > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data
> > > > > >  create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json
> > > > > 
> > > > > ACK, though I believe the design of this monitor API is flawed
> > > > > because it requires you to re-launch QEMU with different accel
> > > > > args
> > > > 
> > > > Not really, this can be used in tcg too. It's just when we want to get
> > > > the data for "host" CPU, we need to enable kvm as tcg knows nothing
> > > > about that CPU. Which makes sense as kvm (the kernel module) influences
> > > > how the "host" CPU will look like.
> > > 
> > > However, you need to have a CPU to be able to ask for his properties
> > > (which kinda makes sense too) and for that you also need a machine with
> > > type != none. Which makes sense too, as the CPU may differ depending on
> > > machine type (which, however, does not happen for "host" CPU).
> > 
> > In addition to the "-cpu host" KVM initialization problem, this is an
> > additional problem with the current interfaces provided by QEMU:
> > 
> > 1) libvirt needs to query data that depend on chosen machine-type and
> >    CPU model
> > 2) Some machine-type behavior is code and not introspectable data
> >    * Luckily most of the data we need in this case should/will be
> >      encoded in the compat_props tables.
> >    * In either case, we don't have an API to query for machine-type
> >      compat_props information yet.
> > 3) CPU model behavior will be modelled as CPU class behavior. Like
> >    on the machine-type case, some of the CPU-model-specific behavior may
> >    be modelled as code, and not introspectable data.
> >    * However, e may be able to eventually encode most or all of
> >      CPU-model-specific behavior simply as different per-CPU-class
> >      property defaults.
> >    * In either case, we don't have an API for QOM class introspection,
> >      yet.
> > 
> > But there's something important in this case: the resulting CPUID data
> > for a specific machine-type + CPU-model combination must be always the
> > same, forever. This means libvirt may even use a static table, or cache
> > this information indefinitely.
> > 
> > (Note that I am not talking about "-cpu host", here, but about all the
> > other CPU models)
> 
> Hmm, so if the CPU filtering can vary per every single individual
> machine type, then the approach Jiri started here, of invoking QEMU
> with machine type set to query the CPU after it was created, is
> definitely not something we can follow. It is just far too inefficient.

I believe there's some confusion here: we are trying to solve two
problems:

1) CPU feature filtering (checking which features are available in a
   given host)
2) CPU model probing (checking what exactly is going to be available
   when a given CPU model is used, in case nothing is filtered out)


Item (1) depends on: host CPU capabilities, host kernel capabilities,
QEMU capabilities, presence of some few QEMU command-line options (e.g.
kernel irqchip), but shouldn't depend on the machine-type. It depends on
/dev/kvm being open.

Item (2) depends on the machine-type, but is static and must never
change on future QEMU versions (if it changes, it is a QEMU bug). It
doesn't depend on opening /dev/kvm.


Item (1) can be solved if libvirt does the work itself, by opening
/dev/kvm and checking for GET_SUPPORTED_CPUID and checking for QEMU
options/capabilities (as long as we document that very carefully). But
adding a more specific QMP command that won't require accel=kvm to work
may be simpler and better for everybody.

Item (2) may be solved today using a static table and/or caching (so
libvirt just need to query this information once in a lifetime). It can
also be solved partially (without machine-type support) in theory if
QEMU let libvirt repeatedly create and destroy CPU objects just to query
the resulting feature properties.

...but both problems could be solved very easily using current QEMU
interfaces, if libvirt simply executed the QEMU binary more than once.
Is "must not run QEMU more than once" a hard requirement? Perfect is the
enemy of good.  :)

> 
> I understand that the QEMU code isn't currently structured in a way
> that lets it easily expose information that varies per machine type,
> but I don't think we need to solve the entire problem space in a
> perfectly generic fashion here. Perfect is the enemy of good.

Right. Also, the more important item (item 1) is not affected by
machine-types. Host features change every time you run on a new
host/kernel, so probing it precisely is very useful, to detect problems
earlier (not just at the last moment before starting a VM).

On the other hand, per-machine-type CPU model changes are more rare, and
libvirt can still detect unexpected results immediately before the VM is
started. (I don't know what libvirt would do in case it detects it,
though. Abort? Log a warning?)

> 
> If we can get all the CPU feature flag filtering information to be
> in statically defined data structures, then it seems that it would
> be pretty straightforward to add a monitor API that takes a CPU
> model name and machine type name, and returns the list of feature
> flags, without actually having to initialize the machine type or
> CPU. It can even just open /dev/kvm & issue the neccessary ioctl,
> without having to initialize the entire KVM CPU subsystem in QEMU.

The "without actually having to initialize the machine" part may be
complicated, but it may be doable. But depending on the direction QEMU
machine-types design is going (I don't know if there are plans to
eventually make them more QOM-friendly), the solution accepted by QEMU
may be different.

I will suggest this as a topic for the next KVM call. Are you interested
in joining the call?

-- 
Eduardo

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Eduardo Habkost]

On Thu, Jul 25, 2013 at 11:14:16AM +0200, Andreas Färber wrote:
> Am 24.07.2013 20:25, schrieb Eduardo Habkost:
> > In addition to the "-cpu host" KVM initialization problem, this is an
> > additional problem with the current interfaces provided by QEMU:
> > 
> > 1) libvirt needs to query data that depend on chosen machine-type and
> >    CPU model
> > 2) Some machine-type behavior is code and not introspectable data
> >    * Luckily most of the data we need in this case should/will be
> >      encoded in the compat_props tables.
> >    * In either case, we don't have an API to query for machine-type
> >      compat_props information yet.
> 
> ... and I don't think we should add such a thing. It is an internal
> implementation detail, whose results should be inspected instead.
> 
> > 3) CPU model behavior will be modelled as CPU class behavior. Like
> >    on the machine-type case, some of the CPU-model-specific behavior may
> >    be modelled as code, and not introspectable data.
> >    * However, e may be able to eventually encode most or all of
> >      CPU-model-specific behavior simply as different per-CPU-class
> >      property defaults.
> >    * In either case, we don't have an API for QOM class introspection,
> >      yet.
> 
> If I understood Anthony correctly on the previous call then that is by
> design. We have a query-cpu-definitions QMP API to obtain CPU models
> though. And qom-list-types to discover QOM types. The CPU can then be
> instantiated via -cpu (the type via -device/-object on other targets),
> inspected with qom-list/qom-get API and modified with qom-set.
> 
> The problem with the latter is that devices/CPUs get realized in code
> rather than shortly before emulation/virtualization starts - that's what
> my recursive QOM realization series prepared to address, but Paolo
> veto'ed that and hasn't provided sufficient feedback on what exactly his
> concerns are founded on and what he proposes instead. In particular:
> Would walking the qdev bus tree instead of the QOM composition tree
> address the concerns?
> 
> Depending on what libvirt is actually trying to do, the above combined
> with Igor's feature properties and -S might do the job. For x86 the CPUs
> are easily locatable in the QOM tree via ICC.

I still don't see how the above would solve the bigger problem: libvirt
needs a way to find out how exactly "-machine foo-1.0 -cpu bar" looks
different from "-machine foo-1.1 -cpu bar", but don't want to execute
QEMU multiple times just to find that out.

-- 
Eduardo

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Andreas Färber]

Am 25.07.2013 16:00, schrieb Eduardo Habkost:
> libvirt
> needs a way to find out how exactly "-machine foo-1.0 -cpu bar" looks
> different from "-machine foo-1.1 -cpu bar",

Why? (What's the actual use case?)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Daniel P. Berrange]

On Thu, Jul 25, 2013 at 04:09:18PM +0200, Andreas Färber wrote:
> Am 25.07.2013 16:00, schrieb Eduardo Habkost:
> > libvirt
> > needs a way to find out how exactly "-machine foo-1.0 -cpu bar" looks
> > different from "-machine foo-1.1 -cpu bar",
> 
> Why? (What's the actual use case?)

It already takes a long time to just probe each QEMU binary, without
expanding that to probe each binary once for each machine type they
include. The idea of 'qemu -M none' was that we'd have a machine type
which did not do any hardware set, to query any aspect of the QEMU
binary capabilities.  If we have to also invoke qemu again with every
other machine type that is a failed design IMHO. It should not be beyond
the realm of possibility to make 'qemu -M none' provide information
about the CPU features for machines/cpus without needing to actually
creating instances of those machines.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Daniel P. Berrange]

On Thu, Jul 25, 2013 at 10:15:56AM -0300, Eduardo Habkost wrote:
> On Thu, Jul 25, 2013 at 10:45:10AM +0100, Daniel P. Berrange wrote:
> > On Wed, Jul 24, 2013 at 03:25:19PM -0300, Eduardo Habkost wrote:
> > > In addition to the "-cpu host" KVM initialization problem, this is an
> > > additional problem with the current interfaces provided by QEMU:
> > > 
> > > 1) libvirt needs to query data that depend on chosen machine-type and
> > >    CPU model
> > > 2) Some machine-type behavior is code and not introspectable data
> > >    * Luckily most of the data we need in this case should/will be
> > >      encoded in the compat_props tables.
> > >    * In either case, we don't have an API to query for machine-type
> > >      compat_props information yet.
> > > 3) CPU model behavior will be modelled as CPU class behavior. Like
> > >    on the machine-type case, some of the CPU-model-specific behavior may
> > >    be modelled as code, and not introspectable data.
> > >    * However, e may be able to eventually encode most or all of
> > >      CPU-model-specific behavior simply as different per-CPU-class
> > >      property defaults.
> > >    * In either case, we don't have an API for QOM class introspection,
> > >      yet.
> > > 
> > > But there's something important in this case: the resulting CPUID data
> > > for a specific machine-type + CPU-model combination must be always the
> > > same, forever. This means libvirt may even use a static table, or cache
> > > this information indefinitely.
> > > 
> > > (Note that I am not talking about "-cpu host", here, but about all the
> > > other CPU models)
> > 
> > Hmm, so if the CPU filtering can vary per every single individual
> > machine type, then the approach Jiri started here, of invoking QEMU
> > with machine type set to query the CPU after it was created, is
> > definitely not something we can follow. It is just far too inefficient.
> 
> I believe there's some confusion here: we are trying to solve two
> problems:
> 
> 1) CPU feature filtering (checking which features are available in a
>    given host)
> 2) CPU model probing (checking what exactly is going to be available
>    when a given CPU model is used, in case nothing is filtered out)

Yep, what Jiri proposed in the original libvirt thread was just a
solution to 1). In seeing that though, I was concerned about how it
scales up once we have to deal with 2) as well, which I believe is
planned future work.

> Item (1) depends on: host CPU capabilities, host kernel capabilities,
> QEMU capabilities, presence of some few QEMU command-line options (e.g.
> kernel irqchip), but shouldn't depend on the machine-type. It depends on
> /dev/kvm being open.
> 
> Item (2) depends on the machine-type, but is static and must never
> change on future QEMU versions (if it changes, it is a QEMU bug). It
> doesn't depend on opening /dev/kvm.
>
>
> Item (1) can be solved if libvirt does the work itself, by opening
> /dev/kvm and checking for GET_SUPPORTED_CPUID and checking for QEMU
> options/capabilities (as long as we document that very carefully). But
> adding a more specific QMP command that won't require accel=kvm to work
> may be simpler and better for everybody.
> 
> Item (2) may be solved today using a static table and/or caching (so
> libvirt just need to query this information once in a lifetime). It can
> also be solved partially (without machine-type support) in theory if
> QEMU let libvirt repeatedly create and destroy CPU objects just to query
> the resulting feature properties.

We really don't want to have static tables, since that creates pain
in the case where distro vendors create their own custom machine types
or CPU models. It would mean libvirt had to record info not only about
upstream QEMU, but about every vendor's QEMU builds. Probing the actual
binary is the only sensible way here.

> ...but both problems could be solved very easily using current QEMU
> interfaces, if libvirt simply executed the QEMU binary more than once.
> Is "must not run QEMU more than once" a hard requirement? Perfect is the
> enemy of good.  :)

Yes, it is a hard requirement.

> > I understand that the QEMU code isn't currently structured in a way
> > that lets it easily expose information that varies per machine type,
> > but I don't think we need to solve the entire problem space in a
> > perfectly generic fashion here. Perfect is the enemy of good.
> 
> Right. Also, the more important item (item 1) is not affected by
> machine-types. Host features change every time you run on a new
> host/kernel, so probing it precisely is very useful, to detect problems
> earlier (not just at the last moment before starting a VM).
> 
> On the other hand, per-machine-type CPU model changes are more rare, and
> libvirt can still detect unexpected results immediately before the VM is
> started. (I don't know what libvirt would do in case it detects it,
> though. Abort? Log a warning?)

We don't want to be running QEMU multiple times during the startup
process for a VM, because that adds delays to the startup process.
It might not sound like much but adding a few 100ms to probe CPUs
by running QEMU is quite significant for apps like libvirt-sandbox
and libguesfs where absolute boot time is important. We used to run
QEMU at startup to probe things & we just recently got rid of that
delay, so I don't want to re-introduce it against. When starting
a VM, we only once to start QEMU once, as the actual instance that
is going to run the VM.

> > If we can get all the CPU feature flag filtering information to be
> > in statically defined data structures, then it seems that it would
> > be pretty straightforward to add a monitor API that takes a CPU
> > model name and machine type name, and returns the list of feature
> > flags, without actually having to initialize the machine type or
> > CPU. It can even just open /dev/kvm & issue the neccessary ioctl,
> > without having to initialize the entire KVM CPU subsystem in QEMU.
> 
> The "without actually having to initialize the machine" part may be
> complicated, but it may be doable. But depending on the direction QEMU
> machine-types design is going (I don't know if there are plans to
> eventually make them more QOM-friendly), the solution accepted by QEMU
> may be different.
> 
> I will suggest this as a topic for the next KVM call. Are you interested
> in joining the call?

Yes, assuming it doesn't clash with anything else i have scheduled.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Eduardo Habkost]

On Thu, Jul 25, 2013 at 04:09:18PM +0200, Andreas Färber wrote:
> Am 25.07.2013 16:00, schrieb Eduardo Habkost:
> > libvirt
> > needs a way to find out how exactly "-machine foo-1.0 -cpu bar" looks
> > different from "-machine foo-1.1 -cpu bar",
> 
> Why? (What's the actual use case?)

libvirt API allows individual CPU features to be configured, so libvirt
needs to know what exactly will be the result of using a
machine-type/CPU-model combination to make sure it will be exactly what
was requested:
http://libvirt.org/formatdomain.html#elementsCPU

Also, libvirt needs to be able to check if migration to a host is
possible (i.e. if all features enabled by a machine-type/CPU-model
combination are supported by the host) before actually starting the
migration process.

-- 
Eduardo

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Andreas Färber]

Am 25.07.2013 20:02, schrieb Eduardo Habkost:
> On Thu, Jul 25, 2013 at 04:09:18PM +0200, Andreas Färber wrote:
>> Am 25.07.2013 16:00, schrieb Eduardo Habkost:
>>> libvirt
>>> needs a way to find out how exactly "-machine foo-1.0 -cpu bar" looks
>>> different from "-machine foo-1.1 -cpu bar",
>>
>> Why? (What's the actual use case?)
> 
> libvirt API allows individual CPU features to be configured, so libvirt
> needs to know what exactly will be the result of using a
> machine-type/CPU-model combination to make sure it will be exactly what
> was requested:
> http://libvirt.org/formatdomain.html#elementsCPU

That's exactly what you added properties for last minute in v1.5!

libvirt instantiates qemu-system-x86_64 -cpu foo,+x,+y and then checks
that it got what it wanted  - if not, die, otherwise continue with
virtualization. One process.

> Also, libvirt needs to be able to check if migration to a host is
> possible (i.e. if all features enabled by a machine-type/CPU-model
> combination are supported by the host) before actually starting the
> migration process.

That's one process on the destination with one -machine pc-i440-x.y.
Is the problem possibly rather that -incoming and QMP exclude each
other? Then we should fix that instead by starting incoming migration
from QMP in the same process that we used to check that migration will
be possible without guest-visible changes.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Eduardo Habkost]

On Fri, Jul 26, 2013 at 02:31:24PM +0200, Andreas Färber wrote:
> Am 25.07.2013 20:02, schrieb Eduardo Habkost:
> > On Thu, Jul 25, 2013 at 04:09:18PM +0200, Andreas Färber wrote:
> >> Am 25.07.2013 16:00, schrieb Eduardo Habkost:
> >>> libvirt
> >>> needs a way to find out how exactly "-machine foo-1.0 -cpu bar" looks
> >>> different from "-machine foo-1.1 -cpu bar",
> >>
> >> Why? (What's the actual use case?)
> > 
> > libvirt API allows individual CPU features to be configured, so libvirt
> > needs to know what exactly will be the result of using a
> > machine-type/CPU-model combination to make sure it will be exactly what
> > was requested:
> > http://libvirt.org/formatdomain.html#elementsCPU
> 
> That's exactly what you added properties for last minute in v1.5!
> 
> libvirt instantiates qemu-system-x86_64 -cpu foo,+x,+y and then checks
> that it got what it wanted  - if not, die, otherwise continue with
> virtualization. One process.

I believe libvirt needs to know what are the results of the CPU model +
machine-type combination at other moments, even before building the QEMU
command-line. But I may be mistaken, so I hope the libvirt developers
can clarify that.

> 
> > Also, libvirt needs to be able to check if migration to a host is
> > possible (i.e. if all features enabled by a machine-type/CPU-model
> > combination are supported by the host) before actually starting the
> > migration process.
> 
> That's one process on the destination with one -machine pc-i440-x.y.
> Is the problem possibly rather that -incoming and QMP exclude each
> other? Then we should fix that instead by starting incoming migration
> from QMP in the same process that we used to check that migration will
> be possible without guest-visible changes.

I don't know the answer for that, and I also don't know if it is
acceptable for libvirt to be required to execute QEMU just to find out
if migration to a host is possible.

I am thinking of cases where there may be dozens or hundreds of hosts
available, and some management system needs to find out quickly what are
the best candidates to which a large set of VMs can be migrated.

-- 
Eduardo