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

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




On 11/05/2018 01:58 PM, John Ferlan wrote:
> 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
>           nothing)
> 
> 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>:
> 
>   https://www.redhat.com/archives/libvir-list/2017-February/msg01047.html
> 
> The work is prompted by continued pleas found in the bz:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1545732
> 
> 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

For the feature itself consider the patch series
Acked-by: Christian Borntraeger <borntraeger de ibm com>

I always wanted this kind of control.


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