On 02/13/2018 11:18 AM, Viktor Mihajlovski wrote:
From: Luiz Capitulino <lcapitul...@redhat.com>

The query-cpus command has an extremely serious side effect:
it always interrupts all running vCPUs so that they can run
ioctl calls. This can cause a huge performance degradation for
some workloads. And most of the information retrieved by the
ioctl calls are not even used by query-cpus.

This commit introduces a replacement for query-cpus called
query-cpus-fast, which has the following features:

  o Never interrupt vCPUs threads. query-cpus-fast only returns
    vCPU information maintained by QEMU itself, which should be
    sufficient for most management software needs

  o Make "halted" field optional: we only return it if the
    halted state is maintained by QEMU. But this also gives
    the option of dropping the field in the future (see below)

  o Drop irrelevant fields such as "current", "pc", "arch"
    and "halted"

Others have pointed out the commit message inconsistencies; I'm focusing on the QMP.

+++ b/qapi-schema.json
@@ -552,6 +552,12 @@
  # Returns a list of information about each virtual CPU.
+# This command causes vCPU threads to exit to userspace, which causes
+# an small interruption guest CPU execution. This will have a negative

s/an small interruption/a small interruption to/

+# impact on realtime guests and other latency sensitive guest workloads.
+# It is recommended to use @query-cpus-fast instead of this command to
+# avoid the vCPU interruption.
  # Returns: a list of @CpuInfo for each virtual CPU
  # Since: 0.14.0
@@ -585,6 +591,70 @@
  { 'command': 'query-cpus', 'returns': ['CpuInfo'] }
+# @CpuInfoFast:
+# Information about a virtual CPU
+# @cpu-index: index of the virtual CPU
+# @qom-path: path to the CPU object in the QOM tree
+# @thread-id: ID of the underlying host thread
+# @props: properties describing to which node/socket/core/thread
+#         virtual CPU belongs to, provided if supported by board
+# Since: 2.12
+{ 'struct': 'CpuInfoFast',
+  'data': {'cpu-index': 'int', 'qom-path': 'str',
+           'thread-id': 'int', '*props': 'CpuInstanceProperties' } }

Comparing against the existing CpuInfo: if I'm not mistaken, you've renamed:

CPU -> cpu-index
qom_path -> qom-path
thread_id -> thread-id

and kept props unchanged.

If we REALLY cared about reducing duplication, we can do:

{ 'struct': 'CpuInfoFast',
  'data': { 'CPU': 'int', 'qom_path': 'str', 'thread_id': 'int',
            '*props': 'CpuInstanceProperties' } }
{ 'struct': 'CpuInfoBase',
  'base': 'CpuInfoFast',
  'data': { 'current': 'bool', 'halted': 'bool',
            'arch': 'CpuInfoArch' } }
{ 'union': 'CpuInfo', 'base': 'CpuInfoBase', 'discriminator': 'arch',
  'data': { ... } }

but I'm not yet convinced whether we need that. If we're going to deprecate the old command, having the new command use modern spelling is a nice change, even though it costs some duplication in the qapi file; where sharing code may save qapi costs in the short run, but will burden us with poor naming down the road even when we delete the slow command.

+# @query-cpus-fast:
+# Returns information about all virtual CPUs. This command does not
+# incur a performance penalty and should be used in production
+# instead of query-cpus.
+# Returns: list of @CpuInfoFast
+# Notes: The CPU architecture name is not returned by query-cpus-fast.
+#        Use query-target to retrieve that information.
+# Since: 2.12

+{ 'command': 'query-cpus-fast', 'returns': [ 'CpuInfoFast' ] }
  # @IOThreadInfo:
  # Information about an iothread

I didn't inspect the code, so similar to 1/3, once you fix the grammar nit and commit message,

Acked-by: Eric Blake <ebl...@redhat.com>

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Reply via email to