Re: [PATCH 2/5] reuse kvm_vm_ioctl

[Glauber Costa <glommer@redhat.com>]

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 <glommer@redhat.com>
> > > > > ---
> > > > >  kvm-all.c      |    2 ++
> > > > >  qemu-kvm-x86.c |   18 +++++++++---------
> > > > >  qemu-kvm.c     |   52 ++++++++++++++++++++++++++--------------------------
> > > > >  qemu-kvm.h     |    6 +++---
> > > > >  4 files changed, 40 insertions(+), 38 deletions(-)
> > > > 
> > > > <snip>
> > > > 
> > > > > @@ -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?