Re: [Qemu-devel] [PATCH 1/1] kvm_physical_sync_dirty_bitmap: ignore ENOENT from kvm_vm_ioctl

[Serge Hallyn]

Quoting Michael Tokarev (address@hidden):
> 09.04.2014 07:21, Serge Hallyn wrote:
> > ENOENT (iiuc) means the kernel has an empty dirty bitmap for this
> > slot.  Don't abort in that case.  This appears to solve the bug
> > reported at
> > 
> > https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1303926
> > 
> > which first showed up with commit b533f658a98325d: fix return check for
> > KVM_GET_DIRTY_LOG ioctl
> > 
> > Signed-off-by: Serge Hallyn <address@hidden>
> > ---
> >  kvm-all.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kvm-all.c b/kvm-all.c
> > index 82a9119..7b7ea8d 100644
> > --- a/kvm-all.c
> > +++ b/kvm-all.c
> > @@ -441,10 +441,13 @@ static int 
> > kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
> >  
> >          d.slot = mem->slot;
> >  
> > -        if (kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d) == -1) {
> > +        ret = kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d);
> > +        if (ret < 0 && ret != -ENOENT) {
> >              DPRINTF("ioctl failed %d\n", errno);
> >              ret = -1;
> >              break;
> > +        } else if (ret < 0) {
> > +            ret = 0;
> >          }
> >  
> >          kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
> 
> Should we omit calling kvm_get_dirty_pages_log_range() if there's
> no bitmap from kernel?

If that's something we can know then certainly that'll be better.
It'll save an ioctl and copy_from_user of the whole of &d.

> In particular, do we trust kernel to not
> touch d.dirty_bitmap when it returns ENOENT?

Seems ok, kvm_vm_ioctl_get_dirty_log() doesn't change anything
in *log before returning when it finds no dirty_mapslot.

-serge