* [PATCH v2 0/1] migration/dirtyrate: Fix segmentation fault @ 2024-04-23 9:13 Masato Imai 2024-04-23 9:13 ` [PATCH v2 1/1] " Masato Imai 0 siblings, 1 reply; 7+ messages in thread From: Masato Imai @ 2024-04-23 9:13 UTC (permalink / raw) To: qemu-devel; +Cc: Masato Imai Changes from v1: - fix typo in commit message - added an extra check for dirty bitmap mode Masato Imai (1): migration/dirtyrate: Fix segmentation fault migration/dirtyrate.c | 7 +++++++ 1 file changed, 7 insertions(+) -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/1] migration/dirtyrate: Fix segmentation fault 2024-04-23 9:13 [PATCH v2 0/1] migration/dirtyrate: Fix segmentation fault Masato Imai @ 2024-04-23 9:13 ` Masato Imai 2024-04-23 13:35 ` Peter Xu 0 siblings, 1 reply; 7+ messages in thread From: Masato Imai @ 2024-04-23 9:13 UTC (permalink / raw) To: qemu-devel; +Cc: Masato Imai, Hyman Huang, Peter Xu, Fabiano Rosas 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; + } if (((mode == DIRTY_RATE_MEASURE_MODE_DIRTY_RING) && !kvm_dirty_ring_enabled()) || ((mode == DIRTY_RATE_MEASURE_MODE_DIRTY_BITMAP) && -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] migration/dirtyrate: Fix segmentation fault 2024-04-23 9:13 ` [PATCH v2 1/1] " Masato Imai @ 2024-04-23 13:35 ` Peter Xu 2024-04-24 1:28 ` Yong Huang 0 siblings, 1 reply; 7+ messages in thread From: Peter Xu @ 2024-04-23 13:35 UTC (permalink / raw) To: Masato Imai; +Cc: qemu-devel, Hyman Huang, Fabiano Rosas 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? > if (((mode == DIRTY_RATE_MEASURE_MODE_DIRTY_RING) && > !kvm_dirty_ring_enabled()) || > ((mode == DIRTY_RATE_MEASURE_MODE_DIRTY_BITMAP) && > -- > 2.34.1 > -- Peter Xu ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] migration/dirtyrate: Fix segmentation fault 2024-04-23 13:35 ` Peter Xu @ 2024-04-24 1:28 ` Yong Huang 2024-04-24 4:52 ` mii 0 siblings, 1 reply; 7+ messages in thread From: Yong Huang @ 2024-04-24 1:28 UTC (permalink / raw) To: Peter Xu; +Cc: Masato Imai, qemu-devel, Fabiano Rosas [-- 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 --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] migration/dirtyrate: Fix segmentation fault 2024-04-24 1:28 ` Yong Huang @ 2024-04-24 4:52 ` mii 2024-04-24 7:17 ` Zhijian Li (Fujitsu) via 0 siblings, 1 reply; 7+ messages in thread From: mii @ 2024-04-24 4:52 UTC (permalink / raw) To: Yong Huang, Peter Xu; +Cc: qemu-devel, Fabiano Rosas [-- Attachment #1: Type: text/plain, Size: 2441 bytes --] On 2024/04/24 10:28, Yong Huang wrote: > > > 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 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; } [-- Attachment #2: Type: text/html, Size: 5644 bytes --] ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] migration/dirtyrate: Fix segmentation fault 2024-04-24 4:52 ` mii @ 2024-04-24 7:17 ` Zhijian Li (Fujitsu) via 2024-04-29 1:51 ` Yong Huang 0 siblings, 1 reply; 7+ messages in thread From: Zhijian Li (Fujitsu) via @ 2024-04-24 7:17 UTC (permalink / raw) To: mii, Yong Huang, Peter Xu; +Cc: qemu-devel@nongnu.org, Fabiano Rosas On 24/04/2024 12:52, mii wrote: > > On 2024/04/24 10:28, Yong Huang wrote: >> >> >> 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 > > 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) > { How about, return kvm_state && vm_state->kvm_dirty_ring_size; Thanks Zhijian > + if (kvm_state == NULL) { > + return false; > + } > return kvm_state->kvm_dirty_ring_size ? true : false; > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] migration/dirtyrate: Fix segmentation fault 2024-04-24 7:17 ` Zhijian Li (Fujitsu) via @ 2024-04-29 1:51 ` Yong Huang 0 siblings, 0 replies; 7+ messages in thread From: Yong Huang @ 2024-04-29 1:51 UTC (permalink / raw) To: Zhijian Li (Fujitsu); +Cc: mii, Peter Xu, qemu-devel@nongnu.org, Fabiano Rosas [-- Attachment #1: Type: text/plain, Size: 3067 bytes --] On Wed, Apr 24, 2024 at 3:17 PM Zhijian Li (Fujitsu) <lizhijian@fujitsu.com> wrote: > > > On 24/04/2024 12:52, mii wrote: > > > > On 2024/04/24 10:28, Yong Huang wrote: > >> > >> > >> 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 > > > > 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) > > { > > > How about, > > return kvm_state && vm_state->kvm_dirty_ring_size; > Hi, Masato Imai, would you like to continue this patch with version 3 as suggested? > > > Thanks > Zhijian > > > > + if (kvm_state == NULL) { > > + return false; > > + } > > return kvm_state->kvm_dirty_ring_size ? true : false; > > } > > Thanks, Yong -- Best regards [-- Attachment #2: Type: text/html, Size: 5365 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-04-29 1:54 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-23 9:13 [PATCH v2 0/1] migration/dirtyrate: Fix segmentation fault Masato Imai 2024-04-23 9:13 ` [PATCH v2 1/1] " Masato Imai 2024-04-23 13:35 ` Peter Xu 2024-04-24 1:28 ` Yong Huang 2024-04-24 4:52 ` mii 2024-04-24 7:17 ` Zhijian Li (Fujitsu) via 2024-04-29 1:51 ` Yong Huang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).