From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 3/3] kvm common: verify that cpu slot is available when creating new vcpu Date: Sun, 08 Feb 2009 11:47:38 +0200 Message-ID: <498EAA3A.8010908@redhat.com> References: <20090205170124.46fee96b@cotte.boeblingen.de.ibm.com> <20090205170501.2a964172@cotte.boeblingen.de.ibm.com> <20090208062616.GD4437@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Carsten Otte , KVM mailing list To: Marcelo Tosatti Return-path: Received: from mx2.redhat.com ([66.187.237.31]:40775 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752698AbZBHJro (ORCPT ); Sun, 8 Feb 2009 04:47:44 -0500 In-Reply-To: <20090208062616.GD4437@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: Marcelo Tosatti wrote: > On Thu, Feb 05, 2009 at 05:05:01PM +0100, Carsten Otte wrote: > >> KVM common code should'nt try to create the same virtual cpu twice. >> In case of s390, it crashes badly in kvm_arch_vcpu_create. >> >> Reported-by: Mijo Safradin >> Signed-off-by: Carsten Otte >> --- >> virt/kvm/kvm_main.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> Index: kvm/virt/kvm/kvm_main.c >> =================================================================== >> --- kvm.orig/virt/kvm/kvm_main.c >> +++ kvm/virt/kvm/kvm_main.c >> @@ -1605,6 +1605,9 @@ static int kvm_vm_ioctl_create_vcpu(stru >> if (!valid_vcpu(n)) >> return -EINVAL; >> >> + if (kvm->vcpus[i]) >> + return -EEXIST; >> + >> vcpu = kvm_arch_vcpu_create(kvm, n); >> if (IS_ERR(vcpu)) >> return PTR_ERR(vcpu); >> > > Its confusing that there is the exact same check below, with kvm->lock > held, and that both are needed since assignment happens under the lock. > Right, also the proposed fix still leaves a race. > Can you also make it straightforward while fixing the bug please. > > Probably just hold it all the way through kvm_vm_ioctl_create_vcpu? Or > is that not possible? > The original intent was that kvm_arch_vcpu_create() not "link in" the vcpu to any registers. That allows most of the vcpu creation to happen outside a lock. If it's not doable for s390 we can give this up, but I suggest checking if it's possible to keep things as is and modify s390's kvm_arch_vcpu_create() not to screw up instead. -- error compiling committee.c: too many arguments to function