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

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

[Serge Hallyn]

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 <serge.hallyn@ubuntu.com>
---
 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);
-- 
1.9.1

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

[Serge E. Hallyn]

D'oh, sorry.  I generated that patch against my qemu tree at the bad_commit^.
That won't apply to the tree...  this should.

>From 0d818e334f6db88b2770e9a1076ae1e68c41e460 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn@ubuntu.com>
Date: Tue, 8 Apr 2014 22:14:20 -0500
Subject: [PATCH 1/1] kvm_physical_sync_dirty_bitmap: ignore ENOENT from
 kvm_vm_ioctl

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

Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
---
 kvm-all.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: qemu-2.0.0~rc1+dfsg/kvm-all.c
===================================================================
--- qemu-2.0.0~rc1+dfsg.orig/kvm-all.c	2014-04-08 22:28:38.000722081 -0500
+++ qemu-2.0.0~rc1+dfsg/kvm-all.c	2014-04-08 22:29:14.644722904 -0500
@@ -441,10 +441,13 @@ static int kvm_physical_sync_dirty_bitma
 
         d.slot = mem->slot;
 
-        if (kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d) < 0) {
+        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);

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

[Michael Tokarev]

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 <serge.hallyn@ubuntu.com>
> ---
>  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?  In particular, do we trust kernel to not
touch d.dirty_bitmap when it returns ENOENT?

Thanks,

/mjt

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

[Serge Hallyn]

Quoting Michael Tokarev (mjt@tls.msk.ru):
> 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 <serge.hallyn@ubuntu.com>
> > ---
> >  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