From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: Re: [PATCH] kvm: Fix overlapping check for memory slots Date: Mon, 13 Apr 2009 16:53:53 +0800 Message-ID: <200904131653.54515.sheng@linux.intel.com> References: <49E06754.8050906@web.de> <200904131347.29389.sheng@linux.intel.com> <49E2FCE0.5010509@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Cc: Avi Kivity , kvm@vger.kernel.org, Glauber Costa , "qemu-devel" To: Jan Kiszka Return-path: Received: from mga09.intel.com ([134.134.136.24]:18601 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751300AbZDMIyA (ORCPT ); Mon, 13 Apr 2009 04:54:00 -0400 In-Reply-To: <49E2FCE0.5010509@web.de> Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On Monday 13 April 2009 16:50:40 Jan Kiszka wrote: > Sheng Yang wrote: > > On Saturday 11 April 2009 17:48:04 Jan Kiszka wrote: > >> This nice little buglet complicates a smarter slot management in qemu > >> user space just "slightly". Sigh... > >> > >> --------> > >> > >> When checking for overlapping slots on registration of a new one, kvm > >> currently also considers zero-length (ie. deleted) slots and rejects > >> requests incorrectly. This finally denies user space from joining slots. > >> Fix the check by skipping deleted slots. > >> > >> Signed-off-by: Jan Kiszka > >> --- > >> > >> virt/kvm/kvm_main.c | 2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > >> index 363af32..18f06d2 100644 > >> --- a/virt/kvm/kvm_main.c > >> +++ b/virt/kvm/kvm_main.c > >> @@ -1117,7 +1117,7 @@ int __kvm_set_memory_region(struct kvm *kvm, > >> for (i = 0; i < KVM_MEMORY_SLOTS; ++i) { > >> struct kvm_memory_slot *s = &kvm->memslots[i]; > >> > >> - if (s == memslot) > >> + if (s == memslot || !s->npages) > >> continue; > >> if (!((base_gfn + npages <= s->base_gfn) || > >> (base_gfn >= s->base_gfn + s->npages))) > > > > Is it necessary to preserve a valid base_gfn/flags/etc for a zeroed slot? > > Seems kvm_free_physmem_slot didn't clean them. > > It is not necessary as long as we ignore such slots (as this patch does). What I think is, if they are invalid and unnecessary to keep, it's better to clean them rather than add a additional check, for it should covered by current check. -- regards Yang, Sheng