Re: [PATCH v2 1/1] migration/dirtyrate: Fix segmentation fault

[Yong Huang <yong.huang@smartx.com>]

[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]

On Tue, Apr 23, 2024 at 9:35 PM Peter Xu <peterx@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 <mii@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

[-- Attachment #2: Type: text/html, Size: 3212 bytes --]