From mboxrd@z Thu Jan 1 00:00:00 1970 From: Glauber Costa Subject: Re: [PATCH 2/5] reuse kvm_vm_ioctl Date: Fri, 17 Jul 2009 13:48:01 -0300 Message-ID: <20090717164801.GF4019@poweredge.glommer> References: <1247675503-7106-1-git-send-email-glommer@redhat.com> <1247675503-7106-2-git-send-email-glommer@redhat.com> <1247675503-7106-3-git-send-email-glommer@redhat.com> <20090717153917.GA4978@amt.cnet> <20090717154904.GE4019@poweredge.glommer> <20090717154623.GB4978@amt.cnet> <20090717155738.GA5414@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, avi@redhat.com, aliguori@us.ibm.com To: Marcelo Tosatti Return-path: Received: from mx2.redhat.com ([66.187.237.31]:40055 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964913AbZGQQlU (ORCPT ); Fri, 17 Jul 2009 12:41:20 -0400 Content-Disposition: inline In-Reply-To: <20090717155738.GA5414@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Jul 17, 2009 at 12:57:38PM -0300, Marcelo Tosatti wrote: > On Fri, Jul 17, 2009 at 12:46:23PM -0300, Marcelo Tosatti wrote: > > On Fri, Jul 17, 2009 at 12:49:04PM -0300, Glauber Costa wrote: > > > On Fri, Jul 17, 2009 at 12:39:17PM -0300, Marcelo Tosatti wrote: > > > > On Wed, Jul 15, 2009 at 12:31:40PM -0400, Glauber Costa wrote: > > > > > Start using kvm_vm_ioctl's code. > > > > > For type safety, delete vm_fd from kvm_context entirely, so the > > > > > compiler can play along with us helping to detect errors I might > > > > > have made. > > > > > > > > > > Signed-off-by: Glauber Costa > > > > > --- > > > > > kvm-all.c | 2 ++ > > > > > qemu-kvm-x86.c | 18 +++++++++--------- > > > > > qemu-kvm.c | 52 ++++++++++++++++++++++++++-------------------------- > > > > > qemu-kvm.h | 6 +++--- > > > > > 4 files changed, 40 insertions(+), 38 deletions(-) > > > > > > > > > > > > > > > > > @@ -516,7 +516,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id) > > > > > vcpu_ctx->kvm = kvm; > > > > > vcpu_ctx->id = id; > > > > > > > > > > - r = ioctl(kvm->vm_fd, KVM_CREATE_VCPU, id); > > > > > + r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id); > > > > > if (r == -1) { > > > > > fprintf(stderr, "kvm_create_vcpu: %m\n"); > > > > > > > > > > > > int kvm_vm_ioctl(KVMState *s, int type, ...) > > > > { > > > > ... > > > > if (ret == -1) > > > > ret = -errno; > > > > } > > > > > > > > Is that fine? > > > I don't see a problem with that. > > > > Problem is the sites in qemu-kvm.c kvm_vm_ioctl to return -1 > > on error, but kvm_vm_ioctl converts -1 to -errno, no? > > BTW kvm-all.c: > > if (kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d) == -1) { > dprintf("ioctl failed %d\n", errno); > ret = -1; > break; > } gee, that's a real bug. Maybe we should in fact change kvm-all to stop doing this altogether. Anthony?