[Qemu-devel] [PATCHv5 0/5] add query-cpu-fast and related s390 changes

Viktor Mihajlovski posted 5 patches 7 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1518797321-28356-1-git-send-email-mihajlov@linux.vnet.ibm.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppcbe passed
Test ppcle passed
Test s390x passed
cpus.c                     |  54 +++++++++++++++++++++
hmp.c                      |  41 +++-------------
hw/intc/s390_flic.c        |   4 +-
hw/s390x/s390-virtio-ccw.c |   2 +-
monitor.c                  |  13 +++--
qapi-schema.json           | 115 ++++++++++++++++++++++++++++++++++++++++++++-
qemu-doc.texi              |   4 ++
target/s390x/cpu.c         |  24 +++++-----
target/s390x/cpu.h         |   7 +--
target/s390x/kvm.c         |   8 ++--
target/s390x/sigp.c        |  38 +++++++--------
11 files changed, 228 insertions(+), 82 deletions(-)
[Qemu-devel] [PATCHv5 0/5] add query-cpu-fast and related s390 changes
Posted by Viktor Mihajlovski 7 years, 8 months ago
v5 synopsis: Split out HMP changes from Patch 2 into Patch 5. Please
   	     re-review, as I've removed the a-b/r-b from Patch 2
	     as well.

This series consolidates patches around a performance issue
caused by the usage of QMP query-cpus.

A performance issue was found in an OpenStack environment, where
ceilometer was collecting domain statistics with libvirt. The domain
statistics reported by libvirt include the vCPU halted state, which 
in turn is retrieved with QMP query-cpus.

This causes two issues:
1. Performance: on most architectures query-cpus needs to issue a KVM ioctl
   to find out whether a vCPU was halted. This is not the case for s390
   but query-cpus is always causing the vCPU to exit the VM.

2. Semantics: on x86 and other architectures, halted is a highly transient
   state, which is likely to have already changed shortly after the state
   information has been retrieved. This is not the case for s390, where
   halted is an indication that the vCPU is stopped, meaning its not
   available to the guest operating system until it has been restarted.

The following patches help to alleviate the issues:

Patch 1/5:
  Adds architecture specific data to the QMP CpuInfo type, exposing
  the existing s390 cpu-state in QMP. The cpu-state is a representation
  more adequate than the ambiguous 'halted' condition.

Patch 2/5:
  Adds a new QMP function query-cpus-fast, which will only retrieve
  vCPU information that can be obtained without interrupting the
  vCPUs of a running VM. It introduces a new return type CpuInfoFast
  with the subset of fields meeting this condition. Specifically, the
  halted state is not part of CpuInfoFast. QMP clients like libvirt
  are encouraged to switch to the new API for vCPU information.

Patch 3/5:
  Adds the s390-specific cpu state to CpuInfoFast, allowing management
  apps to find out whether a vCPU is in the stopped state. This extension
  leads to a partial duplication of field definitions from CpuInfo
  to CpuInfoFast. This should be tolerable if CpuInfo is deprecated and
  eventually removed.

Patch 4/5:
  Starts the deprecation of query-cpus.

Patch 5/5 (NEW):
  Changes HMP 'info cpus' to use query-cpus-fast. Was part of 2/5
  initially.

Series v4 -> v5:
Overall: Updated r-b's and moved HMP changes into a seperate patch.

Patch 1/5:
  o Updated commit message to clarify why no HMP change

Patch 2/5:
  o Split out HMP changes
  o Removed r-b cohuck, a-b eblake

Series v3 -> v4:
Overall: Instead of adding a new HMP 'info cpus_fast', changed
	 'info cpus' to use query-cpus-fast directly.

Patch 1/4:
  o Don't report s390-specific data in HMP 'info cpus'

Patch 2/4:
  o Change HMP 'info cpus' to use query-cpus-fast and
    to return only basic information (no arch-specific data)
  o Drop HMP 'info cpus_fast'
  o Fixed typo in commit message

Patch 3/4:
  o Drop HMP-related changes

Patch 4/4:
  o Drop HMP-related changes

Series v2 -> v3:
Overall: Added r-b's and a-b's.

Patch 2/4:
  o Fixed commit message with respect to the halted field
    disposition.
  o Fixed grammar in qapi-schema documentation.

Patch 3/4:
  o Use CpuInfoS390 type for both query-cpus and query-cpus-fast per
    Eric Blake's comment.
  o Dropped 'duplication blurb' from commit message as it doesn't
    provide relevant information other than query-cpus should be
    deprecated, which is done in the next patch now.

Series v1 -> v2:
Patch 2/3:
  o Changed formatting of hmp info cpus_fast to match that of
    info cpus. This makes it easier for clients to switch to
    the fast call.

Patch 3/3:
  o Same formatting change for info cpus_fast as in 2/3, only
    for s390-specific cpu state.

Luiz Capitulino (1):
  qmp: add query-cpus-fast

Viktor Mihajlovski (4):
  qmp: expose s390-specific CPU info
  qmp: add architecture specific cpu data for query-cpus-fast
  qemu-doc: deprecate query-cpus
  hmp: change hmp_info_cpus to use query-cpus-fast

 cpus.c                     |  54 +++++++++++++++++++++
 hmp.c                      |  41 +++-------------
 hw/intc/s390_flic.c        |   4 +-
 hw/s390x/s390-virtio-ccw.c |   2 +-
 monitor.c                  |  13 +++--
 qapi-schema.json           | 115 ++++++++++++++++++++++++++++++++++++++++++++-
 qemu-doc.texi              |   4 ++
 target/s390x/cpu.c         |  24 +++++-----
 target/s390x/cpu.h         |   7 +--
 target/s390x/kvm.c         |   8 ++--
 target/s390x/sigp.c        |  38 +++++++--------
 11 files changed, 228 insertions(+), 82 deletions(-)

-- 
1.9.1


Re: [Qemu-devel] [PATCHv5 0/5] add query-cpu-fast and related s390 changes
Posted by Cornelia Huck 7 years, 8 months ago
On Fri, 16 Feb 2018 17:08:36 +0100
Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> wrote:

> v5 synopsis: Split out HMP changes from Patch 2 into Patch 5. Please
>    	     re-review, as I've removed the a-b/r-b from Patch 2
> 	     as well.

Series looks good to me. I'd be happy to take it through the s390 tree
with some a-bs/r-bs for patches 2 and 5, but have given some r-bs as
well in case someone else wants to take it.

> 
> This series consolidates patches around a performance issue
> caused by the usage of QMP query-cpus.
> 
> A performance issue was found in an OpenStack environment, where
> ceilometer was collecting domain statistics with libvirt. The domain
> statistics reported by libvirt include the vCPU halted state, which 
> in turn is retrieved with QMP query-cpus.
> 
> This causes two issues:
> 1. Performance: on most architectures query-cpus needs to issue a KVM ioctl
>    to find out whether a vCPU was halted. This is not the case for s390
>    but query-cpus is always causing the vCPU to exit the VM.
> 
> 2. Semantics: on x86 and other architectures, halted is a highly transient
>    state, which is likely to have already changed shortly after the state
>    information has been retrieved. This is not the case for s390, where
>    halted is an indication that the vCPU is stopped, meaning its not
>    available to the guest operating system until it has been restarted.
> 
> The following patches help to alleviate the issues:
> 
> Patch 1/5:
>   Adds architecture specific data to the QMP CpuInfo type, exposing
>   the existing s390 cpu-state in QMP. The cpu-state is a representation
>   more adequate than the ambiguous 'halted' condition.
> 
> Patch 2/5:
>   Adds a new QMP function query-cpus-fast, which will only retrieve
>   vCPU information that can be obtained without interrupting the
>   vCPUs of a running VM. It introduces a new return type CpuInfoFast
>   with the subset of fields meeting this condition. Specifically, the
>   halted state is not part of CpuInfoFast. QMP clients like libvirt
>   are encouraged to switch to the new API for vCPU information.
> 
> Patch 3/5:
>   Adds the s390-specific cpu state to CpuInfoFast, allowing management
>   apps to find out whether a vCPU is in the stopped state. This extension
>   leads to a partial duplication of field definitions from CpuInfo
>   to CpuInfoFast. This should be tolerable if CpuInfo is deprecated and
>   eventually removed.
> 
> Patch 4/5:
>   Starts the deprecation of query-cpus.
> 
> Patch 5/5 (NEW):
>   Changes HMP 'info cpus' to use query-cpus-fast. Was part of 2/5
>   initially.
> 
> Series v4 -> v5:
> Overall: Updated r-b's and moved HMP changes into a seperate patch.
> 
> Patch 1/5:
>   o Updated commit message to clarify why no HMP change
> 
> Patch 2/5:
>   o Split out HMP changes
>   o Removed r-b cohuck, a-b eblake
> 
> Series v3 -> v4:
> Overall: Instead of adding a new HMP 'info cpus_fast', changed
> 	 'info cpus' to use query-cpus-fast directly.
> 
> Patch 1/4:
>   o Don't report s390-specific data in HMP 'info cpus'
> 
> Patch 2/4:
>   o Change HMP 'info cpus' to use query-cpus-fast and
>     to return only basic information (no arch-specific data)
>   o Drop HMP 'info cpus_fast'
>   o Fixed typo in commit message
> 
> Patch 3/4:
>   o Drop HMP-related changes
> 
> Patch 4/4:
>   o Drop HMP-related changes
> 
> Series v2 -> v3:
> Overall: Added r-b's and a-b's.
> 
> Patch 2/4:
>   o Fixed commit message with respect to the halted field
>     disposition.
>   o Fixed grammar in qapi-schema documentation.
> 
> Patch 3/4:
>   o Use CpuInfoS390 type for both query-cpus and query-cpus-fast per
>     Eric Blake's comment.
>   o Dropped 'duplication blurb' from commit message as it doesn't
>     provide relevant information other than query-cpus should be
>     deprecated, which is done in the next patch now.
> 
> Series v1 -> v2:
> Patch 2/3:
>   o Changed formatting of hmp info cpus_fast to match that of
>     info cpus. This makes it easier for clients to switch to
>     the fast call.
> 
> Patch 3/3:
>   o Same formatting change for info cpus_fast as in 2/3, only
>     for s390-specific cpu state.
> 
> Luiz Capitulino (1):
>   qmp: add query-cpus-fast
> 
> Viktor Mihajlovski (4):
>   qmp: expose s390-specific CPU info
>   qmp: add architecture specific cpu data for query-cpus-fast
>   qemu-doc: deprecate query-cpus
>   hmp: change hmp_info_cpus to use query-cpus-fast
> 
>  cpus.c                     |  54 +++++++++++++++++++++
>  hmp.c                      |  41 +++-------------
>  hw/intc/s390_flic.c        |   4 +-
>  hw/s390x/s390-virtio-ccw.c |   2 +-
>  monitor.c                  |  13 +++--
>  qapi-schema.json           | 115 ++++++++++++++++++++++++++++++++++++++++++++-
>  qemu-doc.texi              |   4 ++
>  target/s390x/cpu.c         |  24 +++++-----
>  target/s390x/cpu.h         |   7 +--
>  target/s390x/kvm.c         |   8 ++--
>  target/s390x/sigp.c        |  38 +++++++--------
>  11 files changed, 228 insertions(+), 82 deletions(-)
> 


Re: [Qemu-devel] [PATCHv5 0/5] add query-cpu-fast and related s390 changes
Posted by Cornelia Huck 7 years, 8 months ago
On Fri, 16 Feb 2018 17:08:36 +0100
Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> wrote:

> v5 synopsis: Split out HMP changes from Patch 2 into Patch 5. Please
>    	     re-review, as I've removed the a-b/r-b from Patch 2
> 	     as well.
> 
> This series consolidates patches around a performance issue
> caused by the usage of QMP query-cpus.
> 
> A performance issue was found in an OpenStack environment, where
> ceilometer was collecting domain statistics with libvirt. The domain
> statistics reported by libvirt include the vCPU halted state, which 
> in turn is retrieved with QMP query-cpus.
> 
> This causes two issues:
> 1. Performance: on most architectures query-cpus needs to issue a KVM ioctl
>    to find out whether a vCPU was halted. This is not the case for s390
>    but query-cpus is always causing the vCPU to exit the VM.
> 
> 2. Semantics: on x86 and other architectures, halted is a highly transient
>    state, which is likely to have already changed shortly after the state
>    information has been retrieved. This is not the case for s390, where
>    halted is an indication that the vCPU is stopped, meaning its not
>    available to the guest operating system until it has been restarted.

Thanks, queued to s390-next.