Re: [qemu-s390x] [PATCHv2 2/3] qmp: add query-cpus-fast

[Cornelia Huck]

On Tue, 13 Feb 2018 18:18:47 +0100
Viktor Mihajlovski <address@hidden> wrote:

> From: Luiz Capitulino <address@hidden>
> 
> 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"

I'd suggest updating this description, as "halted" is now gone
completely... what about:

o Drop the "halted" field, even if the halted state is maintained by
QEMU. It had unclear semantics anyway.

o Drop irrelevant fields such as "current", "pc", or "arch"

> 
>  o Rename some fields for better clarification & proper naming
>    standard
> 
> Signed-off-by: Luiz Capitulino <address@hidden>
> Signed-off-by: Viktor Mihajlovski <address@hidden>
> ---
>  cpus.c               | 38 ++++++++++++++++++++++++++++
>  hmp-commands-info.hx | 14 +++++++++++
>  hmp.c                | 14 +++++++++++
>  hmp.h                |  1 +
>  qapi-schema.json     | 70 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 137 insertions(+)

Otherwise, looks good to me.

Reviewed-by: Cornelia Huck <address@hidden>