memory: call log_start after region_add
PATCH 1/1 memory: call log_start after region_add
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.
--- memory.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
memory.c +3 -3
@@ -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);
Signature · 1 line
2.13.5
Quoted 25 lines · David Hildenbrand
It might be confusing for some listener implementations that implement both,
region_addandlog_start(e.g. KVM) if we calllog_startbefore an actual region was added usingregion_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 toleratinglog_starton yet unknown regions.Reported-by Thomas Huth <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- memory.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)memory.c +0
-2@@ -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);
memory.c static void listener_add_address_space(MemoryListener *listener, · line 2618
Quoted 10 lines
memory.c +3
-1- }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.
Signature · 3 lines
Thanks, David
Quoted 13 lines · David Hildenbrand
memory.c static void listener_add_address_space(MemoryListener *listener, · line 2618
Quoted 13 lines
memory.c +3
-3-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);
Quoted 2 lines
... 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
Quoted 14 lines · Paolo Bonzini
Quoted 13 lines · David Hildenbrand
Quoted 13 lines
memory.c +3
-3-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);
Quoted 5 lines
Quoted 2 lines
... 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!
Signature · 3 lines
Thanks, David