|
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 somethingthat's exposed to userspace?Yes, but we can't do this from user space :) (or in other words: thereare already kernels out there which return this invalid code).But since it's not symbolic, at some point in time the meaning of 524can 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 further 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 ENOTSUPP case?
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. Alex
If not, I see no point in handling it here. Please fix KVM and provide apatch 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).Jan
[Prev in Thread] | Current Thread | [Next in Thread] |