Skip to contentSkip to conversation

kvm_physical_sync_dirty_bitmap: ignore ENOENT from kvm_vm_ioctl

PATCH In review Serge Hallyn 3 participants 4 messages Apr 9, 2014 – Apr 10, 2014

lore
Share & tools
lore

Serge Hallyn

author
#0 lore raw

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

which first showed up with commit b533f658a98325d: fix return check for KVM_GET_DIRTY_LOG ioctl

Signed-off-by Serge Hallyn <[email protected]>
---
 kvm-all.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
kvm-all.c +4 -1
index 82a9119..7b7ea8d 100644
@@ -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);
Signature · 1 line
1.9.1
#1 lore raw

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

Quoted 1 line

From 0d818e334f6db88b2770e9a1076ae1e68c41e460 Mon Sep 17 00:00:00 2001

From: Serge Hallyn <[email protected]>
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 <[email protected]>
---
 kvm-all.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
kvm-all.c +4 -1
Index: qemu-2.0.0~rc1+dfsg/kvm-all.c
===================================================================
@@ -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);
#2 lore raw
In reply to Serge Hallyn
Quoted 23 lines · 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 <[email protected]>
---
 kvm-all.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
kvm-all.c +0 -1
index 82a9119..7b7ea8d 100644
@@ -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) {

kvm-all.c static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section) · line 453

Quoted 10 lines
kvm-all.c +4 -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);

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

author
#3 lore raw
Quoted 33 lines · Michael Tokarev
Quoted 32 lines · 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 <[email protected]>
---
 kvm-all.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
kvm-all.c +4 -1
index 82a9119..7b7ea8d 100644
@@ -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;
         }
Quoted 4 lines

kvm-all.c static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section) · line 453

Quoted 1 line
kvm-all.c +0 -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.

Quoted 2 lines

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