[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH] kvm: Fix error detection for KVM_GET_DIRTY_LOG

From: Alexander Graf
Subject: [Qemu-devel] Re: [PATCH] kvm: Fix error detection for KVM_GET_DIRTY_LOG
Date: Wed, 22 Jul 2009 23:17:38 +0200

Am 22.07.2009 um 23:14 schrieb Jan Kiszka <address@hidden>:

Alexander Graf wrote:

Am 22.07.2009 um 22:07 schrieb Jan Kiszka <address@hidden>:

Anthony Liguori wrote:
Jan Kiszka wrote:
That makes me uncomfortable. Shouldn't we make kvm return something
that's exposed to userspace?

Yes, but we can't do this from user space :) (or in other words: there
are already kernels out there which return this invalid code).

But since it's not symbolic, at some point in time the meaning of 524
can potentially change and introduce a very, very subtle bug.

I won't change in old kernel versions, and I expect someone from the
PowerPC folks to fix it for new version fairly soon.

The situation would only be different if Alex said that it takes
kernel patches anyway to make his PowerPC targets work. Dunno.

Certainly, his PPC target is not in any released kernel version so
there's time to fix things properly.

Ah, ok. Then let's do this (Alex can carry a temporary workaround
locally IMHO):


Signed-off-by: Jan Kiszka <address@hidden>

kvm-all.c |    2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 824bb4c..5fb8dba 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -348,7 +348,7 @@ int
kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
       d.slot = mem->slot;

       r = kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d);
-        if (r == -EINVAL) {
+        if (r < 0) {

That's worse as it essentially removes the not implemented case.

You can add the valid return code of a fixed powerpc version of
kvm_vm_ioctl_get_dirty_log here.

The only thing I'd agree with as a good idea instead of the version I
sent is an explicit switch statement with an include of errno.h with
__KERNEL__ set.

Sorry, that's also horrible.

Again the question: Is there already some official, unpatched kernel
version which would work with QEMU if we only handle that borken

No, but I haven't implemented dirty logging at all yet.

I guess the real question is what does IA64 do and check for that.


If not, I see no point in handling it here. Please fix KVM and provide a
patch that checks for the fixed, valid error code.

If there is such a kernel, I see no problem with hard-coding ENOTSUPP's value now, as we are working around a hard-coded bug that will no longer exist in the future (where ENOTSUPP may change its value - theoretically).


reply via email to

[Prev in Thread] Current Thread [Next in Thread]