Re: [PATCH v6 0/8] support mode option for dirtyrate calculation

On 2/20/22 14:28, huangy81(a)chinatelecom.cn wrote: > From: Hyman Huang(黄勇) <huangy81(a)chinatelecom.cn&gt; > > v6: > - rebase on mastr > - drop the commit [PATCH v5 1/9] in this patchset, post an extra > commit if needed in the future. > > Please review, thanks ! > > Regards > Yong > > v5: > - [PATCH v5 9/9]: Fix alignment error in qemuDomainGetStatsDirtyRate. > > v4: > - Rebase the master > - [PATCH v4 1/9]: Refactor dirty page rate calculation status > implementation, display calc_status as string when > 'virsh domstats --dirtyrate' api return. > - [PATCH v4 3/9]: Adjust the 'cap check' block before BeginJob(). > - [PATCH v4 5/9]: Drop the virDomainDirtyRateCalcMode and introduce > internal enum qemuMonitorDirtyRateCalcMode instead. > - [PATCH v4 6/9]: Code clean. > - [PATCH v4 7/9]: Split qemu_driver logic of domdirtyrate-calc virsh > api into a separate commit. > - [PATCH v4 8/9]: Change the 'mode' parameter usage as --mode=[xxx|yyy], > code clean in qemuDomainStartDirtyRateCalc. > - [PATCH v4 9/9]: Display calc_mode as string and do code clean in > qemuMonitorJSONExtractDirtyRateInfo. > Thanks Michal and Peter for reviewing the previous versions, please review. > > Regards > Yong > > v3: > - Rebase the master > - [PATCH v2 2/6]: Fix the usage of virQEMUCapsGet > - [PATCH v2 4/6]: Fix the cleanup missed in qemuDomainStartDirtyRateCalc > - [PATCH v2 4/6]: Move all blocks below ACL check > - [PATCH v2 4/6]: Make the qemuMonitorJSONStartDirtyRateCalc cleaner by > merging the different case of qemuMonitorJSONMakeCommand > - [PATCH v2 4/6]: Code clean, make the error message not be line-broken > - [PATCH v2 5/6]: Abstract the enum definition into a standalone commit > - [PATCH v2 5/6]: Move the validations code above calculating flags block > - [PATCH v2 6/6]: Change the type of 'value' field to unsigned in > struct qemuMonitorDirtyRateVcpu > - [PATCH v2 6/6]: Rename the enum type qemuMonitorDirtyRateCalcMode to > virDomainDirtyRateCalcMode > - [PATCH v2 6/6]: Code clean, align the code in qemuDomainGetStatsDirtyRate > > Thanks Peter for quick and precise response, please review. > > Regards > > Yong > > v2: > Rebase master and fix confilicts with commit > "Introduce QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_PREALLOC" > > Thanks ! > > v1: > This patchset introduce mode option as the supplement of > qemuDomainStartDirtyRateCalc api, add calc_mode for dirtyrate > statistics correspondingly. > > Qemu add mode parameter for calc-dirty-rate command since >= 6.2.0, > either of these three mode "page-sampling, dirty-bitmap, dirty-ring" > can be specified when calculating dirty page rate. > > Page sampling is the original mode and used as default mode. > > Dirty bitmap mode use kvm log sync api to fetch the dirty-bitmap > and count the increased 1 bits number during measurement, thus, > calculate the dirty page rate. > > Dirty ring mode use the dirty-ring mechanism implemented in Qemu > which can count the increased dirty page on virtual cpu granularity, > thus, calculate the per-vcpu dirty page rate. > > These three calculation mode can be used in different scenarios, and > the dirty-bitmap, dirty-ring mode may be more accurate to a certain > degree. So maybe it's time to support the mode option for dirtyrate > calculation. > > This series make main modifications as the following: > 1. introduce QEMU_CAPS_CALC_DIRTY_RATE capability to probe > calc-dirty-rate command in case of failure since it just > introduced since >= 5.2.0 > > 2. introduce QEMU_CAPS_DIRTYRATE_MODE capability to probe > mode option of calc-dirty-rate command in case of failure, same > as 1. > > 3. implement mode option support for dirtyrate calculation. > > Please review, thanks ! > > Best Regards ! > > Hyman Huang(黄勇) (8): > qemu_capabilities: Introduce QEMU_CAPS_CALC_DIRTY_RATE capability > qemu_driver: Probe capability before calculating dirty page rate > qemu_capabilities: Introduce QEMU_CAPS_DIRTYRATE_MODE capability > include: Introduce virDomainDirtyRateCalcFlags > qemu_driver: Add mode parameter to qemuDomainStartDirtyRateCalc > qemu_driver: Extend flags parameter of virDomainStartDirtyRateCalc > virsh: Add mode option to domdirtyrate-calc virsh api > qemu_driver: Add calc_mode for dirtyrate statistics > > docs/manpages/virsh.rst | 7 ++- > include/libvirt/libvirt-domain.h | 13 +++++ > src/libvirt-domain.c | 18 +++++- > src/qemu/qemu_capabilities.c | 4 ++ > src/qemu/qemu_capabilities.h | 2 + > src/qemu/qemu_driver.c | 56 ++++++++++++++++-- > src/qemu/qemu_monitor.c | 5 +- > src/qemu/qemu_monitor.h | 27 ++++++++- > src/qemu/qemu_monitor_json.c | 69 ++++++++++++++++++++++- > src/qemu/qemu_monitor_json.h | 3 +- > tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + > tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + > tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 2 + > tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 2 + > tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 2 + > tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml | 2 + > tools/virsh-completer-domain.c | 17 ++++++ > tools/virsh-completer-domain.h | 4 ++ > tools/virsh-domain.c | 42 +++++++++++++- > tools/virsh-domain.h | 9 +++ > 28 files changed, 278 insertions(+), 16 deletions(-) > Alright, all the issues I've found are trivial to fix before pushing. Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com&gt; and pushed. One more thing, do you think you can post a patch against NEWS.rst to document this new feature?

Michal