It might be confusing for some listener implementations that implement
both, region_add and log_start (e.g. KVM) if we call log_start before an
actual region was added using region_add.
This makes current KVM code trigger an assertion
("kvm_section_update_flags: error finding slot"). So let's just reverse
the order instead of tolerating log_start on yet unknown regions.
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
memory.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/memory.c b/memory.c
index 5e6351a6c1..f39b8592bf 100644
--- a/memory.c
+++ b/memory.c
@@ -2607,12 +2607,12 @@ static void listener_add_address_space(MemoryListener *listener,
.offset_within_address_space = int128_get64(fr->addr.start),
.readonly = fr->readonly,
};
- if (fr->dirty_log_mask && listener->log_start) {
- listener->log_start(listener, §ion, 0, fr->dirty_log_mask);
- }
if (listener->region_add) {
listener->region_add(listener, §ion);
}
+ if (fr->dirty_log_mask && listener->log_start) {
+ listener->log_start(listener, §ion, 0, fr->dirty_log_mask);
+ }
}
if (listener->commit) {
listener->commit(listener);
--
2.13.5
On 16.10.2017 11:18, David Hildenbrand wrote: > It might be confusing for some listener implementations that implement > both, region_add and log_start (e.g. KVM) if we call log_start before an > actual region was added using region_add. > > This makes current KVM code trigger an assertion > ("kvm_section_update_flags: error finding slot"). So let's just reverse > the order instead of tolerating log_start on yet unknown regions. > > Reported-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > memory.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/memory.c b/memory.c > index 5e6351a6c1..f39b8592bf 100644 > --- a/memory.c > +++ b/memory.c > @@ -2607,12 +2607,12 @@ static void listener_add_address_space(MemoryListener *listener, > .offset_within_address_space = int128_get64(fr->addr.start), > .readonly = fr->readonly, > }; > - if (fr->dirty_log_mask && listener->log_start) { > - listener->log_start(listener, §ion, 0, fr->dirty_log_mask); > - } > if (listener->region_add) { > listener->region_add(listener, §ion); > } > + if (fr->dirty_log_mask && listener->log_start) { > + listener->log_start(listener, §ion, 0, fr->dirty_log_mask); > + } > } > if (listener->commit) { > listener->commit(listener); > ... deciding to tolerate it in KVM instead (just as it used to be), so please ignore this patch. -- Thanks, David
On 16/10/2017 11:36, David Hildenbrand wrote: >> - if (fr->dirty_log_mask && listener->log_start) { >> - listener->log_start(listener, §ion, 0, fr->dirty_log_mask); >> - } >> if (listener->region_add) { >> listener->region_add(listener, §ion); >> } >> + if (fr->dirty_log_mask && listener->log_start) { >> + listener->log_start(listener, §ion, 0, fr->dirty_log_mask); >> + } >> } >> if (listener->commit) { >> listener->commit(listener); >> > ... deciding to tolerate it in KVM instead (just as it used to be), so > please ignore this patch. Why? It makes no sense to start logging on a non-existent region, and this behavior is unique to listener_add_address_space. Paolo
On 16.10.2017 14:19, Paolo Bonzini wrote: > On 16/10/2017 11:36, David Hildenbrand wrote: >>> - if (fr->dirty_log_mask && listener->log_start) { >>> - listener->log_start(listener, §ion, 0, fr->dirty_log_mask); >>> - } >>> if (listener->region_add) { >>> listener->region_add(listener, §ion); >>> } >>> + if (fr->dirty_log_mask && listener->log_start) { >>> + listener->log_start(listener, §ion, 0, fr->dirty_log_mask); >>> + } >>> } >>> if (listener->commit) { >>> listener->commit(listener); >>> >> ... deciding to tolerate it in KVM instead (just as it used to be), so >> please ignore this patch. > > Why? It makes no sense to start logging on a non-existent region, and > this behavior is unique to listener_add_address_space. As you requested, will add everything into one patch series and send out for review. Thanks! -- Thanks, David
© 2016 - 2024 Red Hat, Inc.