[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [REPOST PATCH v2 00/12] Allow modification of IOThread polling values (redux)

v2: https://www.redhat.com/archives/libvir-list/2018-October/msg01065.html

NB: Minor mods for this are change using 4.10.0 instead of 4.9.0, merge of
    qemu_capabilities conflict, and updated news.xml

...... v2 Cover Letter:

v1: https://www.redhat.com/archives/libvir-list/2018-October/msg00456.html

Changes since v1 (from code review)

Patch 2: Move the job back into qemuDomainGetIOThreadsLive

Patch 3: Add check for ActiveJob before allowing, use true for
         *StatsWorker, and print 'iothread' in output not 'block'

Patch 5: Use virCheckPositiveArgGoto(nparams, error) instead of using
         virCheckNonNegativeArgGoto(nparams, error).  And then remove
         the if (nparams) before the virCheckNonNullArgGoto(params, error);

Patch 6: Add ability to determine which parameter was set via bool
         set_poll_{max_ns|grow|shrink} values.  Then use those in
         the macro that sets the value to determine whether or not
         the value will be set based on whether it was changed.

Patch 10: Use bool's to set_ when the value is found in the incoming
          params list.  Remove the check that says poll_max_ns needs
          to be set. Testing proves that if it's set to 0, then the
          grow and shrink values can be changed (although they do

Patch 12: (NEW) - News article

...... v1 Cover Letter:

This series attempts to resurrect the concept of being able to modify
the IOThread polling parameters; however, in a slightly different
manner than the previous series posted by posted by Pavel Hrdina
<phrdina redhat com>:


The work is prompted by continued pleas found in the bz:


to provide some way to modify the paremters without needing to supply
QEMU command line pass through values.

It's accepted that the values being changed are fairly or extremely
low level type knobs; however, it's also shown that by being able to
turn the knob it is possible for certain, specific appliances to be
able to gain a performance benefit for the thread at the expense of
other competing threads.

Unlike the previous series, this series does not attempt to save the
polling values in the guest XML. Rather, it only modifies the live
guest's IOThread with new values. It also doesn't provide the polling
values in a virsh iothread* command, rather it uses the domstats
in order to fetch and display the values. The theory being this
leaves the onus on the higher level appliance/application to provide
the "proper guidance" and "concerns" related to changing the values
to the consumer. Not saving the values means whatever values that
are chosen do not "live" in perpetuity. Once the guest is shut down
or the IOThread removed from guest, the hypervisor default values
take over again. Perhaps not a perfect situation in terms of what
the bz requests; however, storage of default values that could
cause performance issues is not an optimal situation. So this I
figured is a "comprimise" of sorts.

If it's still felt that no we don't want to do this, then fine,
but please in doing so own the bz, state your case, and close it.
I'm 50/50 on it, but figured at least I'd present this option and
see what the concensus was.

John Ferlan (12):
  qemu: Check for and return IOThread polling values if available
  qemu: Split qemuDomainGetIOThreadsLive
  qemu: Implement the ability to return IOThread stats
  virsh: Add ability to display IOThread stats
  lib: Introduce virDomainSetIOThreadParams
  qemu: Add monitor functions to set IOThread params
  qemu: Alter qemuDomainChgIOThread to take enum instead of bool
  qemu: Alter qemuDomainChgIOThread to take qemuMonitorIOThreadInfo
  qemu: Detect whether iothread polling is supported
  qemu: Introduce qemuDomainSetIOThreadParams
  tools: Add virsh iothreadset command
  docs: Add news article for IOThread polling

 docs/news.xml                                 |  13 +
 include/libvirt/libvirt-domain.h              |  45 ++
 src/driver-hypervisor.h                       |   8 +
 src/libvirt-domain.c                          | 108 +++++
 src/libvirt_public.syms                       |   5 +
 src/qemu/qemu_capabilities.c                  |   2 +
 src/qemu/qemu_capabilities.h                  |   1 +
 src/qemu/qemu_driver.c                        | 384 ++++++++++++++++--
 src/qemu/qemu_monitor.c                       |  19 +
 src/qemu/qemu_monitor.h                       |   9 +
 src/qemu/qemu_monitor_json.c                  |  50 +++
 src/qemu/qemu_monitor_json.h                  |   4 +
 src/remote/remote_driver.c                    |   1 +
 src/remote/remote_protocol.x                  |  21 +-
 src/remote_protocol-structs                   |  10 +
 .../caps_2.10.0.aarch64.xml                   |   1 +
 .../caps_2.10.0.ppc64.xml                     |   1 +
 .../caps_2.10.0.s390x.xml                     |   1 +
 .../caps_2.10.0.x86_64.xml                    |   1 +
 .../caps_2.11.0.s390x.xml                     |   1 +
 .../caps_2.11.0.x86_64.xml                    |   1 +
 .../caps_2.12.0.aarch64.xml                   |   1 +
 .../caps_2.12.0.ppc64.xml                     |   1 +
 .../caps_2.12.0.s390x.xml                     |   1 +
 .../caps_2.12.0.x86_64.xml                    |   1 +
 .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |   1 +
 .../qemucapabilitiesdata/caps_2.9.0.s390x.xml |   1 +
 .../caps_2.9.0.x86_64.xml                     |   1 +
 .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |   1 +
 .../caps_3.0.0.riscv32.xml                    |   1 +
 .../caps_3.0.0.riscv64.xml                    |   1 +
 .../qemucapabilitiesdata/caps_3.0.0.s390x.xml |   1 +
 .../caps_3.0.0.x86_64.xml                     |   1 +
 tools/virsh-domain-monitor.c                  |   7 +
 tools/virsh-domain.c                          | 110 +++++
 tools/virsh.pod                               |  47 ++-
 36 files changed, 825 insertions(+), 37 deletions(-)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]