kvm: tolerate non-existing slot for log_start and log_stop
PATCH 1/1 kvm: tolerate non-existing slot for log_start and log_stop
log_start might be called by memory.c just before registering the section. So we can actually get a log_start without a region_add, which we can silently ignore.
This makes current KVM code trigger an assertion
("kvm_section_update_flags: error finding slot").
Also, if we want to trap every access to a section, we might not have a slot. So let's just tolerate if we don't have a slot.
--- accel/kvm/kvm-all.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
accel/kvm/kvm-all.c +5 -2
@@ -394,8 +394,11 @@ static int kvm_section_update_flags(KVMMemoryListener *kml, mem =kvm_lookup_matching_slot (kml, start_addr, size);if (!mem) { -fprintf (stderr,"%s: error finding slot\n" , __func__); -abort (); +/* +* log_start() might be called before region_add(), and sometimes +* we don't have a slot as we want to trap every access. +*/ +return 0 ; }return kvm_slot_update_flags (kml, mem, section->mr);
Signature · 1 line
2.13.5
Quoted 26 lines · David Hildenbrand
log_startmight be called bymemory.cjust before registering the section. So we can actually get alog_startwithout aregion_add, which we can silently ignore.This makes current KVM code trigger an assertion
("kvm_section_update_flags: error finding slot").Also, if we want to trap every access to a section, we might not have a slot. So let's just tolerate if we don't have a slot.
Fixes 343562e8fa22 ("kvm: kvm_log_start/stop are only called with known sections")Reported-by Thomas Huth <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- accel/kvm/kvm-all.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)accel/kvm/kvm-all.c +0
-0@@ -394,8 +394,11 @@ static int kvm_section_update_flags(KVMMemoryListener *kml, mem =kvm_lookup_matching_slot (kml, start_addr, size);if (!mem) {
accel/kvm/kvm-all.c static int kvm_section_update_flags(KVMMemoryListener *kml, · line 404
Quoted 10 lines
accel/kvm/kvm-all.c +5
-2-fprintf (stderr,"%s: error finding slot\n" , __func__); -abort (); +/* +* log_start() might be called before region_add(), and sometimes +* we don't have a slot as we want to trap every access. +*/ +return 0 ; }return kvm_slot_update_flags (kml, mem, section->mr);
I'll also send a patch for log_sync(), dropping the same assert.
Looks like adding these assertions was counter productive :)
Signature · 3 lines
Thanks, David
Quoted 2 lines · David Hildenbrand
Looks like adding these assertions was counter productive :)
Or productive. Bugs happen, and if they are a good occasion to add new tests to the testsuite or at least a comment to the code, all the better.
Paolo
Quoted 3 lines · David Hildenbrand
log_startmight be called bymemory.cjust before registering the section. So we can actually get alog_startwithout aregion_add, which we can silently ignore.
This is really a bug in memory.c, I think. When you put together everything as a single patch series, can you include the memory.c change instead, and drop this reference in the commit message and the kvm-all.c comment?
Thanks,
Paolo
Quoted 5 lines
This makes current KVM code trigger an assertion
("kvm_section_update_flags: error finding slot").Also, if we want to trap every access to a section, we might not have a slot. So let's just tolerate if we don't have a slot.