* [Qemu-devel] [PATCH for-2.0] kvm_physical_sync_dirty_bitmap: ignore ENOENT from kvm_vm_ioctl
@ 2014-04-11 8:31 Michael Tokarev
2014-04-11 15:59 ` Serge Hallyn
2014-04-12 1:50 ` Paolo Bonzini
0 siblings, 2 replies; 4+ messages in thread
From: Michael Tokarev @ 2014-04-11 8:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Serge Hallyn, Michael Tokarev, Mario Smarduch
ENOENT 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
Cc: Serge Hallyn <serge.hallyn@ubuntu.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
kvm-all.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index cd4111d..47fa948 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -441,13 +441,19 @@ static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
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) {
+ /* regular case, process returned bitmap */
+ kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
+ } else if (ret == -ENOENT) {
+ /* kernel does not have dirty bitmap in this slot */
+ ret = 0;
+ } else {
DPRINTF("ioctl failed %d\n", errno);
ret = -1;
break;
}
- kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
start_addr = mem->start_addr + mem->memory_size;
}
g_free(d.dirty_bitmap);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH for-2.0] kvm_physical_sync_dirty_bitmap: ignore ENOENT from kvm_vm_ioctl
2014-04-11 8:31 [Qemu-devel] [PATCH for-2.0] kvm_physical_sync_dirty_bitmap: ignore ENOENT from kvm_vm_ioctl Michael Tokarev
@ 2014-04-11 15:59 ` Serge Hallyn
2014-04-12 1:50 ` Paolo Bonzini
1 sibling, 0 replies; 4+ messages in thread
From: Serge Hallyn @ 2014-04-11 15:59 UTC (permalink / raw)
To: Michael Tokarev; +Cc: Peter Maydell, qemu-devel, Mario Smarduch
Quoting Michael Tokarev (mjt@tls.msk.ru):
> ENOENT 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
>
> Cc: Serge Hallyn <serge.hallyn@ubuntu.com>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
This does look better than my version, thanks. As you say my version
was closer to the old behavior, but if the kernel has no dirty
bitmap we shouldn't be acting on it, so this looks better imo.
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
(I've not tested yet, but it looks right and works for you :)
> ---
> kvm-all.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index cd4111d..47fa948 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -441,13 +441,19 @@ static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
>
> 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) {
> + /* regular case, process returned bitmap */
> + kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
> + } else if (ret == -ENOENT) {
> + /* kernel does not have dirty bitmap in this slot */
> + ret = 0;
> + } else {
> DPRINTF("ioctl failed %d\n", errno);
> ret = -1;
> break;
> }
>
> - kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
> start_addr = mem->start_addr + mem->memory_size;
> }
> g_free(d.dirty_bitmap);
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH for-2.0] kvm_physical_sync_dirty_bitmap: ignore ENOENT from kvm_vm_ioctl
2014-04-11 8:31 [Qemu-devel] [PATCH for-2.0] kvm_physical_sync_dirty_bitmap: ignore ENOENT from kvm_vm_ioctl Michael Tokarev
2014-04-11 15:59 ` Serge Hallyn
@ 2014-04-12 1:50 ` Paolo Bonzini
2014-04-12 2:10 ` Serge Hallyn
1 sibling, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2014-04-12 1:50 UTC (permalink / raw)
To: Michael Tokarev, qemu-devel; +Cc: Peter Maydell, Serge Hallyn, Mario Smarduch
Il 11/04/2014 04:31, Michael Tokarev ha scritto:
> ENOENT 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
>
> Cc: Serge Hallyn <serge.hallyn@ubuntu.com>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> kvm-all.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index cd4111d..47fa948 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -441,13 +441,19 @@ static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
>
> 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) {
> + /* regular case, process returned bitmap */
> + kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
> + } else if (ret == -ENOENT) {
> + /* kernel does not have dirty bitmap in this slot */
> + ret = 0;
> + } else {
> DPRINTF("ioctl failed %d\n", errno);
> ret = -1;
> break;
> }
>
> - kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
> start_addr = mem->start_addr + mem->memory_size;
> }
> g_free(d.dirty_bitmap);
>
I'd rather revert b533f658a98325d instead.
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH for-2.0] kvm_physical_sync_dirty_bitmap: ignore ENOENT from kvm_vm_ioctl
2014-04-12 1:50 ` Paolo Bonzini
@ 2014-04-12 2:10 ` Serge Hallyn
0 siblings, 0 replies; 4+ messages in thread
From: Serge Hallyn @ 2014-04-12 2:10 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Peter Maydell, Michael Tokarev, qemu-devel, Mario Smarduch
Quoting Paolo Bonzini (pbonzini@redhat.com):
> Il 11/04/2014 04:31, Michael Tokarev ha scritto:
> >ENOENT 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
> >
> >Cc: Serge Hallyn <serge.hallyn@ubuntu.com>
> >Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> >---
> > kvm-all.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> >diff --git a/kvm-all.c b/kvm-all.c
> >index cd4111d..47fa948 100644
> >--- a/kvm-all.c
> >+++ b/kvm-all.c
> >@@ -441,13 +441,19 @@ static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
> >
> > 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) {
> >+ /* regular case, process returned bitmap */
> >+ kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
> >+ } else if (ret == -ENOENT) {
> >+ /* kernel does not have dirty bitmap in this slot */
> >+ ret = 0;
> >+ } else {
> > DPRINTF("ioctl failed %d\n", errno);
> > ret = -1;
> > break;
> > }
> >
> >- kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
> > start_addr = mem->start_addr + mem->memory_size;
> > }
> > g_free(d.dirty_bitmap);
> >
>
> I'd rather revert b533f658a98325d instead.
That seems wrong though. If we want to ignore all errors that's one
thing, but before that commit we just ignored all errors other than
EPERM.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-04-12 2:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-11 8:31 [Qemu-devel] [PATCH for-2.0] kvm_physical_sync_dirty_bitmap: ignore ENOENT from kvm_vm_ioctl Michael Tokarev
2014-04-11 15:59 ` Serge Hallyn
2014-04-12 1:50 ` Paolo Bonzini
2014-04-12 2:10 ` Serge Hallyn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).