On 2024/04/24 10:28, Yong Huang wrote:


On Tue, Apr 23, 2024 at 9:35 PM Peter Xu <pet...@redhat.com> wrote:

    On Tue, Apr 23, 2024 at 09:13:08AM +0000, Masato Imai wrote:
    > When the KVM acceleration parameter is not set, executing
    calc_dirty_rate
    > with the -r or -b option results in a segmentation fault due to
    accessing
    > a null kvm_state pointer in the kvm_dirty_ring_enabled function.
    > This commit adds a check for kvm_enabled to prevent segmentation
    faults.
    >
    > Signed-off-by: Masato Imai <m...@sfc.wide.ad.jp>
    > ---
    >  migration/dirtyrate.c | 7 +++++++
    >  1 file changed, 7 insertions(+)
    >
    > diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
    > index 1d2e85746f..2a7df52519 100644
    > --- a/migration/dirtyrate.c
    > +++ b/migration/dirtyrate.c
    > @@ -799,6 +799,13 @@ void qmp_calc_dirty_rate(int64_t calc_time,
    >       * dirty ring mode only works when kvm dirty ring is enabled.
    >       * on the contrary, dirty bitmap mode is not.
    >       */
    > +    if (!kvm_enabled() &&
    > +        (mode == DIRTY_RATE_MEASURE_MODE_DIRTY_RING ||
    > +         mode == DIRTY_RATE_MEASURE_MODE_DIRTY_BITMAP)) {
    > +        error_setg(errp, "mode %s requires kvm to be enabled.",
    > +  DirtyRateMeasureMode_str(mode));
    > +        return;
    > +    }

    Logically dirty bitmap should work with tcg.  So the other option
    is to let
    kvm_dirty_ring_enabled() check kvm_state too and return false if
    kvm_state==NULL?


Agree, better solution


    >      if (((mode == DIRTY_RATE_MEASURE_MODE_DIRTY_RING) &&
    >          !kvm_dirty_ring_enabled()) ||
    >          ((mode == DIRTY_RATE_MEASURE_MODE_DIRTY_BITMAP) &&
    > --
    > 2.34.1
    >

-- Peter Xu


Thanks,
Yong


--
Best regards

Thank you for the review. I agree with that solution.

Update will be like:

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 931f74256e..0f8499365d 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2312,6 +2312,9 @@ bool kvm_vcpu_id_is_valid(int vcpu_id)

 bool kvm_dirty_ring_enabled(void)
 {
+    if (kvm_state == NULL) {
+        return false;
+    }
     return kvm_state->kvm_dirty_ring_size ? true : false;
 }

Reply via email to