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 > : > > 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 > > 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_p
Re: [libvirt] [REPOST PATCH v2 00/12] Allow modification of IOThread polling values (redux)
ping? Tks, John On 11/5/18 7:58 AM, 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 > : > > 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 > > 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 +
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 > : > > 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 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 + >
[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 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 : 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 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|