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

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

 



On 2/20/22 14:28, huangy81@xxxxxxxxxxxxxxx wrote:
> From: Hyman Huang(黄勇) <huangy81@xxxxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxx>

and pushed. One more thing, do you think you can post a patch against
NEWS.rst to document this new feature?

Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux