On Wed, Mar 25, 2020 at 05:52:20PM +0000, Dr. David Alan Gilbert wrote: > * Peter Xu (pet...@redhat.com) wrote: > > Provide a helper kvm_slot_get_dirty_log() to make the function > > kvm_physical_sync_dirty_bitmap() clearer. We can even cache the as_id > > into KVMSlot when it is created, so that we don't even need to pass it > > down every time. > > > > Since at it, remove return value of kvm_physical_sync_dirty_bitmap() > > because it should never fail. > > > > Signed-off-by: Peter Xu <pet...@redhat.com> > > --- > > accel/kvm/kvm-all.c | 39 +++++++++++++++++++-------------------- > > include/sysemu/kvm_int.h | 2 ++ > > 2 files changed, 21 insertions(+), 20 deletions(-) > > > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > > index bb635c775f..608216fd53 100644 > > --- a/accel/kvm/kvm-all.c > > +++ b/accel/kvm/kvm-all.c > > @@ -553,6 +553,18 @@ static void kvm_slot_init_dirty_bitmap(KVMSlot *mem) > > mem->dirty_bmap = g_malloc0(bitmap_size); > > } > > > > +/* Sync dirty bitmap from kernel to KVMSlot.dirty_bmap */ > > +static void kvm_slot_get_dirty_log(KVMState *s, KVMSlot *slot) > > +{ > > + struct kvm_dirty_log d = {}; > > + int ret; > > + > > + d.dirty_bitmap = slot->dirty_bmap; > > + d.slot = slot->slot | (slot->as_id << 16); > > + ret = kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d); > > + assert(ret != -1); > > As discussed on irc, that -1 check seems odd given your previous check > but there seems to be some history as to why it was still there. Hmm. > > It also seems very trusting that it can never possibly fail!
Yeah I agree. I've asked the question under patch 1. Before we know the answer, I'll remove the assertion to print an error if it triggers; check against -1 (-EPERM) explicitly does look odd... Thanks, -- Peter Xu