* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.